Skip to content

Commit 96d34db

Browse files
Merge pull request #351 from bg-playground/copilot/bgstm-350-update-useeffectasync
Adopt AbortSignal-based cancellation in `useEffectAsync` and migrate frontend data loaders
2 parents f216d40 + f40dc1d commit 96d34db

21 files changed

Lines changed: 224 additions & 132 deletions

frontend/src/api/auditLog.ts

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import { apiClient } from './client';
2+
import type { AxiosRequestConfig } from 'axios';
23

34
export interface AuditLogEntry {
45
id: string;
@@ -26,7 +27,7 @@ export interface AuditLogFilters {
2627
}
2728

2829
export const auditLogApi = {
29-
list: (filters: AuditLogFilters = {}) => {
30+
list: (filters: AuditLogFilters = {}, config?: AxiosRequestConfig) => {
3031
const params = new URLSearchParams();
3132
if (filters.user_id) params.set('user_id', filters.user_id);
3233
if (filters.action) params.set('action', filters.action);
@@ -35,6 +36,6 @@ export const auditLogApi = {
3536
if (filters.date_to) params.set('date_to', filters.date_to);
3637
params.set('skip', String(filters.skip ?? 0));
3738
params.set('limit', String(filters.limit ?? 25));
38-
return apiClient.get<AuditLogListResponse>(`/audit-log?${params.toString()}`);
39+
return apiClient.get<AuditLogListResponse>(`/audit-log?${params.toString()}`, config);
3940
},
4041
};

frontend/src/api/client.ts

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,9 @@ apiClient.interceptors.request.use((config) => {
2525
apiClient.interceptors.response.use(
2626
(response) => response,
2727
(error) => {
28+
if (axios.isCancel(error)) {
29+
return Promise.reject(error);
30+
}
2831
console.error('API Error:', error.response?.data || error.message);
2932
if (error.response?.status === 401) {
3033
localStorage.removeItem(TOKEN_STORAGE_KEY);
@@ -33,3 +36,7 @@ apiClient.interceptors.response.use(
3336
return Promise.reject(error);
3437
}
3538
);
39+
40+
export function isRequestCanceled(error: unknown): boolean {
41+
return axios.isCancel(error) || (error instanceof DOMException && error.name === 'AbortError');
42+
}

frontend/src/api/externalResults.ts

Lines changed: 17 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import { apiClient } from './client';
2+
import type { AxiosRequestConfig } from 'axios';
23

34
export type RunStatus = 'started' | 'passed' | 'failed' | 'skipped' | 'aborted';
45
export type CaseOutcome = 'passed' | 'failed' | 'skipped' | 'flaky';
@@ -53,18 +54,29 @@ export interface CaseResultListResponse {
5354
}
5455

5556
export const externalResultsApi = {
56-
listSessions: (params: { skip?: number; limit?: number; status?: RunStatus } = {}) => {
57+
listSessions: (
58+
params: { skip?: number; limit?: number; status?: RunStatus } = {},
59+
config?: AxiosRequestConfig
60+
) => {
5761
const p = new URLSearchParams();
5862
if (params.skip !== undefined) p.set('skip', String(params.skip));
5963
if (params.limit !== undefined) p.set('limit', String(params.limit));
6064
if (params.status) p.set('status', params.status);
61-
return apiClient.get<SessionListResponse>(`/external-results/sessions?${p.toString()}`);
65+
return apiClient.get<SessionListResponse>(`/external-results/sessions?${p.toString()}`, config);
6266
},
63-
getSession: (sessionId: string) => apiClient.get<TestSession>(`/external-results/session/${sessionId}`),
64-
listSessionCases: (sessionId: string, params: { skip?: number; limit?: number } = {}) => {
67+
getSession: (sessionId: string, config?: AxiosRequestConfig) =>
68+
apiClient.get<TestSession>(`/external-results/session/${sessionId}`, config),
69+
listSessionCases: (
70+
sessionId: string,
71+
params: { skip?: number; limit?: number } = {},
72+
config?: AxiosRequestConfig
73+
) => {
6574
const p = new URLSearchParams();
6675
if (params.skip !== undefined) p.set('skip', String(params.skip));
6776
if (params.limit !== undefined) p.set('limit', String(params.limit));
68-
return apiClient.get<CaseResultListResponse>(`/external-results/session/${sessionId}/cases?${p.toString()}`);
77+
return apiClient.get<CaseResultListResponse>(
78+
`/external-results/session/${sessionId}/cases?${p.toString()}`,
79+
config
80+
);
6981
},
7082
};

frontend/src/api/links.ts

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,16 @@
11
import { apiClient } from './client';
2+
import type { AxiosRequestConfig } from 'axios';
23
import type { Link, LinkCreate, PaginatedResponse } from '../types/api';
34

45
export const linksApi = {
5-
list: async (page = 1, pageSize = 50): Promise<PaginatedResponse<Link>> => {
6+
list: async (
7+
page = 1,
8+
pageSize = 50,
9+
config?: AxiosRequestConfig
10+
): Promise<PaginatedResponse<Link>> => {
611
const response = await apiClient.get<PaginatedResponse<Link>>(
7-
`/links?page=${page}&page_size=${pageSize}`
12+
`/links?page=${page}&page_size=${pageSize}`,
13+
config
814
);
915
return response.data;
1016
},

frontend/src/api/requirements.ts

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,16 @@
11
import { apiClient } from './client';
2+
import type { AxiosRequestConfig } from 'axios';
23
import type { Requirement, RequirementCreate, RequirementUpdate, PaginatedResponse } from '../types/api';
34

45
export const requirementsApi = {
5-
list: async (page = 1, pageSize = 50): Promise<PaginatedResponse<Requirement>> => {
6+
list: async (
7+
page = 1,
8+
pageSize = 50,
9+
config?: AxiosRequestConfig
10+
): Promise<PaginatedResponse<Requirement>> => {
611
const response = await apiClient.get<PaginatedResponse<Requirement>>(
7-
`/requirements?page=${page}&page_size=${pageSize}`
12+
`/requirements?page=${page}&page_size=${pageSize}`,
13+
config
814
);
915
return response.data;
1016
},

frontend/src/api/suggestions.ts

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import { apiClient } from './client';
2+
import type { AxiosRequestConfig } from 'axios';
23
import type { Suggestion, SuggestionReview, GenerateSuggestionsResponse, SuggestionStatus, PaginatedResponse } from '../types/api';
34

45
export const suggestionsApi = {
@@ -18,7 +19,7 @@ export const suggestionsApi = {
1819
search?: string;
1920
page?: number;
2021
pageSize?: number;
21-
}): Promise<PaginatedResponse<Suggestion>> => {
22+
}, config?: AxiosRequestConfig): Promise<PaginatedResponse<Suggestion>> => {
2223
const searchParams = new URLSearchParams();
2324
if (params?.minScore !== undefined) searchParams.append('min_score', params.minScore.toString());
2425
if (params?.maxScore !== undefined) searchParams.append('max_score', params.maxScore.toString());
@@ -29,7 +30,10 @@ export const suggestionsApi = {
2930
searchParams.append('page', (params?.page ?? 1).toString());
3031
searchParams.append('page_size', (params?.pageSize ?? 50).toString());
3132

32-
const response = await apiClient.get<PaginatedResponse<Suggestion>>(`/suggestions/pending?${searchParams}`);
33+
const response = await apiClient.get<PaginatedResponse<Suggestion>>(
34+
`/suggestions/pending?${searchParams}`,
35+
config
36+
);
3337
return response.data;
3438
},
3539

frontend/src/api/testCases.ts

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,16 @@
11
import { apiClient } from './client';
2+
import type { AxiosRequestConfig } from 'axios';
23
import type { TestCase, TestCaseCreate, TestCaseUpdate, PaginatedResponse } from '../types/api';
34

45
export const testCasesApi = {
5-
list: async (page = 1, pageSize = 50): Promise<PaginatedResponse<TestCase>> => {
6+
list: async (
7+
page = 1,
8+
pageSize = 50,
9+
config?: AxiosRequestConfig
10+
): Promise<PaginatedResponse<TestCase>> => {
611
const response = await apiClient.get<PaginatedResponse<TestCase>>(
7-
`/test-cases?page=${page}&page_size=${pageSize}`
12+
`/test-cases?page=${page}&page_size=${pageSize}`,
13+
config
814
);
915
return response.data;
1016
},

frontend/src/api/traceability.ts

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import { apiClient } from "./client";
2+
import type { AxiosRequestConfig } from "axios";
23

34
export interface LinkedTestCase {
45
test_case_id: string;
@@ -59,13 +60,13 @@ export interface Metrics {
5960
}
6061

6162
const traceabilityApi = {
62-
async getMatrix(): Promise<TraceabilityMatrix> {
63-
const response = await apiClient.get<TraceabilityMatrix>("/traceability-matrix");
63+
async getMatrix(config?: AxiosRequestConfig): Promise<TraceabilityMatrix> {
64+
const response = await apiClient.get<TraceabilityMatrix>("/traceability-matrix", config);
6465
return response.data;
6566
},
6667

67-
async getMetrics(): Promise<Metrics> {
68-
const response = await apiClient.get<Metrics>("/metrics");
68+
async getMetrics(config?: AxiosRequestConfig): Promise<Metrics> {
69+
const response = await apiClient.get<Metrics>("/metrics", config);
6970
return response.data;
7071
},
7172

frontend/src/api/users.ts

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import { apiClient } from './client';
2+
import type { AxiosRequestConfig } from 'axios';
23

34
export interface ManagedUser {
45
id: string;
@@ -22,8 +23,8 @@ export interface UserUpdate {
2223
}
2324

2425
export const usersApi = {
25-
list: (skip = 0, limit = 100) =>
26-
apiClient.get<UserListResponse>(`/users?skip=${skip}&limit=${limit}`),
26+
list: (skip = 0, limit = 100, config?: AxiosRequestConfig) =>
27+
apiClient.get<UserListResponse>(`/users?skip=${skip}&limit=${limit}`, config),
2728

2829
update: (id: string, updates: UserUpdate) =>
2930
apiClient.patch<ManagedUser>(`/users/${id}`, updates),
Lines changed: 41 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -1,83 +1,79 @@
1-
import { act, renderHook } from '@testing-library/react';
1+
import { renderHook } from '@testing-library/react';
22
import { useEffectAsync } from './useEffectAsync';
33

44
describe('useEffectAsync', () => {
55
it('invokes the async callback on mount', async () => {
6-
const callback = vi.fn().mockResolvedValue(undefined);
6+
const callback = vi.fn().mockImplementation(async (signal: AbortSignal) => {
7+
expect(signal).toBeInstanceOf(AbortSignal);
8+
});
79

810
renderHook(() => useEffectAsync(callback, []));
911

1012
await vi.waitFor(() => {
1113
expect(callback).toHaveBeenCalledTimes(1);
1214
});
13-
});
1415

15-
it('does not apply callback side-effects after unmount (cancellation)', async () => {
16-
let sideEffectApplied = false;
17-
let resolveGate!: () => void;
16+
const [signal] = callback.mock.calls[0] as [AbortSignal];
17+
expect(signal.aborted).toBe(false);
18+
});
1819

19-
// Callback with a manual gate — won't complete until we call resolveGate()
20-
const callback = vi.fn().mockImplementation(async () => {
21-
await new Promise<void>((resolve) => {
22-
resolveGate = resolve;
23-
});
24-
sideEffectApplied = true;
20+
it('aborts the callback signal on unmount cleanup', async () => {
21+
let capturedSignal: AbortSignal | undefined;
22+
const callback = vi.fn().mockImplementation(async (signal: AbortSignal) => {
23+
capturedSignal = signal;
2524
});
26-
27-
// render — effect fires (inside act), callback is called and suspended at the gate
2825
const { unmount } = renderHook(() => useEffectAsync(callback, []));
2926

30-
// Callback has been called but is suspended; side-effect not yet applied.
31-
expect(callback).toHaveBeenCalledTimes(1);
32-
expect(sideEffectApplied).toBe(false);
33-
34-
// Unmount while the callback is in-flight — cleanup sets cancelled = true.
35-
unmount();
36-
37-
// Open the gate so the callback can complete.
38-
await act(async () => {
39-
resolveGate();
40-
await Promise.resolve();
27+
await vi.waitFor(() => {
28+
expect(callback).toHaveBeenCalledTimes(1);
4129
});
4230

43-
// The hook's cancelled flag is checked BEFORE calling the callback,
44-
// not after it completes, so the in-flight side-effect does still apply.
45-
// What the hook guarantees is that no *additional* invocation of the
46-
// callback happens after cleanup — which we confirm here.
47-
expect(callback).toHaveBeenCalledTimes(1);
48-
expect(sideEffectApplied).toBe(true);
31+
unmount();
32+
expect(capturedSignal?.aborted).toBe(true);
4933
});
5034

5135
it('cleans up (cancels) the previous effect and re-invokes callback when deps change', async () => {
52-
const callOrder: string[] = [];
53-
54-
const makeCallback = (label: string) =>
55-
vi.fn().mockImplementation(async () => {
56-
callOrder.push(label);
57-
});
58-
59-
const callback1 = makeCallback('first');
60-
const callback2 = makeCallback('second');
36+
const signals: AbortSignal[] = [];
37+
const callback1 = vi.fn().mockImplementation(async (signal: AbortSignal) => {
38+
signals.push(signal);
39+
});
40+
const callback2 = vi.fn().mockImplementation(async (signal: AbortSignal) => {
41+
signals.push(signal);
42+
});
6143

6244
let cb = callback1;
6345
const { rerender } = renderHook(() => useEffectAsync(cb, [cb]));
6446

65-
// First callback runs on mount.
6647
await vi.waitFor(() => {
6748
expect(callback1).toHaveBeenCalledTimes(1);
6849
});
6950

70-
// Change the dependency — triggers cleanup of the old effect and a new effect.
7151
cb = callback2;
7252
rerender();
7353

7454
await vi.waitFor(() => {
7555
expect(callback2).toHaveBeenCalledTimes(1);
7656
});
7757

78-
// Each callback ran exactly once, in the correct order.
79-
expect(callOrder).toEqual(['first', 'second']);
80-
// Old callback was not invoked again after cleanup.
58+
expect(signals[0].aborted).toBe(true);
59+
expect(signals[1].aborted).toBe(false);
8160
expect(callback1).toHaveBeenCalledTimes(1);
8261
});
62+
63+
it('calls AbortController.abort during cleanup', async () => {
64+
const abortSpy = vi.spyOn(AbortController.prototype, 'abort');
65+
const callback = vi.fn().mockImplementation(async (signal: AbortSignal) => {
66+
expect(signal).toBeInstanceOf(AbortSignal);
67+
});
68+
const { unmount } = renderHook(() => useEffectAsync(callback, []));
69+
70+
await vi.waitFor(() => {
71+
expect(callback).toHaveBeenCalledTimes(1);
72+
});
73+
74+
unmount();
75+
expect(abortSpy).toHaveBeenCalledTimes(1);
76+
77+
abortSpy.mockRestore();
78+
});
8379
});

0 commit comments

Comments
 (0)