Skip to content

Commit

Permalink
feat: Do not suppress errors on policy hooks
Browse files Browse the repository at this point in the history
  • Loading branch information
luczsoma committed May 7, 2020
1 parent 3074adf commit 56c5198
Show file tree
Hide file tree
Showing 6 changed files with 11 additions and 178 deletions.
72 changes: 0 additions & 72 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -308,12 +308,6 @@ policy.onRetry(async () => {
policy.onRetry(async () => {
// then this will be awaited
});

// errors thrown by an onRetryFn will be caught and ignored
policy.onRetry(() => {
// throwing an error has no effect outside the method
throw new Error();
});
```

Wait for the specified number of milliseconds before retrying:
Expand Down Expand Up @@ -434,12 +428,6 @@ policy.onFinally(async () => {
policy.onFinally(async () => {
// then this will be awaited
});

// errors thrown by an onFinallyFn will be caught and ignored
policy.onFinally(() => {
// throwing an error has no effect outside the method
throw new Error();
});
```

#### FallbackPolicy
Expand Down Expand Up @@ -500,12 +488,6 @@ policy.onFallback(async () => {
policy.onFallback(async () => {
// then this will be awaited
});

// errors thrown by an onFallbackFn will be caught and ignored
policy.onFallback(() => {
// throwing an error has no effect outside the method
throw new Error();
});
```

Perform certain actions after the execution and all fallbacks finished:
Expand All @@ -528,12 +510,6 @@ policy.onFinally(async () => {
policy.onFinally(async () => {
// then this will be awaited
});

// errors thrown by an onFinallyFn will be caught and ignored
policy.onFinally(() => {
// throwing an error has no effect outside the method
throw new Error();
});
```

#### CircuitBreakerPolicy
Expand Down Expand Up @@ -645,12 +621,6 @@ policy.onClose(async () => {
policy.onClose(async () => {
// then this will be awaited
});

// errors thrown by an onCloseFn will be caught and ignored
policy.onClose(() => {
// throwing an error has no effect outside the method
throw new Error();
});
```

```typescript
Expand All @@ -666,12 +636,6 @@ policy.onOpen(async () => {
policy.onOpen(async () => {
// then this will be awaited
});

// errors thrown by an onOpenFn will be caught and ignored
policy.onOpen(() => {
// throwing an error has no effect outside the method
throw new Error();
});
```

```typescript
Expand All @@ -687,12 +651,6 @@ policy.onAttemptingClose(async () => {
policy.onAttemptingClose(async () => {
// then this will be awaited
});

// errors thrown by an onAttemptingCloseFn will be caught and ignored
policy.onAttemptingClose(() => {
// throwing an error has no effect outside the method
throw new Error();
});
```

```typescript
Expand All @@ -708,12 +666,6 @@ policy.onIsolate(async () => {
policy.onIsolate(async () => {
// then this will be awaited
});

// errors thrown by an onIsolateFn will be caught and ignored
policy.onIsolate(() => {
// throwing an error has no effect outside the method
throw new Error();
});
```

### Proactive policies
Expand Down Expand Up @@ -782,12 +734,6 @@ policy.onTimeout(async () => {
policy.onTimeout(async () => {
// then this will be awaited
});

// errors thrown by an onTimeoutFn will be caught and ignored
policy.onTimeout(() => {
// throwing an error has no effect outside the method
throw new Error();
});
```

Throwing a `TimeoutException` from the executed method is not a timeout, therefore it does not trigger running `onTimeout` functions:
Expand Down Expand Up @@ -948,12 +894,6 @@ policy.onCacheGet(async () => {
policy.onCacheGet(async () => {
// then this will be awaited
});

// errors thrown by an onCacheGetFn will be caught and ignored
policy.onCacheGet(() => {
// throwing an error has no effect outside the method
throw new Error();
});
```

```typescript
Expand All @@ -970,12 +910,6 @@ policy.onCacheMiss(async () => {
policy.onCacheMiss(async () => {
// then this will be awaited
});

// errors thrown by an onCacheMissFn will be caught and ignored
policy.onCacheMiss(() => {
// throwing an error has no effect outside the method
throw new Error();
});
```

```typescript
Expand All @@ -992,12 +926,6 @@ policy.onCachePut(async () => {
policy.onCachePut(async () => {
// then this will be awaited
});

// errors thrown by an onCachePutFn will be caught and ignored
policy.onCachePut(() => {
// throwing an error has no effect outside the method
throw new Error();
});
```

### Helpers and utilities
Expand Down
6 changes: 1 addition & 5 deletions src/policies/proactive/timeoutPolicy/timeoutPolicy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -56,11 +56,7 @@ export class TimeoutPolicy<ResultType> extends ProactivePolicy<ResultType> {
const typedEx: ExecutionException | TimeoutException = ex;
if (typedEx instanceof TimeoutException) {
for (const onTimeoutFn of this.onTimeoutFns) {
try {
await onTimeoutFn(typedEx.timedOutAfterMs);
} catch (onTimeoutError) {
// ignored
}
await onTimeoutFn(typedEx.timedOutAfterMs);
}

throw typedEx;
Expand Down
24 changes: 4 additions & 20 deletions src/policies/reactive/circuitBreakerPolicy/circuitBreakerPolicy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -175,47 +175,31 @@ export class CircuitBreakerPolicy<ResultType> extends ReactivePolicy<ResultType>
case 'Closed': {
this.validateTransition(new Set(['AttemptingClose', 'Isolated']));
for (const onCloseFn of this.onCloseFns) {
try {
await onCloseFn();
} catch (ex) {
// ignored
}
await onCloseFn();
}
break;
}

case 'Open': {
this.validateTransition(new Set(['Closed', 'AttemptingClose']));
for (const onOpenFn of this.onOpenFns) {
try {
await onOpenFn();
} catch (ex) {
// ignored
}
await onOpenFn();
}
break;
}

case 'AttemptingClose': {
this.validateTransition(new Set(['Open']));
for (const onAttemptingCloseFn of this.onAttemptingCloseFns) {
try {
await onAttemptingCloseFn();
} catch (ex) {
// ignored
}
await onAttemptingCloseFn();
}
break;
}

case 'Isolated': {
this.validateTransition(new Set(['Closed', 'Open', 'AttemptingClose']));
for (const onIsolateFn of this.onIsolateFns) {
try {
await onIsolateFn();
} catch (ex) {
// ignored
}
await onIsolateFn();
}
break;
}
Expand Down
18 changes: 3 additions & 15 deletions src/policies/reactive/fallbackPolicy/fallbackPolicy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -50,11 +50,7 @@ export class FallbackPolicy<ResultType> extends ReactivePolicy<ResultType> {
executor = nextExecutor;

for (const onFallbackFn of this.onFallbackFns) {
try {
await onFallbackFn(result, undefined);
} catch (onFallbackError) {
// ignored
}
await onFallbackFn(result, undefined);
}

continue;
Expand All @@ -76,23 +72,15 @@ export class FallbackPolicy<ResultType> extends ReactivePolicy<ResultType> {
executor = nextExecutor;

for (const onFallbackFn of this.onFallbackFns) {
try {
await onFallbackFn(undefined, ex);
} catch (onFallbackError) {
// ignored
}
await onFallbackFn(undefined, ex);
}

continue;
}
}
} finally {
for (const onFinallyFn of this.onFinallyFns) {
try {
await onFinallyFn();
} catch (onFinallyError) {
// ignored
}
await onFinallyFn();
}
}
}
Expand Down
18 changes: 3 additions & 15 deletions src/policies/reactive/retryPolicy/retryPolicy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -75,11 +75,7 @@ export class RetryPolicy<ResultType> extends ReactivePolicy<ResultType> {
}

for (const onRetryFn of this.onRetryFns) {
try {
await onRetryFn(result, undefined, currentRetryCount);
} catch (onRetryError) {
// ignored
}
await onRetryFn(result, undefined, currentRetryCount);
}

continue;
Expand All @@ -100,23 +96,15 @@ export class RetryPolicy<ResultType> extends ReactivePolicy<ResultType> {
}

for (const onRetryFn of this.onRetryFns) {
try {
await onRetryFn(undefined, ex, currentRetryCount);
} catch (onRetryError) {
// ignored
}
await onRetryFn(undefined, ex, currentRetryCount);
}

continue;
}
}
} finally {
for (const onFinallyFn of this.onFinallyFns) {
try {
await onFinallyFn();
} catch (onFinallyError) {
// ignored
}
await onFinallyFn();
}
}
}
Expand Down
51 changes: 0 additions & 51 deletions test/specs/policyCombination.test.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,5 @@
import { expect } from 'chai';
import { PolicyCombination } from '../../src/policies/policyCombination';
import { TimeoutException } from '../../src/policies/proactive/timeoutPolicy/timeoutException';
import { TimeoutPolicy } from '../../src/policies/proactive/timeoutPolicy/timeoutPolicy';
import { FallbackPolicy } from '../../src/policies/reactive/fallbackPolicy/fallbackPolicy';
import { RetryPolicy } from '../../src/policies/reactive/retryPolicy/retryPolicy';

Expand Down Expand Up @@ -70,53 +68,4 @@ describe('PolicyCombination', (): void => {
expect(onRetryExecuted).to.equal(1);
expect(onFallbackExecuted).to.equal(1);
});

it('should construct a policy which combines the other policies sequentially', async (): Promise<void> => {
let onTimeoutExecuted = 0;
let onRetryExecuted = 0;
let onFallbackExecuted = 0;

const fallbackPolicy = new FallbackPolicy<void>();
fallbackPolicy.reactOnException((e): boolean => e instanceof TimeoutException);
fallbackPolicy.fallback((): void => {
// empty
});
fallbackPolicy.onFallback((): void => {
expect(onTimeoutExecuted).to.equal(1);
expect(onRetryExecuted).to.equal(1);
expect(onFallbackExecuted).to.equal(0);
onFallbackExecuted++;
});

const retryPolicy = new RetryPolicy<void>();
retryPolicy.reactOnException((e): boolean => e instanceof TimeoutException);
retryPolicy.onRetry((): void => {
expect(onTimeoutExecuted).to.equal(1);
expect(onRetryExecuted).to.equal(0);
expect(onFallbackExecuted).to.equal(0);
onRetryExecuted++;
});

const timeoutPolicy = new TimeoutPolicy<void>();
timeoutPolicy.timeoutAfter(1);
timeoutPolicy.onTimeout((): void => {
expect(onTimeoutExecuted).to.equal(0);
expect(onRetryExecuted).to.equal(0);
expect(onFallbackExecuted).to.equal(0);
onTimeoutExecuted++;
});

const wrappedPolicies = PolicyCombination.combine<void>([fallbackPolicy, retryPolicy, timeoutPolicy]);
await wrappedPolicies.execute(
async (): Promise<void> => {
return new Promise((resolve): void => {
setTimeout(resolve, 5);
});
},
);

expect(onTimeoutExecuted).to.equal(1);
expect(onRetryExecuted).to.equal(1);
expect(onFallbackExecuted).to.equal(1);
});
});

0 comments on commit 56c5198

Please sign in to comment.