Skip to content

Commit f2248d0

Browse files
Vincent, RobertCopilot
authored andcommitted
Handle checkout 500 retries with backoff
Switch the shared retry helper from randomized delays to exponential backoff so transient GitHub 500 errors are retried predictably. Add coverage for the backoff sequence and regenerate the bundled dist output. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent 900f221 commit f2248d0

3 files changed

Lines changed: 71 additions & 18 deletions

File tree

__test__/retry-helper.test.ts

Lines changed: 58 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -2,16 +2,13 @@ import * as core from '@actions/core'
22
import {RetryHelper} from '../lib/retry-helper'
33

44
let info: string[]
5-
let retryHelper: any
65

76
describe('retry-helper tests', () => {
87
beforeAll(() => {
98
// Mock @actions/core info()
109
jest.spyOn(core, 'info').mockImplementation((message: string) => {
1110
info.push(message)
1211
})
13-
14-
retryHelper = new RetryHelper(3, 0, 0)
1512
})
1613

1714
beforeEach(() => {
@@ -25,14 +22,22 @@ describe('retry-helper tests', () => {
2522
})
2623

2724
it('first attempt succeeds', async () => {
25+
const retryHelper: any = new RetryHelper(3, 1, 10)
26+
const sleep = jest.fn().mockResolvedValue(undefined)
27+
retryHelper.sleep = sleep
28+
2829
const actual = await retryHelper.execute(async () => {
2930
return 'some result'
3031
})
3132
expect(actual).toBe('some result')
3233
expect(info).toHaveLength(0)
34+
expect(sleep).not.toHaveBeenCalled()
3335
})
3436

3537
it('second attempt succeeds', async () => {
38+
const retryHelper: any = new RetryHelper(3, 1, 10)
39+
const sleep = jest.fn().mockResolvedValue(undefined)
40+
retryHelper.sleep = sleep
3641
let attempts = 0
3742
const actual = await retryHelper.execute(() => {
3843
if (++attempts == 1) {
@@ -45,10 +50,15 @@ describe('retry-helper tests', () => {
4550
expect(actual).toBe('some result')
4651
expect(info).toHaveLength(2)
4752
expect(info[0]).toBe('some error')
48-
expect(info[1]).toMatch(/Waiting .+ seconds before trying again/)
53+
expect(info[1]).toBe('Waiting 1 seconds before trying again')
54+
expect(sleep).toHaveBeenCalledTimes(1)
55+
expect(sleep).toHaveBeenCalledWith(1)
4956
})
5057

5158
it('third attempt succeeds', async () => {
59+
const retryHelper: any = new RetryHelper(3, 1, 10)
60+
const sleep = jest.fn().mockResolvedValue(undefined)
61+
retryHelper.sleep = sleep
5262
let attempts = 0
5363
const actual = await retryHelper.execute(() => {
5464
if (++attempts < 3) {
@@ -61,12 +71,18 @@ describe('retry-helper tests', () => {
6171
expect(actual).toBe('some result')
6272
expect(info).toHaveLength(4)
6373
expect(info[0]).toBe('some error 1')
64-
expect(info[1]).toMatch(/Waiting .+ seconds before trying again/)
74+
expect(info[1]).toBe('Waiting 1 seconds before trying again')
6575
expect(info[2]).toBe('some error 2')
66-
expect(info[3]).toMatch(/Waiting .+ seconds before trying again/)
76+
expect(info[3]).toBe('Waiting 2 seconds before trying again')
77+
expect(sleep).toHaveBeenCalledTimes(2)
78+
expect(sleep).toHaveBeenNthCalledWith(1, 1)
79+
expect(sleep).toHaveBeenNthCalledWith(2, 2)
6780
})
6881

6982
it('all attempts fail succeeds', async () => {
83+
const retryHelper: any = new RetryHelper(3, 1, 10)
84+
const sleep = jest.fn().mockResolvedValue(undefined)
85+
retryHelper.sleep = sleep
7086
let attempts = 0
7187
let error: Error = null as unknown as Error
7288
try {
@@ -80,8 +96,42 @@ describe('retry-helper tests', () => {
8096
expect(attempts).toBe(3)
8197
expect(info).toHaveLength(4)
8298
expect(info[0]).toBe('some error 1')
83-
expect(info[1]).toMatch(/Waiting .+ seconds before trying again/)
99+
expect(info[1]).toBe('Waiting 1 seconds before trying again')
84100
expect(info[2]).toBe('some error 2')
85-
expect(info[3]).toMatch(/Waiting .+ seconds before trying again/)
101+
expect(info[3]).toBe('Waiting 2 seconds before trying again')
102+
expect(sleep).toHaveBeenCalledTimes(2)
103+
expect(sleep).toHaveBeenNthCalledWith(1, 1)
104+
expect(sleep).toHaveBeenNthCalledWith(2, 2)
105+
})
106+
107+
it('server-side 500 errors are retried with exponential backoff', async () => {
108+
const retryHelper: any = new RetryHelper(4, 2, 10)
109+
const sleep = jest.fn().mockResolvedValue(undefined)
110+
retryHelper.sleep = sleep
111+
let attempts = 0
112+
113+
const actual = await retryHelper.execute(() => {
114+
if (++attempts < 3) {
115+
const error: Error & {status?: number} = new Error(
116+
`server error ${attempts}`
117+
)
118+
error.status = 500
119+
throw error
120+
}
121+
122+
return Promise.resolve('some result')
123+
})
124+
125+
expect(actual).toBe('some result')
126+
expect(attempts).toBe(3)
127+
expect(info).toEqual([
128+
'server error 1',
129+
'Waiting 2 seconds before trying again',
130+
'server error 2',
131+
'Waiting 4 seconds before trying again'
132+
])
133+
expect(sleep).toHaveBeenCalledTimes(2)
134+
expect(sleep).toHaveBeenNthCalledWith(1, 2)
135+
expect(sleep).toHaveBeenNthCalledWith(2, 4)
86136
})
87137
})

dist/index.js

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2567,7 +2567,7 @@ class RetryHelper {
25672567
core.info(err === null || err === void 0 ? void 0 : err.message);
25682568
}
25692569
// Sleep
2570-
const seconds = this.getSleepAmount();
2570+
const seconds = this.getSleepAmount(attempt);
25712571
core.info(`Waiting ${seconds} seconds before trying again`);
25722572
yield this.sleep(seconds);
25732573
attempt++;
@@ -2576,9 +2576,11 @@ class RetryHelper {
25762576
return yield action();
25772577
});
25782578
}
2579-
getSleepAmount() {
2580-
return (Math.floor(Math.random() * (this.maxSeconds - this.minSeconds + 1)) +
2581-
this.minSeconds);
2579+
getSleepAmount(attempt) {
2580+
if (this.minSeconds === 0) {
2581+
return 0;
2582+
}
2583+
return Math.min(this.minSeconds * Math.pow(2, attempt - 1), this.maxSeconds);
25822584
}
25832585
sleep(seconds) {
25842586
return __awaiter(this, void 0, void 0, function* () {

src/retry-helper.ts

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@ export class RetryHelper {
3333
}
3434

3535
// Sleep
36-
const seconds = this.getSleepAmount()
36+
const seconds = this.getSleepAmount(attempt)
3737
core.info(`Waiting ${seconds} seconds before trying again`)
3838
await this.sleep(seconds)
3939
attempt++
@@ -43,11 +43,12 @@ export class RetryHelper {
4343
return await action()
4444
}
4545

46-
private getSleepAmount(): number {
47-
return (
48-
Math.floor(Math.random() * (this.maxSeconds - this.minSeconds + 1)) +
49-
this.minSeconds
50-
)
46+
private getSleepAmount(attempt: number): number {
47+
if (this.minSeconds === 0) {
48+
return 0
49+
}
50+
51+
return Math.min(this.minSeconds * Math.pow(2, attempt - 1), this.maxSeconds)
5152
}
5253

5354
private async sleep(seconds: number): Promise<void> {

0 commit comments

Comments
 (0)