Skip to content

Commit c6d37b7

Browse files
authored
fix(http): clear per-attempt timeout timers (#137)
* fix(http): clear per-attempt timeout timers * fix(http): preserve timeout race classification
1 parent f7b10b7 commit c6d37b7

2 files changed

Lines changed: 277 additions & 155 deletions

File tree

src/lib/http.test.ts

Lines changed: 73 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -449,6 +449,52 @@ describe('HttpClient per-request timeout', () => {
449449
expect(callCount).toBe(1);
450450
});
451451

452+
it('clears the per-attempt timeout after a successful response body is read', async () => {
453+
let capturedSignal: AbortSignal | undefined;
454+
const fetchImpl = vi.fn(async (_input: unknown, init?: RequestInit) => {
455+
capturedSignal = init?.signal ?? undefined;
456+
return jsonResponse({ ok: true });
457+
});
458+
const client = new HttpClient({
459+
baseUrl: 'https://api.example.com/api/cli/v1',
460+
apiKey: 'sk-test',
461+
fetchImpl: fetchImpl as unknown as typeof fetch,
462+
sleep: () => Promise.resolve(),
463+
random: () => 0,
464+
requestTimeoutMs: 25,
465+
});
466+
467+
await expect(client.get('/me')).resolves.toEqual({ ok: true });
468+
expect(capturedSignal?.aborted).toBe(false);
469+
await new Promise(resolve => setTimeout(resolve, 50));
470+
expect(capturedSignal?.aborted).toBe(false);
471+
});
472+
473+
it('clears a failed attempt timeout before retry backoff sleeps', async () => {
474+
const attemptSignals: AbortSignal[] = [];
475+
let calls = 0;
476+
const fetchImpl = vi.fn(async (_input: unknown, init?: RequestInit) => {
477+
if (init?.signal) attemptSignals.push(init.signal);
478+
calls += 1;
479+
if (calls === 1) return errorEnvelopeResponse(500, 'INTERNAL');
480+
return jsonResponse({ ok: true });
481+
});
482+
const client = new HttpClient({
483+
baseUrl: 'https://api.example.com/api/cli/v1',
484+
apiKey: 'sk-test',
485+
fetchImpl: fetchImpl as unknown as typeof fetch,
486+
sleep: () => new Promise(resolve => setTimeout(resolve, 50)),
487+
random: () => 0,
488+
requestTimeoutMs: 25,
489+
});
490+
491+
await expect(client.get('/me')).resolves.toEqual({ ok: true });
492+
expect(attemptSignals).toHaveLength(2);
493+
expect(attemptSignals.every(signal => signal.aborted === false)).toBe(true);
494+
await new Promise(resolve => setTimeout(resolve, 50));
495+
expect(attemptSignals.every(signal => signal.aborted === false)).toBe(true);
496+
});
497+
452498
it('caller-supplied AbortSignal still propagates as AbortError (not RequestTimeoutError)', async () => {
453499
const controller = new AbortController();
454500
// Abort immediately
@@ -475,6 +521,33 @@ describe('HttpClient per-request timeout', () => {
475521
expect((err as Error).name).toBe('AbortError');
476522
});
477523

524+
it('preserves RequestTimeoutError when the caller aborts after the request timeout wins', async () => {
525+
const controller = new AbortController();
526+
const fetchImpl = vi.fn(async (_input: unknown, init?: { signal?: AbortSignal }) => {
527+
return new Promise<Response>((_resolve, reject) => {
528+
init?.signal?.addEventListener('abort', () => {
529+
const reason = init.signal?.reason;
530+
controller.abort(new Error('caller aborted after timeout'));
531+
const err = new Error(reason?.message ?? 'timed out');
532+
err.name = reason?.name ?? 'TimeoutError';
533+
reject(err);
534+
});
535+
});
536+
});
537+
const client = new HttpClient({
538+
baseUrl: 'https://api.example.com/api/cli/v1',
539+
apiKey: 'sk-test',
540+
fetchImpl: fetchImpl as unknown as typeof fetch,
541+
sleep: () => Promise.resolve(),
542+
random: () => 0,
543+
requestTimeoutMs: 1,
544+
});
545+
const err = await client.get('/me', { signal: controller.signal }).catch(e => e);
546+
expect(err).toBeInstanceOf(RequestTimeoutError);
547+
expect((err as RequestTimeoutError).exitCode).toBe(7);
548+
expect(fetchImpl).toHaveBeenCalledTimes(1);
549+
});
550+
478551
it('defaults to REQUEST_TIMEOUT_DEFAULT_MS when no requestTimeoutMs is supplied', () => {
479552
// Verify the default is wired without actually waiting 120s.
480553
// We test via the exported constant rather than exercising the stall.

0 commit comments

Comments
 (0)