Skip to content

Commit 34f5d43

Browse files
fix(http): avoid retrying keyless writes (#256)
1 parent 94f78ad commit 34f5d43

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
@@ -328,6 +328,31 @@ describe('HttpClient error mapping', () => {
328328
expect(fetchImpl).toHaveBeenCalledTimes(4);
329329
});
330330

331+
it('does not retry transport errors for keyless writes', async () => {
332+
const fetchImpl = vi.fn(async () => {
333+
throw new Error('ECONNRESET');
334+
});
335+
const client = makeClient(fetchImpl as unknown as typeof fetch);
336+
await expect(client.post('/projects', { body: { name: 'Checkout' } })).rejects.toBeInstanceOf(
337+
TransportError,
338+
);
339+
expect(fetchImpl).toHaveBeenCalledTimes(1);
340+
});
341+
342+
it('retries transport errors for writes with an idempotency key', async () => {
343+
const fetchImpl = vi.fn(async () => {
344+
throw new Error('ECONNRESET');
345+
});
346+
const client = makeClient(fetchImpl as unknown as typeof fetch);
347+
await expect(
348+
client.post('/projects', {
349+
body: { name: 'Checkout' },
350+
headers: { 'Idempotency-Key': 'op_123' },
351+
}),
352+
).rejects.toBeInstanceOf(TransportError);
353+
expect(fetchImpl).toHaveBeenCalledTimes(4);
354+
});
355+
331356
it('does not retry AbortError', async () => {
332357
const fetchImpl = vi.fn(async () => {
333358
const err = new Error('aborted');
@@ -399,6 +424,27 @@ describe('HttpClient transport-edge statuses', () => {
399424
},
400425
);
401426

427+
it('does not retry bare transport-edge responses for keyless writes', async () => {
428+
const fetchImpl = vi.fn(async () => new Response('proxy gateway html', { status: 502 }));
429+
const client = makeClient(fetchImpl as unknown as typeof fetch);
430+
await expect(client.post('/projects', { body: { name: 'Checkout' } })).rejects.toBeInstanceOf(
431+
TransportError,
432+
);
433+
expect(fetchImpl).toHaveBeenCalledTimes(1);
434+
});
435+
436+
it('retries bare transport-edge responses for writes with an idempotency key', async () => {
437+
const fetchImpl = vi.fn(async () => new Response('proxy gateway html', { status: 502 }));
438+
const client = makeClient(fetchImpl as unknown as typeof fetch);
439+
await expect(
440+
client.post('/projects', {
441+
body: { name: 'Checkout' },
442+
headers: { 'idempotency-key': 'op_123' },
443+
}),
444+
).rejects.toBeInstanceOf(TransportError);
445+
expect(fetchImpl).toHaveBeenCalledTimes(4);
446+
});
447+
402448
it('502 carrying our envelope still maps to its catalog code', async () => {
403449
const body = {
404450
error: {

src/lib/http.ts

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

488488
const url = buildUrl(this.baseUrl, path, options.query);
489489
const requestId = options.requestId ?? newRequestId();
490+
const allowTransportRetry = canRetryTransport(method, options);
490491

491492
let attempt = 0;
492493
while (true) {
@@ -557,7 +558,9 @@ export class HttpClient {
557558
errorCode: 'TRANSPORT',
558559
durationMs: Date.now() - startedAt,
559560
});
560-
const decision = transportRetryDecision(attempt, this.random);
561+
const decision = allowTransportRetry
562+
? transportRetryDecision(attempt, this.random)
563+
: { retry: false, delayMs: 0 };
561564
if (!decision.retry) throw new TransportError(message, requestId);
562565
this.transition(
563566
`Network error on ${shortPath(path)} — retrying in ${Math.round(decision.delayMs / 1000)}s (attempt ${attempt})`,
@@ -635,7 +638,9 @@ export class HttpClient {
635638
errorCode: 'TRANSPORT',
636639
durationMs,
637640
});
638-
const decision = transportRetryDecision(attempt, this.random);
641+
const decision = allowTransportRetry
642+
? transportRetryDecision(attempt, this.random)
643+
: { retry: false, delayMs: 0 };
639644
if (!decision.retry) {
640645
throw new TransportError(`HTTP ${response.status} from ${url}`, requestId);
641646
}
@@ -945,6 +950,20 @@ function transportRetryDecision(attempt: number, random: () => number): RetryDec
945950
return { retry: true, delayMs: backoffDelay(attempt, random) };
946951
}
947952

953+
function canRetryTransport(method: string, options: RequestOptions): boolean {
954+
return isIdempotentMethod(method) || hasIdempotencyKey(options.headers);
955+
}
956+
957+
function isIdempotentMethod(method: string): boolean {
958+
const normalized = method.toUpperCase();
959+
return normalized === 'GET' || normalized === 'HEAD';
960+
}
961+
962+
function hasIdempotencyKey(headers: Record<string, string> | undefined): boolean {
963+
if (!headers) return false;
964+
return Object.keys(headers).some(name => name.toLowerCase() === 'idempotency-key');
965+
}
966+
948967
function apiRetryDecision(
949968
code: ErrorCode,
950969
attempt: number,

0 commit comments

Comments
 (0)