Skip to content

Commit ae5aa7c

Browse files
committed
fix(http): preserve timeout race classification
1 parent 2af4939 commit ae5aa7c

2 files changed

Lines changed: 37 additions & 4 deletions

File tree

src/lib/http.test.ts

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -474,6 +474,33 @@ describe('HttpClient per-request timeout', () => {
474474
expect((err as Error).name).toBe('AbortError');
475475
});
476476

477+
it('preserves RequestTimeoutError when the caller aborts after the request timeout wins', async () => {
478+
const controller = new AbortController();
479+
const fetchImpl = vi.fn(async (_input: unknown, init?: { signal?: AbortSignal }) => {
480+
return new Promise<Response>((_resolve, reject) => {
481+
init?.signal?.addEventListener('abort', () => {
482+
const reason = init.signal?.reason;
483+
controller.abort(new Error('caller aborted after timeout'));
484+
const err = new Error(reason?.message ?? 'timed out');
485+
err.name = reason?.name ?? 'TimeoutError';
486+
reject(err);
487+
});
488+
});
489+
});
490+
const client = new HttpClient({
491+
baseUrl: 'https://api.example.com/api/cli/v1',
492+
apiKey: 'sk-test',
493+
fetchImpl: fetchImpl as unknown as typeof fetch,
494+
sleep: () => Promise.resolve(),
495+
random: () => 0,
496+
requestTimeoutMs: 1,
497+
});
498+
const err = await client.get('/me', { signal: controller.signal }).catch(e => e);
499+
expect(err).toBeInstanceOf(RequestTimeoutError);
500+
expect((err as RequestTimeoutError).exitCode).toBe(7);
501+
expect(fetchImpl).toHaveBeenCalledTimes(1);
502+
});
503+
477504
it('defaults to REQUEST_TIMEOUT_DEFAULT_MS when no requestTimeoutMs is supplied', () => {
478505
// Verify the default is wired without actually waiting 120s.
479506
// We test via the exported constant rather than exercising the stall.

src/lib/http.ts

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -395,9 +395,15 @@ export class HttpClient {
395395
timeoutSignal: AbortSignal,
396396
callerSignal: AbortSignal | undefined,
397397
requestId: string,
398+
effectiveSignal: AbortSignal = timeoutSignal,
398399
): void {
399400
if (isAbortError(err) || isTimeoutError(err)) {
400-
if (timeoutSignal.aborted && (callerSignal == null || !callerSignal.aborted)) {
401+
const timeoutWon =
402+
timeoutSignal.aborted &&
403+
(callerSignal == null ||
404+
!callerSignal.aborted ||
405+
effectiveSignal.reason === timeoutSignal.reason);
406+
if (timeoutWon) {
401407
throw new RequestTimeoutError(this.requestTimeoutMs, requestId);
402408
}
403409
throw err;
@@ -451,7 +457,7 @@ export class HttpClient {
451457
// the caller hadn't already aborted, surface a clear RequestTimeoutError.
452458
// A timeout/abort during the fetch itself: classify it (RequestTimeoutError
453459
// when our deadline fired; otherwise rethrow the caller's abort unmodified).
454-
this.rethrowIfAbort(err, timeoutSignal, options.signal, requestId);
460+
this.rethrowIfAbort(err, timeoutSignal, options.signal, requestId, effectiveSignal);
455461
// If a RequestTimeoutError already propagated from somewhere (e.g. from a
456462
// nested call or from a test-injected fetchImpl), pass it through unchanged
457463
// rather than re-wrapping it as a TransportError.
@@ -500,7 +506,7 @@ export class HttpClient {
500506
return { body: (await response.json()) as T, requestId, status: response.status };
501507
} catch (err) {
502508
// A timeout/abort can fire mid-body-read (headers received, stream stalls).
503-
this.rethrowIfAbort(err, timeoutSignal, options.signal, requestId);
509+
this.rethrowIfAbort(err, timeoutSignal, options.signal, requestId, effectiveSignal);
504510
throw err;
505511
}
506512
}
@@ -511,7 +517,7 @@ export class HttpClient {
511517
} catch (err) {
512518
// safeReadJson rethrows aborts/timeouts (it swallows only non-abort parse
513519
// errors), so a timeout fired mid-body-read on a non-OK response lands here.
514-
this.rethrowIfAbort(err, timeoutSignal, options.signal, requestId);
520+
this.rethrowIfAbort(err, timeoutSignal, options.signal, requestId, effectiveSignal);
515521
throw err;
516522
}
517523

0 commit comments

Comments
 (0)