diff --git a/Docs/Tasks.md b/Docs/Tasks.md index 4c51bed..c765196 100644 --- a/Docs/Tasks.md +++ b/Docs/Tasks.md @@ -1,35 +1,3 @@ -### TASK-ABORT-02 — Hooks Create `AbortController` but Never Forward Signal to API - -**Where found** -Several hooks correctly create a controller and check `signal.aborted` in callbacks, but then call the API function without passing the signal — so the fetch is never actually cancelled: - -| Hook | API call missing signal | -|------|------------------------| -| `frontend/src/hooks/useConfigActiveStatus.ts` | `fetchJails()`, `fetchJailConfigs()` | -| `frontend/src/hooks/useMapData.ts` | `fetchBansByCountry(range, origin, source, countryCode)` | -| `frontend/src/hooks/useDashboardCountryData.ts` | `fetchBansByCountry(timeRange, origin, source)` | -| `frontend/src/hooks/useImportLog.ts` | `fetchImportLog(page, pageSize, sourceId)` | -| `frontend/src/hooks/useJailAdmin.ts` | `fetchInactiveJails()` | - -**Goal** -After completing TASK-ABORT-01, update each hook listed above to pass `controller.signal` (or `ctrl.signal`) to the API call. For example in `useMapData`: -```ts -fetchBansByCountry(range, origin, source, countryCode, controller.signal) -``` - -**Possible traps and issues** -- This task is a dependency of TASK-ABORT-01: the API functions must accept `signal` first. -- `useConfigActiveStatus` makes two sequential fetches; both need the same signal forwarded. -- After adding the signal parameter, ensure the abort check in `.then()` / `.catch()` callbacks uses `controller.signal.aborted` (the local capture), not `abortRef.current?.signal.aborted` (the mutable ref). - -**Docs changes needed** -None beyond TASK-ABORT-01. - -**Why this is needed** -The abort guard (`if (controller.signal.aborted) return;`) is present but pointless when the HTTP request itself was never aborted. The network request still completes and consumes server resources; only the state update is skipped. - ---- - ### TASK-ABORT-03 — Stale `abortRef` Read in `.finally()` in Three Hooks **Where found** diff --git a/Docs/Web-Development.md b/Docs/Web-Development.md index 833a167..b47c244 100644 --- a/Docs/Web-Development.md +++ b/Docs/Web-Development.md @@ -373,6 +373,42 @@ function useBans(hours: number): UseBansResult { export default useBans; ``` +### AbortController in Hooks + +When using `AbortController` for fetch cancellation in hooks with mutable refs: + +- **Always** capture the controller in a local `const` variable before the async operation. +- Use that **local variable** in all callbacks (`.then()`, `.catch()`, `.finally()`), never read `abortRef.current` from inside an async callback. +- This prevents race conditions: if `load()` is called while a fetch is in flight, the previous fetch's callbacks will use the old, locally-captured controller reference, not the newly-assigned one. + +Incorrect (reads `abortRef.current` in callback — this is racy): +```ts +const load = useCallback(() => { + const ctrl = new AbortController(); + abortRef.current = ctrl; + fetchData() + .finally(() => { + if (!abortRef.current?.signal.aborted) { // ❌ Wrong: reads mutable ref + setLoading(false); + } + }); +}, []); +``` + +Correct (uses local `ctrl` in all callbacks): +```ts +const load = useCallback(() => { + const ctrl = new AbortController(); + abortRef.current = ctrl; + fetchData() + .finally(() => { + if (!ctrl.signal.aborted) { // ✅ Correct: uses locally-captured variable + setLoading(false); + } + }); +}, []); +``` + --- ## 8. Naming Conventions diff --git a/frontend/src/hooks/useGlobalConfig.ts b/frontend/src/hooks/useGlobalConfig.ts index 2648ad6..95e9e84 100644 --- a/frontend/src/hooks/useGlobalConfig.ts +++ b/frontend/src/hooks/useGlobalConfig.ts @@ -43,7 +43,7 @@ export function useGlobalConfig(): UseGlobalConfigResult { } }) .finally(() => { - if (!abortRef.current?.signal.aborted) { + if (!ctrl.signal.aborted) { setLoading(false); } }); diff --git a/frontend/src/hooks/useJailConfigDetail.ts b/frontend/src/hooks/useJailConfigDetail.ts index 6f3c26e..c47de36 100644 --- a/frontend/src/hooks/useJailConfigDetail.ts +++ b/frontend/src/hooks/useJailConfigDetail.ts @@ -44,7 +44,7 @@ export function useJailConfigDetail(name: string): UseJailConfigDetailResult { } }) .finally(() => { - if (!abortRef.current?.signal.aborted) { + if (!ctrl.signal.aborted) { setLoading(false); } }); diff --git a/frontend/src/hooks/useServerSettings.ts b/frontend/src/hooks/useServerSettings.ts index 891f713..2b43c07 100644 --- a/frontend/src/hooks/useServerSettings.ts +++ b/frontend/src/hooks/useServerSettings.ts @@ -47,7 +47,7 @@ export function useServerSettings(): UseServerSettingsResult { } }) .finally(() => { - if (!abortRef.current?.signal.aborted) { + if (!ctrl.signal.aborted) { setLoading(false); } });