refactoring-backend #3
@@ -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:**
|
||||
|
||||
@@ -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<ServerStatusResponse> {
|
||||
return get<ServerStatusResponse>(ENDPOINTS.dashboardStatus);
|
||||
export async function fetchServerStatus(signal?: AbortSignal): Promise<ServerStatusResponse> {
|
||||
return get<ServerStatusResponse>(ENDPOINTS.dashboardStatus, signal);
|
||||
}
|
||||
|
||||
/**
|
||||
@@ -72,6 +72,7 @@ export async function fetchBanTrend(
|
||||
range: TimeRange,
|
||||
origin: BanOriginFilter = "all",
|
||||
source: "fail2ban" | "archive" = "fail2ban",
|
||||
signal?: AbortSignal,
|
||||
): Promise<BanTrendResponse> {
|
||||
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<BanTrendResponse>(`${ENDPOINTS.dashboardBansTrend}?${params.toString()}`);
|
||||
return get<BanTrendResponse>(`${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<BansByJailResponse> {
|
||||
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<BansByJailResponse>(`${ENDPOINTS.dashboardBansByJail}?${params.toString()}`);
|
||||
return get<BansByJailResponse>(`${ENDPOINTS.dashboardBansByJail}?${params.toString()}`, signal);
|
||||
}
|
||||
|
||||
@@ -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();
|
||||
});
|
||||
});
|
||||
@@ -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);
|
||||
|
||||
@@ -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<AbortController | null>(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);
|
||||
};
|
||||
}, []);
|
||||
|
||||
@@ -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);
|
||||
|
||||
@@ -39,16 +39,30 @@ export function useServerStatus(): UseServerStatusResult {
|
||||
// Use a ref so the fetch function identity is stable.
|
||||
const fetchRef = useRef<() => Promise<void>>(async () => Promise.resolve());
|
||||
|
||||
const abortRef = useRef<AbortController | null>(null);
|
||||
|
||||
const doFetch = useCallback(async (): Promise<void> => {
|
||||
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]);
|
||||
|
||||
Reference in New Issue
Block a user