diff --git a/Docs/Tasks.md b/Docs/Tasks.md index 662465d..29d1926 100644 --- a/Docs/Tasks.md +++ b/Docs/Tasks.md @@ -268,6 +268,8 @@ Reference: `Docs/Refactoring.md` for full analysis of each issue. **Docs changes needed:** None. +**Status:** Completed. + **Why this is needed:** Without abort cleanup, a component that unmounts mid-fetch will still call `setState` on the unmounted component, producing React "can't perform state update on unmounted component" warnings and, more critically, stale responses from a fast-then-slow request sequence can silently overwrite fresher data. This is a reliability issue in any view that mounts and unmounts frequently (dialogs, paginated tabs). --- diff --git a/frontend/src/api/blocklist.ts b/frontend/src/api/blocklist.ts index 0e9f0cd..2e71b24 100644 --- a/frontend/src/api/blocklist.ts +++ b/frontend/src/api/blocklist.ts @@ -68,8 +68,8 @@ export async function runImportNow(): Promise { // --------------------------------------------------------------------------- /** Fetch the current schedule config and next/last run times. */ -export async function fetchSchedule(): Promise { - return get(ENDPOINTS.blocklistsSchedule); +export async function fetchSchedule(signal?: AbortSignal): Promise { + return get(ENDPOINTS.blocklistsSchedule, signal); } /** Update the import schedule. */ diff --git a/frontend/src/api/client.ts b/frontend/src/api/client.ts index 2030ca7..8e7a49d 100644 --- a/frontend/src/api/client.ts +++ b/frontend/src/api/client.ts @@ -84,8 +84,8 @@ async function request(url: string, options: RequestInit = {}): Promise { * @param path - API path relative to `BASE_URL`, e.g. `"/jails"`. * @returns Parsed response body typed as `T`. */ -export async function get(path: string): Promise { - return request(`${BASE_URL}${path}`); +export async function get(path: string, signal?: AbortSignal): Promise { + return request(`${BASE_URL}${path}`, { signal }); } /** @@ -93,12 +93,14 @@ export async function get(path: string): Promise { * * @param path - API path relative to `BASE_URL`. * @param body - Request payload to serialise as JSON. + * @param signal - Optional abort signal for request cancellation. * @returns Parsed response body typed as `T`. */ -export async function post(path: string, body: unknown): Promise { +export async function post(path: string, body: unknown, signal?: AbortSignal): Promise { return request(`${BASE_URL}${path}`, { method: "POST", body: JSON.stringify(body), + signal, }); } @@ -107,12 +109,14 @@ export async function post(path: string, body: unknown): Promise { * * @param path - API path relative to `BASE_URL`. * @param body - Request payload to serialise as JSON. + * @param signal - Optional abort signal for request cancellation. * @returns Parsed response body typed as `T`. */ -export async function put(path: string, body: unknown): Promise { +export async function put(path: string, body: unknown, signal?: AbortSignal): Promise { return request(`${BASE_URL}${path}`, { method: "PUT", body: JSON.stringify(body), + signal, }); } @@ -121,12 +125,14 @@ export async function put(path: string, body: unknown): Promise { * * @param path - API path relative to `BASE_URL`. * @param body - Optional request payload. + * @param signal - Optional abort signal for request cancellation. * @returns Parsed response body typed as `T`. */ -export async function del(path: string, body?: unknown): Promise { +export async function del(path: string, body?: unknown, signal?: AbortSignal): Promise { return request(`${BASE_URL}${path}`, { method: "DELETE", body: body !== undefined ? JSON.stringify(body) : undefined, + signal, }); } diff --git a/frontend/src/api/config.ts b/frontend/src/api/config.ts index 2845da0..58b9433 100644 --- a/frontend/src/api/config.ts +++ b/frontend/src/api/config.ts @@ -142,8 +142,9 @@ export async function previewLog( // --------------------------------------------------------------------------- export async function fetchMapColorThresholds( +signal?: AbortSignal, ): Promise { - return get(ENDPOINTS.configMapColorThresholds); + return get(ENDPOINTS.configMapColorThresholds, signal); } export async function updateMapColorThresholds( @@ -257,8 +258,8 @@ export async function createActionFile( // Parsed filter config (Task 2.2 / legacy /parsed endpoint) // --------------------------------------------------------------------------- -export async function fetchParsedFilter(name: string): Promise { - return get(ENDPOINTS.configFilterParsed(name)); +export async function fetchParsedFilter(name: string, signal?: AbortSignal): Promise { + return get(ENDPOINTS.configFilterParsed(name), signal); } export async function updateParsedFilter( @@ -394,8 +395,8 @@ export async function fetchActions(): Promise { * @param name - Action base name (e.g. "iptables" or "iptables.conf"). * @returns ActionConfig with active, used_by_jails, source_file populated. */ -export async function fetchAction(name: string): Promise { - return get(ENDPOINTS.configAction(name)); +export async function fetchAction(name: string, signal?: AbortSignal): Promise { + return get(ENDPOINTS.configAction(name), signal); } /** @@ -478,8 +479,8 @@ export async function removeActionFromJail( // Parsed jail file config (Task 6.1 / 6.2) // --------------------------------------------------------------------------- -export async function fetchParsedJailFile(filename: string): Promise { - return get(ENDPOINTS.configJailFileParsed(filename)); +export async function fetchParsedJailFile(filename: string, signal?: AbortSignal): Promise { + return get(ENDPOINTS.configJailFileParsed(filename), signal); } export async function updateParsedJailFile( diff --git a/frontend/src/api/dashboard.ts b/frontend/src/api/dashboard.ts index 61e429e..080a2b5 100644 --- a/frontend/src/api/dashboard.ts +++ b/frontend/src/api/dashboard.ts @@ -43,6 +43,7 @@ export async function fetchBans( pageSize = 100, origin: BanOriginFilter = "all", source: "fail2ban" | "archive" = "fail2ban", + signal?: AbortSignal, ): Promise { const params = new URLSearchParams({ range, @@ -55,7 +56,7 @@ export async function fetchBans( if (source !== "fail2ban") { params.set("source", source); } - return get(`${ENDPOINTS.dashboardBans}?${params.toString()}`); + return get(`${ENDPOINTS.dashboardBans}?${params.toString()}`, signal); } /** diff --git a/frontend/src/api/setup.ts b/frontend/src/api/setup.ts index 2036390..1070513 100644 --- a/frontend/src/api/setup.ts +++ b/frontend/src/api/setup.ts @@ -40,6 +40,6 @@ export async function submitSetup(data: SetupRequest): Promise { * * @returns The configured timezone identifier (e.g. `"Europe/Berlin"`). */ -export async function fetchTimezone(): Promise { - return api.get(ENDPOINTS.setupTimezone); +export async function fetchTimezone(signal?: AbortSignal): Promise { + return api.get(ENDPOINTS.setupTimezone, signal); } diff --git a/frontend/src/components/blocklist/BlocklistScheduleSection.tsx b/frontend/src/components/blocklist/BlocklistScheduleSection.tsx index 856b7be..d5bc1ae 100644 --- a/frontend/src/components/blocklist/BlocklistScheduleSection.tsx +++ b/frontend/src/components/blocklist/BlocklistScheduleSection.tsx @@ -1,4 +1,4 @@ -import { useCallback, useState } from "react"; +import { useCallback, useEffect, useRef, useState } from "react"; import { Button, Field, Input, MessageBar, MessageBarBody, Select, Spinner, Text } from "@fluentui/react-components"; import { PlayRegular } from "@fluentui/react-icons"; import { useCommonSectionStyles } from "../../theme/commonStyles"; @@ -25,6 +25,7 @@ export function BlocklistScheduleSection({ onRunImport, runImportRunning }: Sche const { info, loading, error, saveSchedule } = useSchedule(); const [saving, setSaving] = useState(false); const [saveMsg, setSaveMsg] = useState(null); + const saveTimeoutRef = useRef | null>(null); const config = info?.config ?? { frequency: "daily" as ScheduleFrequency, @@ -37,12 +38,20 @@ export function BlocklistScheduleSection({ onRunImport, runImportRunning }: Sche const [draft, setDraft] = useState(config); const handleSave = useCallback((): void => { + if (saveTimeoutRef.current) { + clearTimeout(saveTimeoutRef.current); + saveTimeoutRef.current = null; + } + setSaving(true); saveSchedule(draft) .then(() => { setSaveMsg("Schedule saved."); setSaving(false); - setTimeout(() => { setSaveMsg(null); }, 3000); + saveTimeoutRef.current = setTimeout(() => { + setSaveMsg(null); + saveTimeoutRef.current = null; + }, 3000); }) .catch((err: unknown) => { setSaveMsg(err instanceof Error ? err.message : "Failed to save schedule"); @@ -50,6 +59,14 @@ export function BlocklistScheduleSection({ onRunImport, runImportRunning }: Sche }); }, [draft, saveSchedule]); + useEffect(() => { + return (): void => { + if (saveTimeoutRef.current) { + clearTimeout(saveTimeoutRef.current); + } + }; + }, []); + return (
diff --git a/frontend/src/components/config/ActivateJailDialog.tsx b/frontend/src/components/config/ActivateJailDialog.tsx index 3b385d8..d0ad7cb 100644 --- a/frontend/src/components/config/ActivateJailDialog.tsx +++ b/frontend/src/components/config/ActivateJailDialog.tsx @@ -106,12 +106,14 @@ export function ActivateJailDialog({ useEffect(() => { if (!open || !jail) return; + const controller = new AbortController(); setValidating(true); setValidationIssues([]); setValidationWarnings([]); onValidate() .then((result) => { + if (controller.signal.aborted) return; setValidationIssues(result.issues); }) .catch(() => { @@ -119,8 +121,13 @@ export function ActivateJailDialog({ // attempt activation and let the server decide. }) .finally(() => { + if (controller.signal.aborted) return; setValidating(false); }); + + return (): void => { + controller.abort(); + }; }, [open, jail, onValidate]); const handleClose = (): void => { diff --git a/frontend/src/components/config/ConfFilesTab.tsx b/frontend/src/components/config/ConfFilesTab.tsx index 068ad86..269eeac 100644 --- a/frontend/src/components/config/ConfFilesTab.tsx +++ b/frontend/src/components/config/ConfFilesTab.tsx @@ -75,25 +75,33 @@ export function ConfFilesTab({ const [newContent, setNewContent] = useState(""); const [creating, setCreating] = useState(false); - const loadFiles = useCallback(async () => { + const loadFiles = useCallback(async (signal?: AbortSignal) => { setLoading(true); setError(null); try { const resp = await fetchList(); + if (signal?.aborted) return; setFiles(resp.files); } catch (err: unknown) { + if (signal?.aborted) return; setError( err instanceof ApiError ? err.message : `Failed to load ${label.toLowerCase()} files.`, ); } finally { - setLoading(false); + if (!signal?.aborted) { + setLoading(false); + } } }, [fetchList, label]); useEffect(() => { - void loadFiles(); + const controller = new AbortController(); + void loadFiles(controller.signal); + return (): void => { + controller.abort(); + }; }, [loadFiles]); const handleAccordionToggle = useCallback( diff --git a/frontend/src/hooks/useActionConfig.ts b/frontend/src/hooks/useActionConfig.ts index b8e8622..55958a5 100644 --- a/frontend/src/hooks/useActionConfig.ts +++ b/frontend/src/hooks/useActionConfig.ts @@ -24,7 +24,10 @@ export interface UseActionConfigResult { * @param name - Action base name (e.g. ``"iptables"``). */ export function useActionConfig(name: string): UseActionConfigResult { - const fetchFn = useCallback(() => fetchAction(name), [name]); + const fetchFn = useCallback( + (signal: AbortSignal) => fetchAction(name, signal), + [name], + ); const saveFn = useCallback( (update: ActionConfigUpdate) => updateAction(name, update), [name], diff --git a/frontend/src/hooks/useBans.ts b/frontend/src/hooks/useBans.ts index c51471c..6a3dd9c 100644 --- a/frontend/src/hooks/useBans.ts +++ b/frontend/src/hooks/useBans.ts @@ -51,6 +51,7 @@ export function useBans( const [page, setPage] = useState(1); const [loading, setLoading] = useState(true); const [error, setError] = useState(null); + const abortRef = useRef(null); // Reset page when time range, origin filter, or source changes. useEffect(() => { @@ -58,16 +59,25 @@ export function useBans( }, [timeRange, origin, source]); const doFetch = useCallback(async (): Promise => { + abortRef.current?.abort(); + const controller = new AbortController(); + abortRef.current = controller; + setLoading(true); setError(null); + try { - const data = await fetchBans(timeRange, page, PAGE_SIZE, origin, source); + const data = await fetchBans(timeRange, page, PAGE_SIZE, origin, source, controller.signal); + if (controller.signal.aborted) return; setBanItems(data.items); setTotal(data.total); } catch (err: unknown) { + if (controller.signal.aborted) return; handleFetchError(err, setError, "Failed to fetch bans"); } finally { - setLoading(false); + if (!controller.signal.aborted) { + setLoading(false); + } } }, [timeRange, page, origin, source]); @@ -77,6 +87,9 @@ export function useBans( useEffect(() => { void doFetch(); + return (): void => { + abortRef.current?.abort(); + }; }, [doFetch]); const refresh = useCallback((): void => { diff --git a/frontend/src/hooks/useFilterConfig.ts b/frontend/src/hooks/useFilterConfig.ts index 387cada..d666f24 100644 --- a/frontend/src/hooks/useFilterConfig.ts +++ b/frontend/src/hooks/useFilterConfig.ts @@ -24,7 +24,10 @@ export interface UseFilterConfigResult { * @param name - Filter base name (e.g. ``"sshd"``). */ export function useFilterConfig(name: string): UseFilterConfigResult { - const fetchFn = useCallback(() => fetchParsedFilter(name), [name]); + const fetchFn = useCallback( + (signal: AbortSignal) => fetchParsedFilter(name, signal), + [name], + ); const saveFn = useCallback( (update: FilterConfigUpdate) => updateParsedFilter(name, update), [name], diff --git a/frontend/src/hooks/useJailFileConfig.ts b/frontend/src/hooks/useJailFileConfig.ts index dcfe40e..3dc04d4 100644 --- a/frontend/src/hooks/useJailFileConfig.ts +++ b/frontend/src/hooks/useJailFileConfig.ts @@ -22,7 +22,10 @@ export interface UseJailFileConfigResult { * @param filename - Filename including extension (e.g. ``"sshd.conf"``). */ export function useJailFileConfig(filename: string): UseJailFileConfigResult { - const fetchFn = useCallback(() => fetchParsedJailFile(filename), [filename]); + const fetchFn = useCallback( + (signal: AbortSignal) => fetchParsedJailFile(filename, signal), + [filename], + ); const saveFn = useCallback( (update: JailFileConfigUpdate) => updateParsedJailFile(filename, update), [filename], diff --git a/frontend/src/hooks/useMapColorThresholds.ts b/frontend/src/hooks/useMapColorThresholds.ts index 79b46d7..7e09694 100644 --- a/frontend/src/hooks/useMapColorThresholds.ts +++ b/frontend/src/hooks/useMapColorThresholds.ts @@ -19,22 +19,30 @@ export function useMapColorThresholds(): UseMapColorThresholdsResult { const [loading, setLoading] = useState(true); const [error, setError] = useState(null); - const load = useCallback(async (): Promise => { + const load = useCallback(async (signal?: AbortSignal): Promise => { setLoading(true); setError(null); try { - const data = await fetchMapColorThresholds(); + const data = await fetchMapColorThresholds(signal); + if (signal?.aborted) return; setThresholds(data); } catch (err: unknown) { + if (signal?.aborted) return; handleFetchError(err, setError, "Failed to fetch map color thresholds"); } finally { - setLoading(false); + if (!signal?.aborted) { + setLoading(false); + } } }, []); useEffect(() => { - void load(); + const controller = new AbortController(); + void load(controller.signal); + return (): void => { + controller.abort(); + }; }, [load]); const updateThresholds = useCallback( diff --git a/frontend/src/hooks/useSchedule.ts b/frontend/src/hooks/useSchedule.ts index 8a5c269..d6316e2 100644 --- a/frontend/src/hooks/useSchedule.ts +++ b/frontend/src/hooks/useSchedule.ts @@ -23,16 +23,27 @@ export function useSchedule(): UseScheduleReturn { const [error, setError] = useState(null); useEffect(() => { + const controller = new AbortController(); setLoading(true); - fetchSchedule() + setError(null); + + fetchSchedule(controller.signal) .then((data) => { + if (controller.signal.aborted) return; setInfo(data); - setLoading(false); }) .catch((err: unknown) => { + if (controller.signal.aborted) return; handleFetchError(err, setError, "Failed to load schedule"); + }) + .finally(() => { + if (controller.signal.aborted) return; setLoading(false); }); + + return (): void => { + controller.abort(); + }; }, []); const saveSchedule = useCallback(async (config: ScheduleConfig): Promise => { diff --git a/frontend/src/hooks/useTimezoneData.ts b/frontend/src/hooks/useTimezoneData.ts index dbfa69f..c77d228 100644 --- a/frontend/src/hooks/useTimezoneData.ts +++ b/frontend/src/hooks/useTimezoneData.ts @@ -14,23 +14,31 @@ export function useTimezoneData(): UseTimezoneDataResult { const [loading, setLoading] = useState(true); const [error, setError] = useState(null); - const load = useCallback(async (): Promise => { + const load = useCallback(async (signal?: AbortSignal): Promise => { setLoading(true); setError(null); try { - const resp = await fetchTimezone(); + const resp = await fetchTimezone(signal); + if (signal?.aborted) return; setTimezone(resp.timezone); } catch (err: unknown) { + if (signal?.aborted) return; handleFetchError(err, setError, "Failed to fetch timezone"); setTimezone("UTC"); } finally { - setLoading(false); + if (!signal?.aborted) { + setLoading(false); + } } }, []); useEffect(() => { - void load(); + const controller = new AbortController(); + void load(controller.signal); + return (): void => { + controller.abort(); + }; }, [load]); return {