Skip to content

Commit 674e8e2

Browse files
fix(http): avoid retrying keyless writes
1 parent 60d55e4 commit 674e8e2

2 files changed

Lines changed: 67 additions & 2 deletions

File tree

src/lib/http.test.ts

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -260,6 +260,31 @@ describe('HttpClient error mapping', () => {
260260
expect(fetchImpl).toHaveBeenCalledTimes(4);
261261
});
262262

263+
it('does not retry transport errors for keyless writes', async () => {
264+
const fetchImpl = vi.fn(async () => {
265+
throw new Error('ECONNRESET');
266+
});
267+
const client = makeClient(fetchImpl as unknown as typeof fetch);
268+
await expect(client.post('/projects', { body: { name: 'Checkout' } })).rejects.toBeInstanceOf(
269+
TransportError,
270+
);
271+
expect(fetchImpl).toHaveBeenCalledTimes(1);
272+
});
273+
274+
it('retries transport errors for writes with an idempotency key', async () => {
275+
const fetchImpl = vi.fn(async () => {
276+
throw new Error('ECONNRESET');
277+
});
278+
const client = makeClient(fetchImpl as unknown as typeof fetch);
279+
await expect(
280+
client.post('/projects', {
281+
body: { name: 'Checkout' },
282+
headers: { 'Idempotency-Key': 'op_123' },
283+
}),
284+
).rejects.toBeInstanceOf(TransportError);
285+
expect(fetchImpl).toHaveBeenCalledTimes(4);
286+
});
287+
263288
it('does not retry AbortError', async () => {
264289
const fetchImpl = vi.fn(async () => {
265290
const err = new Error('aborted');
@@ -331,6 +356,27 @@ describe('HttpClient transport-edge statuses', () => {
331356
},
332357
);
333358

359+
it('does not retry bare transport-edge responses for keyless writes', async () => {
360+
const fetchImpl = vi.fn(async () => new Response('proxy gateway html', { status: 502 }));
361+
const client = makeClient(fetchImpl as unknown as typeof fetch);
362+
await expect(client.post('/projects', { body: { name: 'Checkout' } })).rejects.toBeInstanceOf(
363+
TransportError,
364+
);
365+
expect(fetchImpl).toHaveBeenCalledTimes(1);
366+
});
367+
368+
it('retries bare transport-edge responses for writes with an idempotency key', async () => {
369+
const fetchImpl = vi.fn(async () => new Response('proxy gateway html', { status: 502 }));
370+
const client = makeClient(fetchImpl as unknown as typeof fetch);
371+
await expect(
372+
client.post('/projects', {
373+
body: { name: 'Checkout' },
374+
headers: { 'idempotency-key': 'op_123' },
375+
}),
376+
).rejects.toBeInstanceOf(TransportError);
377+
expect(fetchImpl).toHaveBeenCalledTimes(4);
378+
});
379+
334380
it('502 carrying our envelope still maps to its catalog code', async () => {
335381
const body = {
336382
error: {

src/lib/http.ts

Lines changed: 21 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -419,6 +419,7 @@ export class HttpClient {
419419

420420
const url = buildUrl(this.baseUrl, path, options.query);
421421
const requestId = options.requestId ?? newRequestId();
422+
const allowTransportRetry = canRetryTransport(method, options);
422423

423424
let attempt = 0;
424425
while (true) {
@@ -472,7 +473,9 @@ export class HttpClient {
472473
errorCode: 'TRANSPORT',
473474
durationMs: Date.now() - startedAt,
474475
});
475-
const decision = transportRetryDecision(attempt, this.random);
476+
const decision = allowTransportRetry
477+
? transportRetryDecision(attempt, this.random)
478+
: { retry: false, delayMs: 0 };
476479
if (!decision.retry) throw new TransportError(message, requestId);
477480
this.transition(
478481
`Network error on ${shortPath(path)} — retrying in ${Math.round(decision.delayMs / 1000)}s (attempt ${attempt})`,
@@ -542,7 +545,9 @@ export class HttpClient {
542545
errorCode: 'TRANSPORT',
543546
durationMs,
544547
});
545-
const decision = transportRetryDecision(attempt, this.random);
548+
const decision = allowTransportRetry
549+
? transportRetryDecision(attempt, this.random)
550+
: { retry: false, delayMs: 0 };
546551
if (!decision.retry) {
547552
throw new TransportError(`HTTP ${response.status} from ${url}`, requestId);
548553
}
@@ -825,6 +830,20 @@ function transportRetryDecision(attempt: number, random: () => number): RetryDec
825830
return { retry: true, delayMs: backoffDelay(attempt, random) };
826831
}
827832

833+
function canRetryTransport(method: string, options: RequestOptions): boolean {
834+
return isIdempotentMethod(method) || hasIdempotencyKey(options.headers);
835+
}
836+
837+
function isIdempotentMethod(method: string): boolean {
838+
const normalized = method.toUpperCase();
839+
return normalized === 'GET' || normalized === 'HEAD';
840+
}
841+
842+
function hasIdempotencyKey(headers: Record<string, string> | undefined): boolean {
843+
if (!headers) return false;
844+
return Object.keys(headers).some(name => name.toLowerCase() === 'idempotency-key');
845+
}
846+
828847
function apiRetryDecision(
829848
code: ErrorCode,
830849
attempt: number,

0 commit comments

Comments
 (0)