Skip to content
This repository was archived by the owner on Aug 1, 2022. It is now read-only.

Commit 58a7921

Browse files
fernyettheplantfernyettheplant
authored
Merge pull request #107 from sam-n-johnston/feat/add-better-retries
2 parents 8071e71 + bedc83b commit 58a7921

File tree

6 files changed

+80
-68
lines changed

6 files changed

+80
-68
lines changed

‎package-lock.json

Lines changed: 1 addition & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎package.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
{
22
"name": "@fernthedev/serverless-offline-step-functions",
3-
"version": "1.0.0-alpha.14",
3+
"version": "1.0.0-alpha.15",
44
"description": "Serverless Offline Plugin to Support Step Functions for Local Development",
55
"main": "dist/main.js",
66
"types": "dist/main.d.ts",

‎src/stateTasks/executors/TaskExecutor.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -35,8 +35,8 @@ export class TaskExecutor extends StateTypeExecutor {
3535
let output: any;
3636
try {
3737
if (stateDefinition.Retry) {
38-
const retrier = Retriers.create(stateDefinition.Retry);
39-
output = await retrier.retry(() => functionLambda[stateInfo.handlerName](input, context), context);
38+
const retriers = Retriers.create(stateDefinition.Retry);
39+
output = await retriers.retry(() => functionLambda[stateInfo.handlerName](input, context), context);
4040
} else {
4141
output = await functionLambda[stateInfo.handlerName](input, context);
4242
}

‎src/types/Retrier.ts

Lines changed: 16 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -1,21 +1,17 @@
1-
import { Context } from '../Context/Context';
2-
import { Logger } from '../utils/Logger';
31
import { TaskRetryRule } from './State';
42
import { StatesErrors } from './StatesErrors';
53

64
export class Retrier {
7-
private currentNumberOfRetries = 0;
8-
private currentIntervalSeconds: number;
9-
private logger: Logger;
5+
private _currentNumberOfRetries = 0;
6+
private _currentIntervalSeconds: number;
107

118
private constructor(
129
private readonly _ErrorEquals: StatesErrors[],
1310
private readonly _IntervalSeconds: number = 1,
1411
private readonly _MaxAttempts: number = 3,
1512
private readonly _BackoffRate: number = 2.0,
1613
) {
17-
this.currentIntervalSeconds = _IntervalSeconds;
18-
this.logger = Logger.getInstance();
14+
this._currentIntervalSeconds = _IntervalSeconds;
1915
}
2016

2117
public static create(taskRetryRule: TaskRetryRule): Retrier {
@@ -27,38 +23,19 @@ export class Retrier {
2723
);
2824
}
2925

30-
async retry(fn: () => any, context: Context): Promise<any> {
31-
let output: unknown;
32-
try {
33-
output = await fn();
34-
return output;
35-
} catch (error) {
36-
this.logger.error(
37-
`Caught an error in Retrier for ${context.StateMachine.Name}-${context.State.Name}: ${error.stack}`,
38-
);
39-
if (this.currentNumberOfRetries < this._MaxAttempts) {
40-
this.currentNumberOfRetries++;
41-
this.logger.log(
42-
`Retrying ${context.StateMachine.Name}-${context.State.Name}, attempt #${this.currentNumberOfRetries}`,
43-
);
44-
return await new Promise((resolve, reject) => {
45-
setTimeout(async () => {
46-
this.currentIntervalSeconds = this.currentIntervalSeconds * this._BackoffRate;
47-
try {
48-
const output = await this.retry(fn, context);
49-
return resolve(output);
50-
} catch (error) {
51-
return reject(error);
52-
}
53-
}, this.currentIntervalSeconds * 1000);
54-
});
55-
} else {
56-
this.logger.log(
57-
`Already retried MaxAttemps of ${this.currentNumberOfRetries} for ${context.StateMachine.Name}-${context.State.Name}`,
58-
);
59-
throw error;
60-
}
61-
}
26+
public shouldRetry(): boolean {
27+
return this._currentNumberOfRetries < this._MaxAttempts;
28+
}
29+
public retried(): void {
30+
this._currentNumberOfRetries = this._currentNumberOfRetries + 1;
31+
this._currentIntervalSeconds = this._currentIntervalSeconds * this._BackoffRate;
32+
}
33+
34+
get currentNumberOfRetries(): number {
35+
return this._currentNumberOfRetries;
36+
}
37+
get currentIntervalSeconds(): number {
38+
return this._currentIntervalSeconds;
6239
}
6340

6441
get ErrorEquals(): StatesErrors[] {

‎src/types/Retriers.ts

Lines changed: 41 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,15 @@
11
import { Context } from '../Context/Context';
2+
import { Logger } from '../utils/Logger';
23
import { Retrier } from './Retrier';
34
import { TaskRetryRule } from './State';
45
import { StatesErrors } from './StatesErrors';
56

67
export class Retriers {
7-
private constructor(private readonly _retriers: Retrier[]) {}
8+
private logger: Logger;
9+
10+
private constructor(private readonly _retriers: Retrier[]) {
11+
this.logger = Logger.getInstance();
12+
}
813

914
public static create(taskRetryRules: TaskRetryRule[]): Retriers {
1015
const retriers = taskRetryRules.map((taskRetryRule) => {
@@ -13,9 +18,16 @@ export class Retriers {
1318
return new Retriers(retriers);
1419
}
1520

16-
private getRetrierBasedOn(statesError: StatesErrors): Retrier | undefined {
21+
// eslint-disable-next-line @typescript-eslint/no-unused-vars
22+
private getStatesErrorFrom(error: Error): StatesErrors {
23+
// TODO: Use error to determine if timeout occured
24+
return StatesErrors.TaskFailed;
25+
}
26+
27+
private getRetrierBasedOn(error: Error): Retrier | undefined {
28+
const errorType: StatesErrors = this.getStatesErrorFrom(error);
1729
const retrier = this._retriers.find((retrier) => {
18-
return retrier.ErrorEquals.includes(statesError);
30+
return retrier.ErrorEquals.includes(errorType) || retrier.ErrorEquals.includes(StatesErrors.All);
1931
});
2032
if (!retrier) {
2133
return;
@@ -25,12 +37,33 @@ export class Retriers {
2537

2638
// TODO: Add the number of retries to the context object
2739
async retry(fn: () => any, context: Context): Promise<any> {
28-
const retrier = this.getRetrierBasedOn(StatesErrors.TaskFailed);
40+
try {
41+
const output = await fn();
42+
return output;
43+
} catch (error) {
44+
const retrier = this.getRetrierBasedOn(error);
45+
46+
if (retrier && retrier.shouldRetry()) {
47+
this.logger.log(
48+
`Retrying ${context.StateMachine.Name}-${context.State.Name}, retry #${retrier.currentNumberOfRetries}`,
49+
);
50+
51+
const interval = retrier.currentIntervalSeconds;
52+
53+
return await new Promise((resolve, reject) => {
54+
setTimeout(async () => {
55+
try {
56+
retrier.retried();
57+
const output = await this.retry(fn, context);
58+
return resolve(output);
59+
} catch (error) {
60+
return reject(error);
61+
}
62+
}, interval * 1000);
63+
});
64+
}
2965

30-
if (retrier) {
31-
return await retrier.retry(fn, context);
32-
} else {
33-
return fn();
66+
throw error;
3467
}
3568
}
3669
}

‎tests/src/Retrier.test.ts renamed to ‎tests/src/Retriers.test.ts

Lines changed: 19 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,9 @@
11
import { Context } from '../../src/Context/Context';
2-
import { Retrier } from '../../src/types/Retrier';
2+
import { Retriers } from '../../src/types/Retriers';
33
import { StatesErrors } from '../../src/types/StatesErrors';
44
import { Logger } from '../../src/utils/Logger';
55

6-
describe('Retrier', () => {
6+
describe('Retriers', () => {
77
const context = ({
88
Task: { Token: 'OneTaskToken' },
99
Execution: { Id: 'MyExecutionId' },
@@ -25,17 +25,17 @@ describe('Retrier', () => {
2525

2626
describe('when the function succeeds', () => {
2727
it('should call it once', async () => {
28-
const retrier = Retrier.create({ ErrorEquals: [StatesErrors.TaskFailed] });
28+
const retriers = Retriers.create([{ ErrorEquals: [StatesErrors.TaskFailed] }]);
2929
const retriedFunction = jest.fn();
30-
await retrier.retry(retriedFunction, context);
30+
await retriers.retry(retriedFunction, context);
3131
expect(retriedFunction).toHaveBeenCalledTimes(1);
3232
});
3333

3434
describe('when the function is async', () => {
3535
it('should call it once', async () => {
36-
const retrier = Retrier.create({ ErrorEquals: [StatesErrors.TaskFailed] });
36+
const retriers = Retriers.create([{ ErrorEquals: [StatesErrors.TaskFailed] }]);
3737
const retriedFunction = jest.fn().mockImplementation(() => Promise.resolve);
38-
await retrier.retry(retriedFunction, context);
38+
await retriers.retry(retriedFunction, context);
3939
expect(retriedFunction).toHaveBeenCalledTimes(1);
4040
});
4141
});
@@ -46,12 +46,12 @@ describe('Retrier', () => {
4646
it('should call it 4 times', (done) => {
4747
expect.assertions(2);
4848

49-
const retrier = Retrier.create({ ErrorEquals: [StatesErrors.TaskFailed] });
49+
const retriers = Retriers.create([{ ErrorEquals: [StatesErrors.TaskFailed] }]);
5050
const retriedFunction = jest.fn().mockImplementation(() => {
5151
throw new Error('MyError');
5252
});
5353

54-
retrier.retry(retriedFunction, context).catch((error) => {
54+
retriers.retry(retriedFunction, context).catch((error) => {
5555
expect(error.message).toEqual('MyError');
5656
expect(retriedFunction).toHaveBeenCalledTimes(4);
5757
done();
@@ -63,12 +63,12 @@ describe('Retrier', () => {
6363
it('should call it 6 times', (done) => {
6464
expect.assertions(2);
6565

66-
const retrier = Retrier.create({ ErrorEquals: [StatesErrors.TaskFailed], MaxAttempts: 5 });
66+
const retriers = Retriers.create([{ ErrorEquals: [StatesErrors.TaskFailed], MaxAttempts: 5 }]);
6767
const retriedFunction = jest.fn().mockImplementation(() => {
6868
throw new Error('MyError');
6969
});
7070

71-
retrier.retry(retriedFunction, context).catch((error) => {
71+
retriers.retry(retriedFunction, context).catch((error) => {
7272
expect(error.message).toEqual('MyError');
7373
expect(retriedFunction).toHaveBeenCalledTimes(6);
7474
done();
@@ -79,18 +79,20 @@ describe('Retrier', () => {
7979
it('should call it with correct interval', (done) => {
8080
expect.assertions(5);
8181

82-
const retrier = Retrier.create({
83-
ErrorEquals: [StatesErrors.TaskFailed],
84-
MaxAttempts: 5,
85-
BackoffRate: 2.0,
86-
IntervalSeconds: 5,
87-
});
82+
const retriers = Retriers.create([
83+
{
84+
ErrorEquals: [StatesErrors.TaskFailed],
85+
MaxAttempts: 5,
86+
BackoffRate: 2.0,
87+
IntervalSeconds: 5,
88+
},
89+
]);
8890

8991
const retriedFunction = jest.fn().mockImplementation(() => {
9092
throw new Error('MyError');
9193
});
9294

93-
retrier.retry(retriedFunction, context).catch(() => {
95+
retriers.retry(retriedFunction, context).catch(() => {
9496
expect(setTimeout).toHaveBeenNthCalledWith(1, expect.any(Function), 5 * 1000);
9597
expect(setTimeout).toHaveBeenNthCalledWith(2, expect.any(Function), 10 * 1000);
9698
expect(setTimeout).toHaveBeenNthCalledWith(3, expect.any(Function), 20 * 1000);

0 commit comments

Comments
 (0)