Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement throw error with original error and reraise error #19

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
44 changes: 43 additions & 1 deletion src/retry.decorator.test.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { BackOffPolicy, Retryable } from './retry.decorator';
import { BackOffPolicy, MaxAttemptsError, Retryable } from './retry.decorator';

class TestClass {
count: number;
Expand Down Expand Up @@ -48,6 +48,15 @@ class TestClass {
await this.called();
}

@Retryable({
maxAttempts: 1,
reraise: true,
})
async reraiseError(): Promise<void> {
console.info(`Calling ExponentialBackOffRetry backOff 1s, multiplier=3 for the ${++this.count} time at ${new Date().toLocaleTimeString()}`);
await this.called();
}

async called(): Promise<string> {
return 'from real implementation';
}
Expand Down Expand Up @@ -132,5 +141,38 @@ describe('Retry Test', () => {
} catch (e) {}
expect(calledSpy).toHaveBeenCalledTimes(4);
});

class CustomError extends Error {
code = '999';
constructor(message: string) {
super(message);
Object.setPrototypeOf(this, CustomError.prototype);
}
}

test('original error is contained inside MaxAttemptsError', async () => {
jest.setTimeout(60000);
const calledSpy = jest.spyOn(testClass, 'called');
calledSpy.mockImplementation(() => { throw new CustomError("test-error"); });
try {
await testClass.testMethod();
} catch (e) {
expect(e).toBeInstanceOf(MaxAttemptsError);
expect(e.originalError).toBeInstanceOf(CustomError);
expect(e.originalError.message).toBe("test-error");
}
});

test('reraise will rethrow original Error', async () => {
jest.setTimeout(60000);
const calledSpy = jest.spyOn(testClass, 'called');
calledSpy.mockImplementation(() => { throw new CustomError("test-error"); });
try {
await testClass.reraiseError();
} catch (e) {
expect(e).toBeInstanceOf(CustomError);
expect(e.message).toBe("test-error");
}
});
});

58 changes: 28 additions & 30 deletions src/retry.decorator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,36 +22,30 @@ export function Retryable(options: RetryOptions): DecoratorFunction {
setExponentialBackOffPolicyDefault();
}
descriptor.value = async function(...args: any[]) {
try {
return await retryAsync.apply(this, [originalFn, args, options.maxAttempts, options.backOff]);
} catch (e) {
if (e instanceof MaxAttemptsError) {
const msgPrefix = `Failed for '${propertyKey}' for ${options.maxAttempts} times.`;
e.message = e.message ? `${msgPrefix} Original Error: ${e.message}` : msgPrefix;
}
throw e;
}
return await retryAsync.apply(this, [originalFn, args, options.maxAttempts, options.backOff]);
};
return descriptor;
};

async function retryAsync(fn: () => any, args: any[], maxAttempts: number, backOff?: number): Promise<any> {
try {
return await fn.apply(this, args);
} catch (e) {
if (--maxAttempts < 0) {
e?.message && console.error(e.message);
throw new MaxAttemptsError(e?.message);
}
if (!canRetry(e)) {
throw e;
}
backOff && (await sleep(backOff));
if (options.backOffPolicy === BackOffPolicy.ExponentialBackOffPolicy) {
const newBackOff: number = backOff * options.exponentialOption.multiplier;
backOff = newBackOff > options.exponentialOption.maxInterval ? options.exponentialOption.maxInterval : newBackOff;
}
return retryAsync.apply(this, [fn, args, maxAttempts, backOff]);
for (let i = 0; i<(maxAttempts+1); i++) {
try {
return await fn.apply(this, args);
} catch (e) {
if (i == maxAttempts) {
if (options.reraise) {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can we rename this option as something like useOriginalError to reduce confusion?

throw e;
}
throw new MaxAttemptsError(e, i);
}
if (!canRetry(e)) {
throw e;
}
backOff && (await sleep(backOff));
if (options.backOffPolicy === BackOffPolicy.ExponentialBackOffPolicy) {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

great simplification. 👍

backOff = Math.min(backOff * Math.pow(options.exponentialOption.multiplier, i), options.exponentialOption.maxInterval);
}
}
}
}

Expand All @@ -76,11 +70,14 @@ export function Retryable(options: RetryOptions): DecoratorFunction {

export class MaxAttemptsError extends Error {
code = '429';
/* if target is ES5, need the 'new.target.prototype'
constructor(msg?: string) {
super(msg)
Object.setPrototypeOf(this, new.target.prototype)
} */
public retryCount: number;
public originalError: Error;
constructor(originalError: Error, retryCount: number) {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

since this is a public/exported class, I guess it would be better to make them optional to keep backward comparability. What do you think?

super(`Max retry reached: ${retryCount}, original error: ${originalError.message}`);
this.originalError = originalError;
this.retryCount = retryCount;
Object.setPrototypeOf(this, MaxAttemptsError.prototype);
}
}

export interface RetryOptions {
Expand All @@ -90,6 +87,7 @@ export interface RetryOptions {
doRetry?: (e: any) => boolean;
value?: ErrorConstructor[];
exponentialOption?: { maxInterval: number; multiplier: number };
reraise?: boolean;
}

export enum BackOffPolicy {
Expand Down
1 change: 1 addition & 0 deletions tsconfig.json
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
"declarationMap": true,
"skipLibCheck": true,
"lib": [
"dom",
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

any reason we need dom here?

"es2017",
"esnext.asynciterable",
],
Expand Down