Skip to content

Commit fcdb15c

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 b9e0990 commit fcdb15c

3 files changed

Lines changed: 72 additions & 18 deletions

File tree

__test__/retry-helper.test.ts

Lines changed: 59 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -23,11 +23,9 @@ jest.unstable_mockModule('@actions/core', () => ({
2323
// Dynamic imports after mocking
2424
const {RetryHelper} = await import('../src/retry-helper.js')
2525

26-
let retryHelper: any
27-
2826
describe('retry-helper tests', () => {
2927
beforeAll(() => {
30-
retryHelper = new RetryHelper(3, 0, 0)
28+
// @actions/core is mocked at module load above; nothing to set up here.
3129
})
3230

3331
beforeEach(() => {
@@ -40,14 +38,22 @@ describe('retry-helper tests', () => {
4038
})
4139

4240
it('first attempt succeeds', async () => {
41+
const retryHelper: any = new RetryHelper(3, 1, 10)
42+
const sleep = jest.fn().mockResolvedValue(undefined)
43+
retryHelper.sleep = sleep
44+
4345
const actual = await retryHelper.execute(async () => {
4446
return 'some result'
4547
})
4648
expect(actual).toBe('some result')
4749
expect(info).toHaveLength(0)
50+
expect(sleep).not.toHaveBeenCalled()
4851
})
4952

5053
it('second attempt succeeds', async () => {
54+
const retryHelper: any = new RetryHelper(3, 1, 10)
55+
const sleep = jest.fn().mockResolvedValue(undefined)
56+
retryHelper.sleep = sleep
5157
let attempts = 0
5258
const actual = await retryHelper.execute(() => {
5359
if (++attempts == 1) {
@@ -60,10 +66,15 @@ describe('retry-helper tests', () => {
6066
expect(actual).toBe('some result')
6167
expect(info).toHaveLength(2)
6268
expect(info[0]).toBe('some error')
63-
expect(info[1]).toMatch(/Waiting .+ seconds before trying again/)
69+
expect(info[1]).toBe('Waiting 1 seconds before trying again')
70+
expect(sleep).toHaveBeenCalledTimes(1)
71+
expect(sleep).toHaveBeenCalledWith(1)
6472
})
6573

6674
it('third attempt succeeds', async () => {
75+
const retryHelper: any = new RetryHelper(3, 1, 10)
76+
const sleep = jest.fn().mockResolvedValue(undefined)
77+
retryHelper.sleep = sleep
6778
let attempts = 0
6879
const actual = await retryHelper.execute(() => {
6980
if (++attempts < 3) {
@@ -76,12 +87,18 @@ describe('retry-helper tests', () => {
7687
expect(actual).toBe('some result')
7788
expect(info).toHaveLength(4)
7889
expect(info[0]).toBe('some error 1')
79-
expect(info[1]).toMatch(/Waiting .+ seconds before trying again/)
90+
expect(info[1]).toBe('Waiting 1 seconds before trying again')
8091
expect(info[2]).toBe('some error 2')
81-
expect(info[3]).toMatch(/Waiting .+ seconds before trying again/)
92+
expect(info[3]).toBe('Waiting 2 seconds before trying again')
93+
expect(sleep).toHaveBeenCalledTimes(2)
94+
expect(sleep).toHaveBeenNthCalledWith(1, 1)
95+
expect(sleep).toHaveBeenNthCalledWith(2, 2)
8296
})
8397

8498
it('all attempts fail succeeds', async () => {
99+
const retryHelper: any = new RetryHelper(3, 1, 10)
100+
const sleep = jest.fn().mockResolvedValue(undefined)
101+
retryHelper.sleep = sleep
85102
let attempts = 0
86103
let error: Error = null as unknown as Error
87104
try {
@@ -95,8 +112,42 @@ describe('retry-helper tests', () => {
95112
expect(attempts).toBe(3)
96113
expect(info).toHaveLength(4)
97114
expect(info[0]).toBe('some error 1')
98-
expect(info[1]).toMatch(/Waiting .+ seconds before trying again/)
115+
expect(info[1]).toBe('Waiting 1 seconds before trying again')
99116
expect(info[2]).toBe('some error 2')
100-
expect(info[3]).toMatch(/Waiting .+ seconds before trying again/)
117+
expect(info[3]).toBe('Waiting 2 seconds before trying again')
118+
expect(sleep).toHaveBeenCalledTimes(2)
119+
expect(sleep).toHaveBeenNthCalledWith(1, 1)
120+
expect(sleep).toHaveBeenNthCalledWith(2, 2)
121+
})
122+
123+
it('server-side 500 errors are retried with exponential backoff', async () => {
124+
const retryHelper: any = new RetryHelper(4, 2, 10)
125+
const sleep = jest.fn().mockResolvedValue(undefined)
126+
retryHelper.sleep = sleep
127+
let attempts = 0
128+
129+
const actual = await retryHelper.execute(() => {
130+
if (++attempts < 3) {
131+
const error: Error & {status?: number} = new Error(
132+
`server error ${attempts}`
133+
)
134+
error.status = 500
135+
throw error
136+
}
137+
138+
return Promise.resolve('some result')
139+
})
140+
141+
expect(actual).toBe('some result')
142+
expect(attempts).toBe(3)
143+
expect(info).toEqual([
144+
'server error 1',
145+
'Waiting 2 seconds before trying again',
146+
'server error 2',
147+
'Waiting 4 seconds before trying again'
148+
])
149+
expect(sleep).toHaveBeenCalledTimes(2)
150+
expect(sleep).toHaveBeenNthCalledWith(1, 2)
151+
expect(sleep).toHaveBeenNthCalledWith(2, 4)
101152
})
102153
})

dist/index.js

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -35491,17 +35491,19 @@ class retry_helper_RetryHelper {
3549135491
info(err?.message);
3549235492
}
3549335493
// Sleep
35494-
const seconds = this.getSleepAmount();
35494+
const seconds = this.getSleepAmount(attempt);
3549535495
info(`Waiting ${seconds} seconds before trying again`);
3549635496
await this.sleep(seconds);
3549735497
attempt++;
3549835498
}
3549935499
// Last attempt
3550035500
return await action();
3550135501
}
35502-
getSleepAmount() {
35503-
return (Math.floor(Math.random() * (this.maxSeconds - this.minSeconds + 1)) +
35504-
this.minSeconds);
35502+
getSleepAmount(attempt) {
35503+
if (this.minSeconds === 0) {
35504+
return 0;
35505+
}
35506+
return Math.min(this.minSeconds * Math.pow(2, attempt - 1), this.maxSeconds);
3550535507
}
3550635508
async sleep(seconds) {
3550735509
return new Promise(resolve => setTimeout(resolve, seconds * 1000));

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)