Skip to content

Commit f17bbdc

Browse files
committed
fix(api): clean up fetch retry abort listeners
1 parent 7b0e5e2 commit f17bbdc

2 files changed

Lines changed: 67 additions & 6 deletions

File tree

lib/github.test.ts

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest';
22
import {
33
fetchGitHubContributions,
4+
fetchWithRetry,
45
fetchUserProfile,
56
fetchUserRepos,
67
getFullDashboardData,
@@ -60,6 +61,54 @@ afterEach(() => {
6061
}
6162
});
6263

64+
describe('fetchWithRetry', () => {
65+
beforeEach(() => {
66+
vi.spyOn(global, 'fetch');
67+
});
68+
69+
afterEach(() => {
70+
vi.restoreAllMocks();
71+
});
72+
73+
it('removes caller abort listeners after a successful request', async () => {
74+
const controller = new AbortController();
75+
const addListenerSpy = vi.spyOn(controller.signal, 'addEventListener');
76+
const removeListenerSpy = vi.spyOn(controller.signal, 'removeEventListener');
77+
78+
vi.mocked(fetch).mockResolvedValue(mockResponse({ ok: true }));
79+
80+
await fetchWithRetry('https://api.github.com/test', { signal: controller.signal }, 0, 1000);
81+
82+
const abortListener = addListenerSpy.mock.calls.find(([event]) => event === 'abort')?.[1];
83+
84+
expect(abortListener).toEqual(expect.any(Function));
85+
expect(addListenerSpy).toHaveBeenCalledWith('abort', abortListener, { once: true });
86+
expect(removeListenerSpy).toHaveBeenCalledWith('abort', abortListener);
87+
});
88+
89+
it('still aborts the in-flight request when the caller signal is aborted', async () => {
90+
const controller = new AbortController();
91+
92+
vi.mocked(fetch).mockImplementation(
93+
(_url: RequestInfo | URL, options?: RequestInit) =>
94+
new Promise<Response>((_resolve, reject) => {
95+
options?.signal?.addEventListener(
96+
'abort',
97+
() => reject(new DOMException('Aborted', 'AbortError')),
98+
{ once: true }
99+
);
100+
})
101+
);
102+
103+
const request = fetchWithRetry('https://api.github.com/test', { signal: controller.signal });
104+
105+
controller.abort();
106+
107+
await expect(request).rejects.toThrow('Aborted');
108+
expect(fetch).toHaveBeenCalledOnce();
109+
});
110+
});
111+
63112
describe('fetchGitHubContributions', () => {
64113
beforeEach(() => {
65114
vi.spyOn(global, 'fetch');

lib/github.ts

Lines changed: 18 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -30,27 +30,39 @@ export async function fetchWithRetry(
3030

3131
const controller = new AbortController();
3232
const timeoutId = setTimeout(() => controller.abort(), resolvedTimeout);
33+
const abortRequest = () => controller.abort();
3334

3435
if (options.signal) {
35-
options.signal.addEventListener('abort', () => controller.abort(), { once: true });
36+
options.signal.addEventListener('abort', abortRequest, { once: true });
3637
}
3738

3839
let res: Response | null = null;
40+
let fetchError: unknown;
41+
let didThrow = false;
42+
3943
try {
4044
res = await fetch(url, { ...options, signal: controller.signal });
41-
} catch (err) {
45+
} catch (err: unknown) {
46+
fetchError = err;
47+
didThrow = true;
48+
} finally {
4249
clearTimeout(timeoutId);
43-
if (options.signal?.aborted) throw err;
44-
if (err instanceof Error && err.name === 'AbortError') {
50+
options.signal?.removeEventListener('abort', abortRequest);
51+
}
52+
53+
if (didThrow) {
54+
if (options.signal?.aborted) throw fetchError;
55+
if (fetchError instanceof Error && fetchError.name === 'AbortError') {
4556
throw new Error(`GitHub API request timed out after ${resolvedTimeout / 1000}s`);
4657
}
47-
if (attempt >= MAX_RETRIES) throw err;
58+
if (attempt >= MAX_RETRIES) throw fetchError;
4859
const delay = BASE_DELAY_MS * Math.pow(2, attempt);
4960
await new Promise((resolve) => setTimeout(resolve, delay));
5061
return fetchWithRetry(url, options, attempt + 1, timeoutMs);
5162
}
5263

53-
clearTimeout(timeoutId);
64+
if (!res) throw new Error('GitHub API request failed without a response');
65+
5466
const shouldRetry = res.status === 429 || res.status >= 500;
5567
if (!shouldRetry || attempt >= MAX_RETRIES) return res;
5668

0 commit comments

Comments
 (0)