Skip to content

Commit 9f862a5

Browse files
committed
fix: abort in-flight mutations in UpdatesDashboard on unmount
handleAction fires prepare/execute/automated/delete against the gateway without an abort signal. If the user navigates away while a mutation is in-flight, the promise resolves and fires toast.success + setBusyIds on an unmounted component. Add an AbortController created in a mount effect and aborted in its cleanup, thread its signal through the four mutation helpers, and short-circuit post-await side effects when the signal is aborted. The polling path in useUpdatesPolling already had this treatment; this brings mutations to parity. Tests assert (a) signal is an AbortSignal, (b) it becomes aborted on unmount, (c) a late-resolving mutation does not fire a success toast.
1 parent 0843e07 commit 9f862a5

2 files changed

Lines changed: 75 additions & 10 deletions

File tree

src/components/UpdatesDashboard.test.tsx

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -199,4 +199,54 @@ describe('UpdatesDashboard', () => {
199199

200200
expect(toast.error).toHaveBeenCalled();
201201
});
202+
203+
it('passes an AbortSignal to mutation actions', async () => {
204+
const user = userEvent.setup();
205+
mockTriggerPrepare.mockResolvedValue(undefined);
206+
mockUseUpdatesPolling.mockReturnValue(makeResult({ updates: [makeEntry('fw-v2', 'pending')] }));
207+
208+
render(<UpdatesDashboard />);
209+
210+
const prepareBtn = screen.getByRole('button', { name: /prepare/i });
211+
await user.click(prepareBtn);
212+
213+
// triggerPrepare is called with (baseUrl, id, data, signal) - assert signal is an AbortSignal
214+
const call = mockTriggerPrepare.mock.calls[0]!;
215+
const signal = call[3];
216+
expect(signal).toBeInstanceOf(AbortSignal);
217+
});
218+
219+
it('aborts in-flight mutations on unmount', async () => {
220+
const user = userEvent.setup();
221+
let resolvePrepare: () => void = () => {};
222+
mockTriggerPrepare.mockImplementation(
223+
() =>
224+
new Promise<void>((resolve) => {
225+
resolvePrepare = resolve;
226+
})
227+
);
228+
mockUseUpdatesPolling.mockReturnValue(makeResult({ updates: [makeEntry('fw-v2', 'pending')] }));
229+
const { toast } = await import('react-toastify');
230+
vi.mocked(toast.success).mockClear();
231+
232+
const { unmount } = render(<UpdatesDashboard />);
233+
234+
const prepareBtn = screen.getByRole('button', { name: /prepare/i });
235+
await user.click(prepareBtn);
236+
237+
// Capture the signal before unmount
238+
const signal = mockTriggerPrepare.mock.calls[0]![3] as AbortSignal;
239+
expect(signal.aborted).toBe(false);
240+
241+
unmount();
242+
243+
expect(signal.aborted).toBe(true);
244+
245+
// Late-resolve the prepare call - must NOT fire a success toast on
246+
// the unmounted component.
247+
resolvePrepare();
248+
await new Promise((r) => setTimeout(r, 0));
249+
250+
expect(toast.success).not.toHaveBeenCalled();
251+
});
202252
});

src/components/UpdatesDashboard.tsx

Lines changed: 25 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@
1212
// See the License for the specific language governing permissions and
1313
// limitations under the License.
1414

15-
import { useCallback, useMemo, useState } from 'react';
15+
import { useCallback, useEffect, useMemo, useRef, useState } from 'react';
1616
import { useShallow } from 'zustand/shallow';
1717
import { Package, RefreshCw, AlertTriangle, Server } from 'lucide-react';
1818
import { toast } from 'react-toastify';
@@ -39,6 +39,16 @@ export function UpdatesDashboard() {
3939
const { updates, isLoading, error, notAvailable, refresh } = useUpdatesPolling(baseUrl);
4040
const [busyIds, setBusyIds] = useState<Set<string>>(new Set());
4141

42+
// AbortController for mutation actions (prepare/execute/automated/delete).
43+
// Aborted on unmount so in-flight requests don't resolve into setBusyIds
44+
// on an unmounted component or issue stale toast notifications.
45+
const actionAbortRef = useRef<AbortController | null>(null);
46+
useEffect(() => {
47+
const controller = new AbortController();
48+
actionAbortRef.current = controller;
49+
return () => controller.abort();
50+
}, []);
51+
4252
const summary = useMemo(() => {
4353
let active = 0;
4454
let failed = 0;
@@ -59,22 +69,27 @@ export function UpdatesDashboard() {
5969
const confirmed = window.confirm(`Delete update "${id}"? This cannot be undone.`);
6070
if (!confirmed) return;
6171
}
72+
const signal = actionAbortRef.current?.signal;
6273
setBusyIds((prev) => new Set(prev).add(id));
6374
try {
64-
if (action === 'prepare') await triggerPrepare(baseUrl, id);
65-
else if (action === 'execute') await triggerExecute(baseUrl, id);
66-
else if (action === 'automated') await triggerAutomated(baseUrl, id);
67-
else if (action === 'delete') await deleteUpdate(baseUrl, id);
75+
if (action === 'prepare') await triggerPrepare(baseUrl, id, undefined, signal);
76+
else if (action === 'execute') await triggerExecute(baseUrl, id, undefined, signal);
77+
else if (action === 'automated') await triggerAutomated(baseUrl, id, undefined, signal);
78+
else if (action === 'delete') await deleteUpdate(baseUrl, id, signal);
79+
if (signal?.aborted) return;
6880
toast.success(`${action} triggered for ${id}`);
6981
refresh();
7082
} catch (err) {
83+
if (signal?.aborted || (err as { name?: string })?.name === 'AbortError') return;
7184
toast.error(err instanceof Error ? err.message : String(err));
7285
} finally {
73-
setBusyIds((prev) => {
74-
const next = new Set(prev);
75-
next.delete(id);
76-
return next;
77-
});
86+
if (!signal?.aborted) {
87+
setBusyIds((prev) => {
88+
const next = new Set(prev);
89+
next.delete(id);
90+
return next;
91+
});
92+
}
7893
}
7994
},
8095
[baseUrl, refresh]

0 commit comments

Comments
 (0)