Skip to content

Commit 53921b9

Browse files
committed
fix: retry transient HTTP failures
1 parent 2f829ae commit 53921b9

2 files changed

Lines changed: 256 additions & 11 deletions

File tree

‎src/client/http.ts‎

Lines changed: 74 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,50 @@ export interface RequestOpts {
1818
authStyle?: 'bearer' | 'x-api-key';
1919
}
2020

21+
const MAX_ATTEMPTS = 3;
22+
const BASE_RETRY_DELAY_MS = 250;
23+
const MAX_RETRY_DELAY_MS = 30_000;
24+
const RETRYABLE_STATUSES = new Set([408, 429, 500, 502, 503, 504]);
25+
26+
function parseRetryAfter(value: string | null, now: number): number | undefined {
27+
if (value === null) return undefined;
28+
29+
const seconds = Number(value);
30+
if (Number.isFinite(seconds) && seconds >= 0) {
31+
return seconds * 1000;
32+
}
33+
34+
const date = Date.parse(value);
35+
if (Number.isNaN(date)) return undefined;
36+
return Math.max(0, date - now);
37+
}
38+
39+
export function retryDelayMs(
40+
retryNumber: number,
41+
retryAfter: string | null,
42+
now = Date.now(),
43+
random = Math.random(),
44+
): number {
45+
const serverDelay = parseRetryAfter(retryAfter, now);
46+
if (serverDelay !== undefined) {
47+
return Math.min(serverDelay, MAX_RETRY_DELAY_MS);
48+
}
49+
50+
const exponentialDelay = BASE_RETRY_DELAY_MS * 2 ** (retryNumber - 1);
51+
return Math.min(exponentialDelay * Math.max(0, Math.min(random, 1)), MAX_RETRY_DELAY_MS);
52+
}
53+
54+
function isRetryableNetworkError(error: unknown): boolean {
55+
return error instanceof TypeError ||
56+
(error instanceof DOMException && error.name === 'TimeoutError');
57+
}
58+
59+
async function waitBeforeRetry(retryNumber: number, retryAfter: string | null): Promise<void> {
60+
const delay = retryDelayMs(retryNumber, retryAfter);
61+
if (delay <= 0) return;
62+
await new Promise(resolve => setTimeout(resolve, delay));
63+
}
64+
2165
export async function request(config: Config, opts: RequestOpts): Promise<Response> {
2266
const isFormData = typeof FormData !== 'undefined' && opts.body instanceof FormData;
2367

@@ -53,17 +97,37 @@ export async function request(config: Config, opts: RequestOpts): Promise<Respon
5397
}
5498

5599
const timeoutMs = (opts.timeout ?? config.timeout) * 1000;
100+
const requestBody = opts.body
101+
? isFormData
102+
? (opts.body as FormData)
103+
: JSON.stringify(opts.body)
104+
: undefined;
105+
106+
let res: Response | undefined;
107+
for (let attempt = 1; attempt <= MAX_ATTEMPTS; attempt++) {
108+
try {
109+
res = await fetch(opts.url, {
110+
method: opts.method ?? 'GET',
111+
headers,
112+
body: requestBody,
113+
signal: opts.stream ? undefined : AbortSignal.timeout(timeoutMs),
114+
});
115+
} catch (error) {
116+
if (attempt === MAX_ATTEMPTS || !isRetryableNetworkError(error)) throw error;
117+
await waitBeforeRetry(attempt, null);
118+
continue;
119+
}
56120

57-
const res = await fetch(opts.url, {
58-
method: opts.method ?? 'GET',
59-
headers,
60-
body: opts.body
61-
? isFormData
62-
? (opts.body as FormData)
63-
: JSON.stringify(opts.body)
64-
: undefined,
65-
signal: opts.stream ? undefined : AbortSignal.timeout(timeoutMs),
66-
});
121+
if (!RETRYABLE_STATUSES.has(res.status) || attempt === MAX_ATTEMPTS) break;
122+
123+
const retryAfter = res.headers.get('retry-after');
124+
await res.body?.cancel();
125+
await waitBeforeRetry(attempt, retryAfter);
126+
}
127+
128+
if (!res) {
129+
throw new Error('HTTP request completed without a response');
130+
}
67131

68132
if (config.verbose) {
69133
process.stderr.write(`< ${res.status} ${res.statusText}\n`);

‎test/client/http.test.ts‎

Lines changed: 182 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import { describe, it, expect, afterEach } from 'bun:test';
2-
import { requestJson } from '../../src/client/http';
2+
import { requestJson, retryDelayMs } from '../../src/client/http';
33
import { CLI_VERSION } from '../../src/version';
44
import { createMockServer, jsonResponse, type MockServer } from '../helpers/mock-server';
55
import type { Config } from '../../src/config/schema';
@@ -23,9 +23,11 @@ function makeConfig(baseUrl: string): Config {
2323

2424
describe('HTTP client', () => {
2525
let server: MockServer;
26+
const originalFetch = globalThis.fetch;
2627

2728
afterEach(() => {
2829
server?.close();
30+
globalThis.fetch = originalFetch;
2931
});
3032

3133
it('makes authenticated GET request', async () => {
@@ -93,4 +95,183 @@ describe('HTTP client', () => {
9395
requestJson(config, { url: `${server.url}/v1/test` }),
9496
).rejects.toThrow('Rate limit');
9597
});
98+
99+
it.each([408, 429, 500, 502, 503, 504])('retries HTTP %d and then succeeds', async (status) => {
100+
let attempts = 0;
101+
server = createMockServer({
102+
routes: {
103+
'/v1/test': () => {
104+
attempts++;
105+
if (attempts < 3) {
106+
return new Response('transient', {
107+
status,
108+
headers: { 'Retry-After': '0' },
109+
});
110+
}
111+
return jsonResponse({ result: 'ok' });
112+
},
113+
},
114+
});
115+
116+
const result = await requestJson<{ result: string }>(makeConfig(server.url), {
117+
url: `${server.url}/v1/test`,
118+
});
119+
120+
expect(result.result).toBe('ok');
121+
expect(attempts).toBe(3);
122+
});
123+
124+
it('stops after the bounded number of attempts', async () => {
125+
let attempts = 0;
126+
server = createMockServer({
127+
routes: {
128+
'/v1/test': () => {
129+
attempts++;
130+
return new Response('unavailable', {
131+
status: 503,
132+
headers: { 'Retry-After': '0' },
133+
});
134+
},
135+
},
136+
});
137+
138+
await expect(
139+
requestJson(makeConfig(server.url), { url: `${server.url}/v1/test` }),
140+
).rejects.toThrow('HTTP 503');
141+
expect(attempts).toBe(3);
142+
});
143+
144+
it.each([400, 401, 403, 404, 409, 422])('does not retry permanent HTTP %d errors', async (status) => {
145+
let attempts = 0;
146+
server = createMockServer({
147+
routes: {
148+
'/v1/test': () => {
149+
attempts++;
150+
return jsonResponse({ error: { message: 'permanent' } }, status);
151+
},
152+
},
153+
});
154+
155+
await expect(
156+
requestJson(makeConfig(server.url), { url: `${server.url}/v1/test` }),
157+
).rejects.toBeInstanceOf(Error);
158+
expect(attempts).toBe(1);
159+
});
160+
161+
it('retries transient network failures', async () => {
162+
let attempts = 0;
163+
globalThis.fetch = Object.assign(
164+
async (...args: Parameters<typeof fetch>) => {
165+
attempts++;
166+
if (attempts === 1) throw new TypeError('connection reset');
167+
return originalFetch(...args);
168+
},
169+
{ preconnect: originalFetch.preconnect },
170+
);
171+
server = createMockServer({
172+
routes: { '/v1/test': () => jsonResponse({ result: 'ok' }) },
173+
});
174+
175+
const result = await requestJson<{ result: string }>(makeConfig(server.url), {
176+
url: `${server.url}/v1/test`,
177+
});
178+
179+
expect(result.result).toBe('ok');
180+
expect(attempts).toBe(2);
181+
});
182+
183+
it('retries per-attempt timeout failures', async () => {
184+
let attempts = 0;
185+
globalThis.fetch = Object.assign(
186+
async (...args: Parameters<typeof fetch>) => {
187+
attempts++;
188+
if (attempts === 1) throw new DOMException('timed out', 'TimeoutError');
189+
return originalFetch(...args);
190+
},
191+
{ preconnect: originalFetch.preconnect },
192+
);
193+
server = createMockServer({
194+
routes: { '/v1/test': () => jsonResponse({ result: 'ok' }) },
195+
});
196+
197+
const result = await requestJson<{ result: string }>(makeConfig(server.url), {
198+
url: `${server.url}/v1/test`,
199+
});
200+
201+
expect(result.result).toBe('ok');
202+
expect(attempts).toBe(2);
203+
});
204+
205+
it('replays JSON request bodies on retry', async () => {
206+
const bodies: unknown[] = [];
207+
server = createMockServer({
208+
routes: {
209+
'/v1/test': async (req) => {
210+
bodies.push(await req.json());
211+
if (bodies.length === 1) {
212+
return new Response('unavailable', {
213+
status: 503,
214+
headers: { 'Retry-After': '0' },
215+
});
216+
}
217+
return jsonResponse({ result: 'ok' });
218+
},
219+
},
220+
});
221+
222+
await requestJson(makeConfig(server.url), {
223+
url: `${server.url}/v1/test`,
224+
method: 'POST',
225+
body: { hello: 'world' },
226+
});
227+
228+
expect(bodies).toEqual([{ hello: 'world' }, { hello: 'world' }]);
229+
});
230+
231+
it('replays FormData request bodies on retry', async () => {
232+
const values: string[] = [];
233+
server = createMockServer({
234+
routes: {
235+
'/v1/test': async (req) => {
236+
const form = await req.formData();
237+
values.push(String(form.get('purpose')));
238+
if (values.length === 1) {
239+
return new Response('unavailable', {
240+
status: 503,
241+
headers: { 'Retry-After': '0' },
242+
});
243+
}
244+
return jsonResponse({ result: 'ok' });
245+
},
246+
},
247+
});
248+
const body = new FormData();
249+
body.append('purpose', 'retrieval');
250+
251+
await requestJson(makeConfig(server.url), {
252+
url: `${server.url}/v1/test`,
253+
method: 'POST',
254+
body,
255+
});
256+
257+
expect(values).toEqual(['retrieval', 'retrieval']);
258+
});
259+
});
260+
261+
describe('retryDelayMs', () => {
262+
it('uses deterministic exponential backoff with injected jitter', () => {
263+
expect(retryDelayMs(1, null, 0, 1)).toBe(250);
264+
expect(retryDelayMs(2, null, 0, 0.5)).toBe(250);
265+
});
266+
267+
it('supports Retry-After seconds and HTTP dates', () => {
268+
const now = Date.parse('2026-08-05T00:00:00Z');
269+
expect(retryDelayMs(1, '2', now, 0)).toBe(2_000);
270+
expect(retryDelayMs(1, 'Wed, 05 Aug 2026 00:00:03 GMT', now, 0)).toBe(3_000);
271+
});
272+
273+
it('caps Retry-After and falls back for invalid values', () => {
274+
expect(retryDelayMs(1, '120', 0, 0)).toBe(30_000);
275+
expect(retryDelayMs(1, 'invalid', 0, 1)).toBe(250);
276+
});
96277
});

0 commit comments

Comments
 (0)