From cf5a000bf563166f98bdab13a6b3895cfdaec757 Mon Sep 17 00:00:00 2001 From: Lukas Date: Tue, 21 Apr 2026 17:29:05 +0200 Subject: [PATCH] Add AbortSignal support to dashboard/blocklist APIs and hooks --- Docs/Tasks.md | 4 +- frontend/src/api/dashboard.ts | 10 ++- .../useServerAndBlocklistStatus.test.ts | 78 +++++++++++++++++++ frontend/src/hooks/useBanTrend.ts | 2 +- frontend/src/hooks/useBlocklistStatus.ts | 18 +++-- frontend/src/hooks/useJailDistribution.ts | 2 +- frontend/src/hooks/useServerStatus.ts | 24 +++++- 7 files changed, 122 insertions(+), 16 deletions(-) create mode 100644 frontend/src/hooks/__tests__/useServerAndBlocklistStatus.test.ts diff --git a/Docs/Tasks.md b/Docs/Tasks.md index fbcd874..b41c342 100644 --- a/Docs/Tasks.md +++ b/Docs/Tasks.md @@ -262,13 +262,15 @@ Issues are grouped by category and ordered roughly by severity. Each entry descr --- -### TASK-014 — Add `AbortSignal` to all API functions missing it +### TASK-014 — Add `AbortSignal` to all API functions missing it (done) **Where found:** - `frontend/src/api/dashboard.ts` — `fetchServerStatus`, `fetchBansByJail`, `fetchBanTrend` have no signal parameter. - `frontend/src/api/jails.ts` — `fetchJailBannedIps` (and others) have no signal parameter. - `frontend/src/api/blocklist.ts` — `fetchSchedule` (used by the polling hook) has no signal parameter. +**Summary:** Added optional `signal` parameters to dashboard API functions and updated `useServerStatus`, `useBanTrend`, `useJailDistribution`, and `useBlocklistStatus` to pass abort signals from refs. Added tests to verify server status and blocklist schedule polling use `AbortSignal`. + **Goal:** Add `signal?: AbortSignal` as the last parameter to every `get`/`post`/`put`/`del` wrapper call in these modules. The pattern is already established in `api/config.ts` and `api/map.ts` and should be replicated uniformly. After adding the parameters, update the consuming hooks to pass their `abortRef.current.signal`. **Possible traps:** diff --git a/frontend/src/api/dashboard.ts b/frontend/src/api/dashboard.ts index 080a2b5..50860da 100644 --- a/frontend/src/api/dashboard.ts +++ b/frontend/src/api/dashboard.ts @@ -22,8 +22,8 @@ import type { ServerStatusResponse } from "../types/server"; * `active_jails`, `total_bans`, and `total_failures`. * @throws {ApiError} When the server returns a non-2xx status. */ -export async function fetchServerStatus(): Promise { - return get(ENDPOINTS.dashboardStatus); +export async function fetchServerStatus(signal?: AbortSignal): Promise { + return get(ENDPOINTS.dashboardStatus, signal); } /** @@ -72,6 +72,7 @@ export async function fetchBanTrend( range: TimeRange, origin: BanOriginFilter = "all", source: "fail2ban" | "archive" = "fail2ban", + signal?: AbortSignal, ): Promise { const params = new URLSearchParams({ range }); if (origin !== "all") { @@ -80,7 +81,7 @@ export async function fetchBanTrend( if (source !== "fail2ban") { params.set("source", source); } - return get(`${ENDPOINTS.dashboardBansTrend}?${params.toString()}`); + return get(`${ENDPOINTS.dashboardBansTrend}?${params.toString()}`, signal); } /** @@ -96,6 +97,7 @@ export async function fetchBansByJail( range: TimeRange, origin: BanOriginFilter = "all", source: "fail2ban" | "archive" = "fail2ban", + signal?: AbortSignal, ): Promise { const params = new URLSearchParams({ range }); if (origin !== "all") { @@ -104,5 +106,5 @@ export async function fetchBansByJail( if (source !== "fail2ban") { params.set("source", source); } - return get(`${ENDPOINTS.dashboardBansByJail}?${params.toString()}`); + return get(`${ENDPOINTS.dashboardBansByJail}?${params.toString()}`, signal); } diff --git a/frontend/src/hooks/__tests__/useServerAndBlocklistStatus.test.ts b/frontend/src/hooks/__tests__/useServerAndBlocklistStatus.test.ts new file mode 100644 index 0000000..786bf4c --- /dev/null +++ b/frontend/src/hooks/__tests__/useServerAndBlocklistStatus.test.ts @@ -0,0 +1,78 @@ +import { afterEach, describe, expect, it, vi } from "vitest"; +import { renderHook, waitFor } from "@testing-library/react"; +import { useServerStatus } from "../useServerStatus"; +import { useBlocklistStatus } from "../useBlocklistStatus"; +import * as dashboardApi from "../../api/dashboard"; +import * as blocklistApi from "../../api/blocklist"; + +vi.mock("../../api/dashboard"); +vi.mock("../../api/blocklist"); + +describe("useServerStatus", () => { + const fetchMock = vi.mocked(dashboardApi.fetchServerStatus); + + afterEach(() => { + fetchMock.mockReset(); + }); + + it("fetches server status with an AbortSignal", async () => { + fetchMock.mockResolvedValue({ + status: { + online: true, + version: "1.0.0", + active_jails: 2, + total_bans: 5, + total_failures: 1, + }, + }); + + const { result, unmount } = renderHook(() => useServerStatus()); + + await waitFor(() => { + expect(result.current.loading).toBe(false); + }); + + expect(fetchMock).toHaveBeenCalledWith(expect.any(AbortSignal)); + expect(result.current.status).toEqual({ + online: true, + version: "1.0.0", + active_jails: 2, + total_bans: 5, + total_failures: 1, + }); + + unmount(); + }); +}); + +describe("useBlocklistStatus", () => { + const fetchMock = vi.mocked(blocklistApi.fetchSchedule); + + afterEach(() => { + fetchMock.mockReset(); + }); + + it("polls schedule state with an AbortSignal", async () => { + fetchMock.mockResolvedValue({ + config: { + frequency: "daily", + interval_hours: 24, + hour: 1, + minute: 0, + day_of_week: 0, + }, + next_run_at: null, + last_run_at: null, + last_run_errors: true, + }); + + const { result, unmount } = renderHook(() => useBlocklistStatus()); + + await waitFor(() => { + expect(fetchMock).toHaveBeenCalledWith(expect.any(AbortSignal)); + }); + + expect(result.current.hasErrors).toBe(true); + unmount(); + }); +}); diff --git a/frontend/src/hooks/useBanTrend.ts b/frontend/src/hooks/useBanTrend.ts index b258081..39713a7 100644 --- a/frontend/src/hooks/useBanTrend.ts +++ b/frontend/src/hooks/useBanTrend.ts @@ -59,7 +59,7 @@ export function useBanTrend( setIsLoading(true); setError(null); - fetchBanTrend(timeRange, origin, source) + fetchBanTrend(timeRange, origin, source, controller.signal) .then((data) => { if (controller.signal.aborted) return; setBuckets(data.buckets); diff --git a/frontend/src/hooks/useBlocklistStatus.ts b/frontend/src/hooks/useBlocklistStatus.ts index ee810eb..f28d39f 100644 --- a/frontend/src/hooks/useBlocklistStatus.ts +++ b/frontend/src/hooks/useBlocklistStatus.ts @@ -2,7 +2,7 @@ * React hook for polling blocklist schedule error state. */ -import { useEffect, useState } from "react"; +import { useEffect, useRef, useState } from "react"; import { fetchSchedule } from "../api/blocklist"; const BLOCKLIST_POLL_INTERVAL_MS = 60_000; @@ -17,16 +17,20 @@ export interface UseBlocklistStatusReturn { */ export function useBlocklistStatus(): UseBlocklistStatusReturn { const [hasErrors, setHasErrors] = useState(false); + const abortRef = useRef(null); useEffect(() => { - let cancelled = false; - const poll = (): void => { - fetchSchedule() + abortRef.current?.abort(); + const controller = new AbortController(); + abortRef.current = controller; + + fetchSchedule(controller.signal) .then((info) => { - if (!cancelled) { - setHasErrors(info.last_run_errors === true); + if (controller.signal.aborted) { + return; } + setHasErrors(info.last_run_errors === true); }) .catch(() => { // Silently swallow network errors — do not change indicator state. @@ -36,7 +40,7 @@ export function useBlocklistStatus(): UseBlocklistStatusReturn { poll(); const id = window.setInterval(poll, BLOCKLIST_POLL_INTERVAL_MS); return (): void => { - cancelled = true; + abortRef.current?.abort(); window.clearInterval(id); }; }, []); diff --git a/frontend/src/hooks/useJailDistribution.ts b/frontend/src/hooks/useJailDistribution.ts index ef52a8c..56715ae 100644 --- a/frontend/src/hooks/useJailDistribution.ts +++ b/frontend/src/hooks/useJailDistribution.ts @@ -58,7 +58,7 @@ export function useJailDistribution( setIsLoading(true); setError(null); - fetchBansByJail(timeRange, origin) + fetchBansByJail(timeRange, origin, "fail2ban", controller.signal) .then((data) => { if (controller.signal.aborted) return; setJails(data.jails); diff --git a/frontend/src/hooks/useServerStatus.ts b/frontend/src/hooks/useServerStatus.ts index 8e2470b..5972c0b 100644 --- a/frontend/src/hooks/useServerStatus.ts +++ b/frontend/src/hooks/useServerStatus.ts @@ -39,16 +39,30 @@ export function useServerStatus(): UseServerStatusResult { // Use a ref so the fetch function identity is stable. const fetchRef = useRef<() => Promise>(async () => Promise.resolve()); + const abortRef = useRef(null); + const doFetch = useCallback(async (): Promise => { + abortRef.current?.abort(); + const controller = new AbortController(); + abortRef.current = controller; + setLoading(true); try { - const data = await fetchServerStatus(); + const data = await fetchServerStatus(controller.signal); + if (controller.signal.aborted) { + return; + } setStatus(data.status); setError(null); } catch (err: unknown) { + if (controller.signal.aborted) { + return; + } handleFetchError(err, setError, "Failed to fetch server status"); } finally { - setLoading(false); + if (!controller.signal.aborted) { + setLoading(false); + } } }, []); @@ -74,6 +88,12 @@ export function useServerStatus(): UseServerStatusResult { return (): void => { window.removeEventListener("focus", onFocus); }; }, []); + useEffect(() => { + return (): void => { + abortRef.current?.abort(); + }; + }, []); + const refresh = useCallback((): void => { void doFetch().catch((): void => undefined); }, [doFetch]);