refactoring-backend #3
@@ -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**
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -43,7 +43,7 @@ export function useGlobalConfig(): UseGlobalConfigResult {
|
||||
}
|
||||
})
|
||||
.finally(() => {
|
||||
if (!abortRef.current?.signal.aborted) {
|
||||
if (!ctrl.signal.aborted) {
|
||||
setLoading(false);
|
||||
}
|
||||
});
|
||||
|
||||
@@ -44,7 +44,7 @@ export function useJailConfigDetail(name: string): UseJailConfigDetailResult {
|
||||
}
|
||||
})
|
||||
.finally(() => {
|
||||
if (!abortRef.current?.signal.aborted) {
|
||||
if (!ctrl.signal.aborted) {
|
||||
setLoading(false);
|
||||
}
|
||||
});
|
||||
|
||||
@@ -47,7 +47,7 @@ export function useServerSettings(): UseServerSettingsResult {
|
||||
}
|
||||
})
|
||||
.finally(() => {
|
||||
if (!abortRef.current?.signal.aborted) {
|
||||
if (!ctrl.signal.aborted) {
|
||||
setLoading(false);
|
||||
}
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user