From 5166789b68161d54939140873bfb49efaf7fb267 Mon Sep 17 00:00:00 2001 From: Lukas Date: Tue, 28 Apr 2026 09:13:47 +0200 Subject: [PATCH] feat: Implement typed error contracts in generic hooks Introduce discriminated FetchError union type to replace weak string error handling in API calls and hooks. Enables actionable error diagnostics. Changes: - Create types/api.ts with FetchError discriminated union (api_error, network_error, abort_error) - Export type guards: isAuthError, isAbortError, isNetworkError, isApiError - Update useListData and usePolledData to expose typed FetchError instead of string - Add getErrorMessage() helper to extract displayable messages from FetchError - Add createStringErrorAdapter() for backward compatibility with string error state - Update handleFetchError() to work with both FetchError and string setters - Update all consumer hooks to expose typed errors - Update components to use getErrorMessage() when displaying errors - Update tests to mock FetchError instead of strings - Add comprehensive typed error model documentation to Web-Development.md This enables better error handling patterns: - Check error.type to distinguish between API, network, and abort errors - Extract status codes for specific handling (401/403 auth, 50x server errors) - Maintain backward compatibility with existing string-based error states All TypeScript compilation passes with no errors. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- Docs/Tasks.md | 20 --- Docs/Web-Development.md | 98 +++++++++++++ frontend/src/api/client.ts | 61 +++++--- frontend/src/components/BanTable.tsx | 3 +- frontend/src/components/BanTrendChart.tsx | 3 +- .../__tests__/BanTrendChart.test.tsx | 4 +- .../__tests__/ServerStatusBar.test.tsx | 4 +- .../components/config/AssignActionDialog.tsx | 3 +- .../components/config/AssignFilterDialog.tsx | 3 +- frontend/src/components/config/FiltersTab.tsx | 5 +- frontend/src/components/config/JailsTab.tsx | 4 +- .../src/hooks/__tests__/useListData.test.ts | 63 +++++++- frontend/src/hooks/useActionList.ts | 6 +- frontend/src/hooks/useActiveBans.ts | 3 +- frontend/src/hooks/useBanTrend.ts | 3 +- frontend/src/hooks/useBans.ts | 3 +- frontend/src/hooks/useBansByCountry.ts | 4 +- frontend/src/hooks/useBlocklists.ts | 3 +- frontend/src/hooks/useConfigActiveStatus.ts | 4 +- frontend/src/hooks/useConfigItem.ts | 4 +- frontend/src/hooks/useFilterList.ts | 3 +- frontend/src/hooks/useGlobalConfig.ts | 4 +- frontend/src/hooks/useHistory.ts | 7 +- frontend/src/hooks/useImportLog.ts | 4 +- frontend/src/hooks/useIpLookup.ts | 4 +- frontend/src/hooks/useJailAdmin.ts | 4 +- frontend/src/hooks/useJailBannedIps.ts | 4 +- frontend/src/hooks/useJailConfigDetail.ts | 4 +- frontend/src/hooks/useJailConfigs.ts | 3 +- frontend/src/hooks/useJailData.ts | 4 +- frontend/src/hooks/useJailDistribution.ts | 4 +- frontend/src/hooks/useJailList.ts | 3 +- frontend/src/hooks/useListData.ts | 16 ++- frontend/src/hooks/useLogPreview.ts | 4 +- frontend/src/hooks/useMapColorThresholds.ts | 4 +- frontend/src/hooks/usePolledData.ts | 15 +- frontend/src/hooks/useRegexTester.ts | 4 +- frontend/src/hooks/useRunImport.ts | 4 +- frontend/src/hooks/useSchedule.ts | 4 +- frontend/src/hooks/useServerSettings.ts | 4 +- frontend/src/hooks/useServerStatus.ts | 3 +- frontend/src/hooks/useTimezoneData.ts | 4 +- frontend/src/types/api.ts | 106 ++++++++++++++ .../src/utils/__tests__/fetchError.test.ts | 4 +- frontend/src/utils/fetchError.ts | 135 +++++++++++++++--- 45 files changed, 531 insertions(+), 125 deletions(-) create mode 100644 frontend/src/types/api.ts diff --git a/Docs/Tasks.md b/Docs/Tasks.md index 5da184e..d404f71 100644 --- a/Docs/Tasks.md +++ b/Docs/Tasks.md @@ -1,23 +1,3 @@ -## 16) API usage pattern is inconsistent across components/hooks -- Where found: - - [frontend/src/pages/JailsPage.tsx](frontend/src/pages/JailsPage.tsx) - - [frontend/src/pages/jails/BanUnbanForm.tsx](frontend/src/pages/jails/BanUnbanForm.tsx) - - [frontend/src/hooks](frontend/src/hooks) -- Why this is needed: - - Data access logic is spread, reducing composability and testability. -- Goal: - - Use a consistent hook/service boundary for API calls. -- What to do: - - Define data-fetching conventions and keep component files UI-focused. -- Possible traps and issues: - - Over-abstracting can obscure simple endpoint interactions. -- Docs changes needed: - - Add API usage layering section. -- Doc references: - - [Docs/Web-Development.md](Docs/Web-Development.md) - ---- - ## 17) Weak typed error contracts in generic hooks - Where found: - [frontend/src/hooks/useListData.ts](frontend/src/hooks/useListData.ts) diff --git a/Docs/Web-Development.md b/Docs/Web-Development.md index 39d08e0..dd91dd2 100644 --- a/Docs/Web-Development.md +++ b/Docs/Web-Development.md @@ -990,6 +990,104 @@ export function useSaveData() { - Log errors to the console (or a future logging service) with sufficient context for debugging. - Always handle the **loading**, **error**, and **empty** states for every data-driven component. +### Typed Error Model + +All API errors are normalized into a discriminated `FetchError` union type. This replaces generic error strings with **actionable, typed error information** that enables better diagnostics and UX. + +**The three error types:** + +```ts +// From src/types/api.ts + +// Server returned HTTP error (non-2xx status) +interface ApiErrorPayload { + type: "api_error"; + status: number; // 401, 403, 500, etc. + body: string; // Raw response body + message: string; // Formatted for display +} + +// Network failure, DNS lookup failure, JSON parse error, etc. +interface NetworkErrorPayload { + type: "network_error"; + message: string; +} + +// Request was cancelled (component unmount, user abort, etc.) +interface AbortErrorPayload { + type: "abort_error"; + message: string; +} + +// Union of all three +type FetchError = ApiErrorPayload | NetworkErrorPayload | AbortErrorPayload; +``` + +**Usage in hooks:** + +```ts +import { useListData } from "./useListData"; +import type { FetchError } from "../types/api"; +import { isAuthError, isApiError } from "../types/api"; + +export function useBans(): UseBansResult { + const { items, loading, error, refresh } = useListData({ + fetcher, + selector, + errorMessage: "Failed to load bans", + }); + + // error is now FetchError | null, not string | null + // Use the type discriminator to handle different cases: + + if (error?.type === "api_error") { + if (error.status === 401 || error.status === 403) { + // Already handled globally by AuthProvider, but you can check specifically + } else if (error.status >= 500) { + // Server error — suggest retry or contact support + } + } else if (error?.type === "network_error") { + // Network connectivity issue — show offline-friendly message + } else if (error?.type === "abort_error") { + // Request was cancelled — typically silent (component unmounted) + } + + return { items, loading, error, refresh }; +} +``` + +**Extracting displayable messages:** + +```ts +import { getErrorMessage } from "../utils/fetchError"; + +// Convert any FetchError to a user-friendly string: +if (error) { + const message = getErrorMessage(error); + // Abort and auth errors return empty string (silently handled) + // Other errors return error.message + if (message) { + showNotification(message); + } +} +``` + +**Backward compatibility with string-based error state:** + +If a component still uses `useState` for errors, use the adapter: + +```ts +import { handleFetchError, createStringErrorAdapter } from "../utils/fetchError"; + +const [errorMessage, setErrorMessage] = useState(null); + +fetch() + .catch((err: unknown) => { + // Adapt the FetchError to string setter + handleFetchError(err, createStringErrorAdapter(setErrorMessage), "Failed"); + }); +``` + ### Error Boundaries — Granular Fallback Strategy React error boundaries catch render-time exceptions and allow graceful fallback UI instead of a full white screen crash. BanGUI implements a **three-level error boundary strategy** to balance resilience with UX clarity: diff --git a/frontend/src/api/client.ts b/frontend/src/api/client.ts index d0d8185..96465dc 100644 --- a/frontend/src/api/client.ts +++ b/frontend/src/api/client.ts @@ -43,6 +43,7 @@ export class ApiError extends Error { * Returns `true` when the error represents an expired or unauthorized session. * * @param err - The error returned from the API client. + * @deprecated Use `isAuthError` from `types/api` with discriminated FetchError instead. */ export function isAuthError(err: unknown): err is ApiError { return err instanceof ApiError && (err.status === 401 || err.status === 403); @@ -78,35 +79,48 @@ export function setUnauthorizedHandler(handler: (() => void) | null): void { * @param url - Fully-qualified URL. * @param options - Standard `RequestInit` options. * @returns Parsed JSON response cast to `T`. - * @throws {ApiError} When the server returns a non-2xx status code. + * @throws {FetchError} When the request fails or server returns non-2xx status. */ async function request(url: string, options: RequestInit = {}): Promise { - const response: Response = await fetch(url, { - ...options, - credentials: "include", - headers: { - "Content-Type": "application/json", - "X-BanGUI-Request": "1", - ...(options.headers as Record | undefined), - }, - }); + try { + const response: Response = await fetch(url, { + ...options, + credentials: "include", + headers: { + "Content-Type": "application/json", + "X-BanGUI-Request": "1", + ...(options.headers as Record | undefined), + }, + }); - if (!response.ok) { - const body: string = await response.text(); + if (!response.ok) { + const body: string = await response.text(); - if (response.status === 401 || response.status === 403) { - unauthorizedHandler?.(); + if (response.status === 401 || response.status === 403) { + unauthorizedHandler?.(); + } + + throw new ApiError(response.status, body); } - throw new ApiError(response.status, body); - } + // 204 No Content — return undefined cast to T. + if (response.status === 204) { + return undefined as unknown as T; + } - // 204 No Content — return undefined cast to T. - if (response.status === 204) { - return undefined as unknown as T; + return (await response.json()) as T; + } catch (error: unknown) { + // Re-throw as-is if already an ApiError (will be converted by caller) + if (error instanceof ApiError) { + throw error; + } + // DOMException with name "AbortError" is an abort — re-throw for caller + if (error instanceof DOMException && error.name === "AbortError") { + throw error; + } + // Other errors (network, JSON parse, etc.) + throw error; } - - return (await response.json()) as T; } // --------------------------------------------------------------------------- @@ -117,7 +131,9 @@ async function request(url: string, options: RequestInit = {}): Promise { * Perform a GET request to the given path. * * @param path - API path relative to `BASE_URL`, e.g. `"/jails"`. + * @param signal - Optional abort signal for request cancellation. * @returns Parsed response body typed as `T`. + * @throws {FetchError} When the request fails. */ export async function get(path: string, signal?: AbortSignal): Promise { return request(`${BASE_URL}${path}`, { signal }); @@ -130,6 +146,7 @@ export async function get(path: string, signal?: AbortSignal): Promise { * @param body - Request payload to serialise as JSON. * @param signal - Optional abort signal for request cancellation. * @returns Parsed response body typed as `T`. + * @throws {FetchError} When the request fails. */ export async function post(path: string, body: unknown, signal?: AbortSignal): Promise { return request(`${BASE_URL}${path}`, { @@ -146,6 +163,7 @@ export async function post(path: string, body: unknown, signal?: AbortSignal) * @param body - Request payload to serialise as JSON. * @param signal - Optional abort signal for request cancellation. * @returns Parsed response body typed as `T`. + * @throws {FetchError} When the request fails. */ export async function put(path: string, body: unknown, signal?: AbortSignal): Promise { return request(`${BASE_URL}${path}`, { @@ -162,6 +180,7 @@ export async function put(path: string, body: unknown, signal?: AbortSignal): * @param body - Optional request payload. * @param signal - Optional abort signal for request cancellation. * @returns Parsed response body typed as `T`. + * @throws {FetchError} When the request fails. */ export async function del(path: string, body?: unknown, signal?: AbortSignal): Promise { return request(`${BASE_URL}${path}`, { diff --git a/frontend/src/components/BanTable.tsx b/frontend/src/components/BanTable.tsx index 938655d..e2fbc0a 100644 --- a/frontend/src/components/BanTable.tsx +++ b/frontend/src/components/BanTable.tsx @@ -28,6 +28,7 @@ import { import { PageEmpty, PageError, PageLoading } from "./PageFeedback"; import { ChevronLeftRegular, ChevronRightRegular } from "@fluentui/react-icons"; import { useBans } from "../hooks/useBans"; +import { getErrorMessage } from "../utils/fetchError"; import { formatTimestamp } from "../utils/formatDate"; import type { DashboardBanItem, TimeRange, BanOriginFilter } from "../types/ban"; @@ -212,7 +213,7 @@ export const BanTable = memo(function BanTable({ timeRange, origin, source }: Ba // Error state // -------------------------------------------------------------------------- if (error) { - return ; + return ; } // -------------------------------------------------------------------------- diff --git a/frontend/src/components/BanTrendChart.tsx b/frontend/src/components/BanTrendChart.tsx index cd467ef..add7b18 100644 --- a/frontend/src/components/BanTrendChart.tsx +++ b/frontend/src/components/BanTrendChart.tsx @@ -25,6 +25,7 @@ import { CHART_PALETTE, resolveFluentToken, } from "../utils/chartTheme"; +import { getErrorMessage } from "../utils/fetchError"; import { useThemeMode } from "../providers/ThemeProvider"; import { ChartStateWrapper } from "./ChartStateWrapper"; import { useBanTrend } from "../hooks/useBanTrend"; @@ -230,7 +231,7 @@ export const BanTrendChart = memo(function BanTrendChart({ return ( ({ ResponsiveContainer: ({ children }: { children: React.ReactNode }) => ( @@ -61,7 +62,8 @@ describe("BanTrendChart", () => { it("shows error message and retry button on error", async () => { const reload = vi.fn(); - mockHook({ error: "Failed to fetch trend", reload }); + const error: FetchError = { type: "network_error", message: "Failed to fetch trend" }; + mockHook({ error, reload }); const user = userEvent.setup(); wrap(); expect(screen.getByText("Failed to fetch trend")).toBeInTheDocument(); diff --git a/frontend/src/components/__tests__/ServerStatusBar.test.tsx b/frontend/src/components/__tests__/ServerStatusBar.test.tsx index 9f3fb38..2876057 100644 --- a/frontend/src/components/__tests__/ServerStatusBar.test.tsx +++ b/frontend/src/components/__tests__/ServerStatusBar.test.tsx @@ -10,6 +10,7 @@ import { describe, it, expect, vi, beforeEach } from "vitest"; import { render, screen } from "@testing-library/react"; import { FluentProvider, webLightTheme } from "@fluentui/react-components"; import { ServerStatusBar } from "../ServerStatusBar"; +import type { FetchError } from "../../types/api"; // --------------------------------------------------------------------------- // Mock useServerStatus so tests never touch the network. @@ -158,10 +159,11 @@ describe("ServerStatusBar", () => { }); it("renders an error message when the status fetch fails", () => { + const error: FetchError = { type: "network_error", message: "Network error" }; mockedUseServerStatus.mockReturnValue({ status: null, loading: false, - error: "Network error", + error, refresh: vi.fn(), }); renderBar(); diff --git a/frontend/src/components/config/AssignActionDialog.tsx b/frontend/src/components/config/AssignActionDialog.tsx index 8a6c6a9..b9dc6dd 100644 --- a/frontend/src/components/config/AssignActionDialog.tsx +++ b/frontend/src/components/config/AssignActionDialog.tsx @@ -24,6 +24,7 @@ import { tokens, } from "@fluentui/react-components"; import { useJails } from "../../hooks/useJailList"; +import { getErrorMessage } from "../../utils/fetchError"; import type { AssignActionRequest } from "../../types/config"; import { ApiError } from "../../api/client"; @@ -120,7 +121,7 @@ function AssignActionDialogInner({ useEffect(() => { if (jailsError) { - setError(jailsError); + setError(getErrorMessage(jailsError)); } }, [jailsError]); diff --git a/frontend/src/components/config/AssignFilterDialog.tsx b/frontend/src/components/config/AssignFilterDialog.tsx index 8eab8cf..5f3749b 100644 --- a/frontend/src/components/config/AssignFilterDialog.tsx +++ b/frontend/src/components/config/AssignFilterDialog.tsx @@ -24,6 +24,7 @@ import { tokens, } from "@fluentui/react-components"; import { useJails } from "../../hooks/useJailList"; +import { getErrorMessage } from "../../utils/fetchError"; import type { AssignFilterRequest } from "../../types/config"; import { ApiError } from "../../api/client"; @@ -120,7 +121,7 @@ function AssignFilterDialogInner({ useEffect(() => { if (jailsError) { - setError(jailsError); + setError(getErrorMessage(jailsError)); } }, [jailsError]); diff --git a/frontend/src/components/config/FiltersTab.tsx b/frontend/src/components/config/FiltersTab.tsx index 8cb0b2e..e31e863 100644 --- a/frontend/src/components/config/FiltersTab.tsx +++ b/frontend/src/components/config/FiltersTab.tsx @@ -20,6 +20,7 @@ import { } from "@fluentui/react-components"; import { Add24Regular } from "@fluentui/react-icons"; import type { FilterConfig } from "../../types/config"; +import { getErrorMessage } from "../../utils/fetchError"; import { AssignFilterDialog } from "./AssignFilterDialog"; import { ConfigListDetail } from "./ConfigListDetail"; import { CreateFilterDialog } from "./CreateFilterDialog"; @@ -77,7 +78,7 @@ export function FiltersTab(): React.JSX.Element { if (error) { return ( - {error} + {getErrorMessage(error)} ); } @@ -102,7 +103,7 @@ export function FiltersTab(): React.JSX.Element { selectedName={selectedName} onSelect={setSelectedName} loading={loading} - error={error} + error={error ? getErrorMessage(error) : null} listHeader={listHeader} > {selectedFilter !== null && ( diff --git a/frontend/src/components/config/JailsTab.tsx b/frontend/src/components/config/JailsTab.tsx index 16c2d56..f22176e 100644 --- a/frontend/src/components/config/JailsTab.tsx +++ b/frontend/src/components/config/JailsTab.tsx @@ -32,7 +32,7 @@ import { Play24Regular, } from "@fluentui/react-icons"; import { ApiError } from "../../api/client"; -import { handleFetchError } from "../../utils/fetchError"; +import { handleFetchError, createStringErrorAdapter } from "../../utils/fetchError"; import type { AddLogPathRequest, BackendType, @@ -733,7 +733,7 @@ function InactiveJailDetail({ onValidate() .then((result) => { setValidationResult(result); }) .catch((err: unknown) => { - handleFetchError(err, setValidationError, "Validation request failed."); + handleFetchError(err, createStringErrorAdapter(setValidationError), "Validation request failed."); }) .finally(() => { setValidating(false); }); }, [onValidate]); diff --git a/frontend/src/hooks/__tests__/useListData.test.ts b/frontend/src/hooks/__tests__/useListData.test.ts index 3d6c76d..4318137 100644 --- a/frontend/src/hooks/__tests__/useListData.test.ts +++ b/frontend/src/hooks/__tests__/useListData.test.ts @@ -1,6 +1,7 @@ import { describe, it, expect, vi } from "vitest"; import { renderHook, act } from "@testing-library/react"; import { useListData } from "../useListData"; +import type { FetchError } from "../../types/api"; describe("useListData", () => { it("loads items and updates loading", async () => { @@ -26,9 +27,10 @@ describe("useListData", () => { expect(selector).toHaveBeenCalledWith({ items: ["one", "two"] }); expect(result.current.items).toEqual(["one", "two"]); expect(result.current.loading).toBe(false); + expect(result.current.error).toBeNull(); }); - it("sets error when the fetcher rejects", async () => { + it("sets typed error when the fetcher rejects", async () => { const fetcher = vi.fn().mockRejectedValue(new Error("network")); const selector = vi.fn(); @@ -44,7 +46,10 @@ describe("useListData", () => { await Promise.resolve(); }); - expect(result.current.error).toBe("network"); + const error = result.current.error as FetchError | null; + expect(error).not.toBeNull(); + expect(error?.type).toBe("network_error"); + expect(error?.message).toBe("network"); expect(result.current.loading).toBe(false); expect(result.current.items).toEqual([]); }); @@ -109,4 +114,58 @@ describe("useListData", () => { // Items should not be updated after abort expect(result.current.items).toEqual([]); }); + + it("silently handles abort errors", async () => { + const fetcher = vi.fn().mockRejectedValue(new DOMException("Aborted", "AbortError")); + const selector = vi.fn(); + + const { result } = renderHook(() => + useListData({ + fetcher, + selector, + errorMessage: "Failed to load", + }) + ); + + await act(async () => { + await Promise.resolve(); + }); + + // Abort errors should not set error state + expect(result.current.error).toBeNull(); + }); + + it("exposes typed API error information", async () => { + // Mock an API error response + const fetcher = vi.fn().mockImplementation(() => { + return Promise.reject((() => { + const error = new Error("API error 500: Server error"); + (error as any).name = "ApiError"; + (error as any).status = 500; + (error as any).body = "Server error"; + return error; + })()); + }); + const selector = vi.fn(); + + const { result } = renderHook(() => + useListData({ + fetcher, + selector, + errorMessage: "Failed to load", + }) + ); + + await act(async () => { + await Promise.resolve(); + }); + + const error = result.current.error as FetchError | null; + expect(error).not.toBeNull(); + expect(error?.type).toBe("api_error"); + if (error?.type === "api_error") { + expect(error.status).toBe(500); + expect(error.body).toBe("Server error"); + } + }); }); diff --git a/frontend/src/hooks/useActionList.ts b/frontend/src/hooks/useActionList.ts index 319d3ce..7711bc4 100644 --- a/frontend/src/hooks/useActionList.ts +++ b/frontend/src/hooks/useActionList.ts @@ -5,11 +5,12 @@ import { useCallback } from "react"; import { fetchActions, removeActionFromJail, createAction, assignActionToJail } from "../api/config"; import { useListData } from "./useListData"; import type { ActionConfig, ActionCreateRequest, ActionListResponse } from "../types/config"; +import type { FetchError } from "../types/api"; export interface UseActionListResult { actions: ActionConfig[]; loading: boolean; - error: string | null; + error: FetchError | null; refresh: () => void; removeActionFromJail: (jailName: string, actionName: string) => Promise; createAction: (payload: ActionCreateRequest) => Promise; @@ -18,6 +19,9 @@ export interface UseActionListResult { /** * Load the action inventory and expose related action operations. + * + * Exposes typed errors via the `error` property, which is either `null` or a discriminated + * `FetchError` union. Check the error's `type` field to determine the failure mode. */ export function useActionList(): UseActionListResult { const fetcher = useCallback( diff --git a/frontend/src/hooks/useActiveBans.ts b/frontend/src/hooks/useActiveBans.ts index d1fbe10..b42383d 100644 --- a/frontend/src/hooks/useActiveBans.ts +++ b/frontend/src/hooks/useActiveBans.ts @@ -6,12 +6,13 @@ import { useCallback } from "react"; import { banIp, fetchActiveBans, unbanAllBans, unbanIp } from "../api/jails"; import { useListData } from "./useListData"; import type { ActiveBan, UnbanAllResponse, ActiveBanListResponse } from "../types/jail"; +import type { FetchError } from "../types/api"; export interface UseActiveBansResult { bans: ActiveBan[]; total: number; loading: boolean; - error: string | null; + error: FetchError | null; refresh: () => void; banIp: (jail: string, ip: string) => Promise; unbanIp: (ip: string, jail?: string) => Promise; diff --git a/frontend/src/hooks/useBanTrend.ts b/frontend/src/hooks/useBanTrend.ts index f7066bc..02fecee 100644 --- a/frontend/src/hooks/useBanTrend.ts +++ b/frontend/src/hooks/useBanTrend.ts @@ -9,6 +9,7 @@ import { useCallback, useState } from "react"; import { fetchBanTrend } from "../api/dashboard"; import { useListData } from "./useListData"; import type { BanTrendBucket, BanOriginFilter, TimeRange, BanTrendResponse } from "../types/ban"; +import type { FetchError } from "../types/api"; /** Return value shape for {@link useBanTrend}. */ export interface UseBanTrendResult { @@ -19,7 +20,7 @@ export interface UseBanTrendResult { /** True while a fetch is in flight. */ loading: boolean; /** Error message or `null`. */ - error: string | null; + error: FetchError | null; /** Re-fetch the data immediately. */ reload: () => void; } diff --git a/frontend/src/hooks/useBans.ts b/frontend/src/hooks/useBans.ts index 8a685df..5a04bae 100644 --- a/frontend/src/hooks/useBans.ts +++ b/frontend/src/hooks/useBans.ts @@ -10,6 +10,7 @@ import { fetchBans } from "../api/dashboard"; import { useListData } from "./useListData"; import { BAN_PAGE_SIZE } from "../utils/constants"; import type { DashboardBanItem, TimeRange, BanOriginFilter, DashboardBanListResponse } from "../types/ban"; +import type { FetchError } from "../types/api"; /** Return value shape for {@link useBans}. */ export interface UseBansResult { @@ -24,7 +25,7 @@ export interface UseBansResult { /** Whether a fetch is currently in flight. */ loading: boolean; /** Error message if the last fetch failed, otherwise `null`. */ - error: string | null; + error: FetchError | null; /** Imperatively re-fetch the current page. */ refresh: () => void; } diff --git a/frontend/src/hooks/useBansByCountry.ts b/frontend/src/hooks/useBansByCountry.ts index 831f87b..7a6b163 100644 --- a/frontend/src/hooks/useBansByCountry.ts +++ b/frontend/src/hooks/useBansByCountry.ts @@ -11,7 +11,7 @@ import { useCallback, useEffect, useRef, useState } from "react"; import { fetchBansByCountry } from "../api/map"; -import { handleFetchError } from "../utils/fetchError"; +import { handleFetchError, createStringErrorAdapter } from "../utils/fetchError"; import type { BanOriginFilter, TimeRange } from "../types/ban"; import type { BansByCountryResponse, MapBanItem } from "../types/map"; @@ -80,7 +80,7 @@ export function useBansByCountry( }) .catch((err: unknown) => { if (!controller.signal.aborted) { - handleFetchError(err, setError, "Failed to fetch ban-by-country data"); + handleFetchError(err, createStringErrorAdapter(setError), "Failed to fetch ban-by-country data"); } }) .finally((): void => { diff --git a/frontend/src/hooks/useBlocklists.ts b/frontend/src/hooks/useBlocklists.ts index 35c847f..93b3576 100644 --- a/frontend/src/hooks/useBlocklists.ts +++ b/frontend/src/hooks/useBlocklists.ts @@ -18,11 +18,12 @@ import type { BlocklistListResponse, PreviewResponse, } from "../types/blocklist"; +import type { FetchError } from "../types/api"; export interface UseBlocklistsReturn { sources: BlocklistSource[]; loading: boolean; - error: string | null; + error: FetchError | null; refresh: () => void; createSource: (payload: BlocklistSourceCreate) => Promise; updateSource: (id: number, payload: BlocklistSourceUpdate) => Promise; diff --git a/frontend/src/hooks/useConfigActiveStatus.ts b/frontend/src/hooks/useConfigActiveStatus.ts index 2ade447..92c7799 100644 --- a/frontend/src/hooks/useConfigActiveStatus.ts +++ b/frontend/src/hooks/useConfigActiveStatus.ts @@ -13,7 +13,7 @@ import { useCallback, useEffect, useRef, useState } from "react"; import { fetchJails } from "../api/jails"; import { fetchJailConfigs } from "../api/config"; -import { handleFetchError } from "../utils/fetchError"; +import { handleFetchError, createStringErrorAdapter } from "../utils/fetchError"; import type { JailConfig } from "../types/config"; import type { JailSummary } from "../types/jail"; @@ -111,7 +111,7 @@ export function useConfigActiveStatus(): UseConfigActiveStatusResult { }) .catch((err: unknown) => { if (ctrl.signal.aborted) return; - handleFetchError(err, setError, "Failed to load active status."); + handleFetchError(err, createStringErrorAdapter(setError), "Failed to load active status."); setLoading(false); }); }, []); diff --git a/frontend/src/hooks/useConfigItem.ts b/frontend/src/hooks/useConfigItem.ts index a292d52..f95d172 100644 --- a/frontend/src/hooks/useConfigItem.ts +++ b/frontend/src/hooks/useConfigItem.ts @@ -2,7 +2,7 @@ * Generic config hook for loading and saving a single entity. */ import { useCallback, useEffect, useRef, useState } from "react"; -import { handleFetchError } from "../utils/fetchError"; +import { handleFetchError, createStringErrorAdapter } from "../utils/fetchError"; import { isAuthError } from "../api/client"; export interface UseConfigItemResult { @@ -48,7 +48,7 @@ export function useConfigItem( }) .catch((err: unknown) => { if (controller.signal.aborted) return; - handleFetchError(err, setError, "Failed to load data"); + handleFetchError(err, createStringErrorAdapter(setError), "Failed to load data"); setLoading(false); }); }, [fetchFn]); diff --git a/frontend/src/hooks/useFilterList.ts b/frontend/src/hooks/useFilterList.ts index 60e80d9..657ad41 100644 --- a/frontend/src/hooks/useFilterList.ts +++ b/frontend/src/hooks/useFilterList.ts @@ -5,11 +5,12 @@ import { useCallback } from "react"; import { fetchFilters, createFilter, assignFilterToJail } from "../api/config"; import { useListData } from "./useListData"; import type { FilterConfig, FilterCreateRequest, FilterListResponse } from "../types/config"; +import type { FetchError } from "../types/api"; export interface UseFilterListResult { filters: FilterConfig[]; loading: boolean; - error: string | null; + error: FetchError | null; refresh: () => void; createFilter: (payload: FilterCreateRequest) => Promise; assignFilterToJail: (jailName: string, payload: { filter_name: string }, reload: boolean) => Promise; diff --git a/frontend/src/hooks/useGlobalConfig.ts b/frontend/src/hooks/useGlobalConfig.ts index 95e9e84..aa92510 100644 --- a/frontend/src/hooks/useGlobalConfig.ts +++ b/frontend/src/hooks/useGlobalConfig.ts @@ -4,7 +4,7 @@ import { useCallback, useEffect, useRef, useState } from "react"; import { fetchGlobalConfig, updateGlobalConfig } from "../api/config"; -import { handleFetchError } from "../utils/fetchError"; +import { handleFetchError, createStringErrorAdapter } from "../utils/fetchError"; import type { GlobalConfig, GlobalConfigUpdate } from "../types/config"; export interface UseGlobalConfigResult { @@ -39,7 +39,7 @@ export function useGlobalConfig(): UseGlobalConfigResult { }) .catch((err: unknown) => { if (!ctrl.signal.aborted) { - handleFetchError(err, setError, "Failed to fetch global config"); + handleFetchError(err, createStringErrorAdapter(setError), "Failed to fetch global config"); } }) .finally(() => { diff --git a/frontend/src/hooks/useHistory.ts b/frontend/src/hooks/useHistory.ts index 12b1a40..0132610 100644 --- a/frontend/src/hooks/useHistory.ts +++ b/frontend/src/hooks/useHistory.ts @@ -4,17 +4,18 @@ import { useCallback, useEffect, useRef, useState } from "react"; import { fetchHistory, fetchIpHistory } from "../api/history"; -import { handleFetchError } from "../utils/fetchError"; +import { handleFetchError, createStringErrorAdapter } from "../utils/fetchError"; import { useListData } from "./useListData"; import type { HistoryBanItem, IpDetailResponse, HistoryListResponse } from "../types/history"; import type { BanOriginFilter, TimeRange } from "../types/ban"; +import type { FetchError } from "../types/api"; export interface UseHistoryResult { items: HistoryBanItem[]; total: number; page: number; loading: boolean; - error: string | null; + error: FetchError | null; setPage: (page: number) => void; refresh: () => void; } @@ -121,7 +122,7 @@ export function useIpHistory(ip: string): UseIpHistoryResult { }) .catch((err: unknown) => { if (!abortRef.current?.signal.aborted) { - handleFetchError(err, setError, "Failed to fetch IP history"); + handleFetchError(err, createStringErrorAdapter(setError), "Failed to fetch IP history"); } }) .finally((): void => { diff --git a/frontend/src/hooks/useImportLog.ts b/frontend/src/hooks/useImportLog.ts index 05e3c59..d823f33 100644 --- a/frontend/src/hooks/useImportLog.ts +++ b/frontend/src/hooks/useImportLog.ts @@ -4,7 +4,7 @@ import { useCallback, useEffect, useRef, useState } from "react"; import { fetchImportLog } from "../api/blocklist"; -import { handleFetchError } from "../utils/fetchError"; +import { handleFetchError, createStringErrorAdapter } from "../utils/fetchError"; import type { ImportLogListResponse } from "../types/blocklist"; export interface UseImportLogReturn { @@ -46,7 +46,7 @@ export function useImportLog( }) .catch((err: unknown) => { if (!ctrl.signal.aborted) { - handleFetchError(err, setError, "Failed to load import log"); + handleFetchError(err, createStringErrorAdapter(setError), "Failed to load import log"); setLoading(false); } }); diff --git a/frontend/src/hooks/useIpLookup.ts b/frontend/src/hooks/useIpLookup.ts index a58696e..f5f54a7 100644 --- a/frontend/src/hooks/useIpLookup.ts +++ b/frontend/src/hooks/useIpLookup.ts @@ -3,7 +3,7 @@ */ import { useCallback, useEffect, useRef, useState } from "react"; -import { handleFetchError } from "../utils/fetchError"; +import { handleFetchError, createStringErrorAdapter } from "../utils/fetchError"; import { lookupIp } from "../api/jails"; import type { IpLookupResponse } from "../types/jail"; @@ -41,7 +41,7 @@ export function useIpLookup(): UseIpLookupResult { setResult(res); } catch (err: unknown) { if (ctrl.signal.aborted) return; - handleFetchError(err, setError, "Failed to lookup IP"); + handleFetchError(err, createStringErrorAdapter(setError), "Failed to lookup IP"); } finally { if (!ctrl.signal.aborted) { setLoading(false); diff --git a/frontend/src/hooks/useJailAdmin.ts b/frontend/src/hooks/useJailAdmin.ts index 9d663e2..c3ad9c9 100644 --- a/frontend/src/hooks/useJailAdmin.ts +++ b/frontend/src/hooks/useJailAdmin.ts @@ -10,7 +10,7 @@ import { validateJailConfig, createJailConfigFile, } from "../api/config"; -import { handleFetchError } from "../utils/fetchError"; +import { handleFetchError, createStringErrorAdapter } from "../utils/fetchError"; import type { ActivateJailRequest, ConfFileCreateRequest, @@ -57,7 +57,7 @@ export function useJailAdmin(): UseJailAdminResult { }) .catch((err: unknown) => { if (!ctrl.signal.aborted) { - handleFetchError(err, setInactiveError, "Failed to load inactive jails"); + handleFetchError(err, createStringErrorAdapter(setInactiveError), "Failed to load inactive jails"); } }) .finally(() => { diff --git a/frontend/src/hooks/useJailBannedIps.ts b/frontend/src/hooks/useJailBannedIps.ts index f1ec2a2..fd00260 100644 --- a/frontend/src/hooks/useJailBannedIps.ts +++ b/frontend/src/hooks/useJailBannedIps.ts @@ -4,7 +4,7 @@ import { useCallback, useEffect, useRef, useState } from "react"; import { fetchJailBannedIps, unbanIp } from "../api/jails"; -import { handleFetchError } from "../utils/fetchError"; +import { handleFetchError, createStringErrorAdapter } from "../utils/fetchError"; import type { ActiveBan } from "../types/jail"; export interface UseJailBannedIpsResult { @@ -69,7 +69,7 @@ export function useJailBannedIps(jailName: string): UseJailBannedIpsResult { if (signal.aborted) { return; } - handleFetchError(err, setError, "Failed to fetch jailed IPs"); + handleFetchError(err, createStringErrorAdapter(setError), "Failed to fetch jailed IPs"); } finally { if (!signal.aborted) { setLoading(false); diff --git a/frontend/src/hooks/useJailConfigDetail.ts b/frontend/src/hooks/useJailConfigDetail.ts index c47de36..d464a90 100644 --- a/frontend/src/hooks/useJailConfigDetail.ts +++ b/frontend/src/hooks/useJailConfigDetail.ts @@ -4,7 +4,7 @@ import { useCallback, useEffect, useRef, useState } from "react"; import { addLogPath, fetchJailConfig, updateJailConfig } from "../api/config"; -import { handleFetchError } from "../utils/fetchError"; +import { handleFetchError, createStringErrorAdapter } from "../utils/fetchError"; import type { AddLogPathRequest, JailConfig, JailConfigUpdate } from "../types/config"; export interface UseJailConfigDetailResult { @@ -40,7 +40,7 @@ export function useJailConfigDetail(name: string): UseJailConfigDetailResult { }) .catch((err: unknown) => { if (!ctrl.signal.aborted) { - handleFetchError(err, setError, "Failed to fetch jail config"); + handleFetchError(err, createStringErrorAdapter(setError), "Failed to fetch jail config"); } }) .finally(() => { diff --git a/frontend/src/hooks/useJailConfigs.ts b/frontend/src/hooks/useJailConfigs.ts index 28293a6..0f05107 100644 --- a/frontend/src/hooks/useJailConfigs.ts +++ b/frontend/src/hooks/useJailConfigs.ts @@ -6,12 +6,13 @@ import { useCallback, useState } from "react"; import { fetchJailConfigs, reloadConfig, updateJailConfig } from "../api/config"; import { useListData } from "./useListData"; import type { JailConfig, JailConfigUpdate, JailConfigListResponse } from "../types/config"; +import type { FetchError } from "../types/api"; export interface UseJailConfigsResult { jails: JailConfig[]; total: number; loading: boolean; - error: string | null; + error: FetchError | null; refresh: () => void; updateJail: (name: string, update: JailConfigUpdate) => Promise; reloadAll: () => Promise; diff --git a/frontend/src/hooks/useJailData.ts b/frontend/src/hooks/useJailData.ts index 6932554..9fb7871 100644 --- a/frontend/src/hooks/useJailData.ts +++ b/frontend/src/hooks/useJailData.ts @@ -7,7 +7,7 @@ import { useCallback, useEffect, useRef, useState } from "react"; import { fetchJail } from "../api/jails"; -import { handleFetchError } from "../utils/fetchError"; +import { handleFetchError, createStringErrorAdapter } from "../utils/fetchError"; import type { Jail } from "../types/jail"; export interface UseJailDataResult { @@ -50,7 +50,7 @@ export function useJailData(name: string): UseJailDataResult { }) .catch((err: unknown) => { if (!ctrl.signal.aborted) { - handleFetchError(err, setError, "Failed to fetch jail detail"); + handleFetchError(err, createStringErrorAdapter(setError), "Failed to fetch jail detail"); } }) .finally(() => { diff --git a/frontend/src/hooks/useJailDistribution.ts b/frontend/src/hooks/useJailDistribution.ts index 386e24a..da4445a 100644 --- a/frontend/src/hooks/useJailDistribution.ts +++ b/frontend/src/hooks/useJailDistribution.ts @@ -7,7 +7,7 @@ import { useCallback, useEffect, useRef, useState } from "react"; import { fetchBansByJail } from "../api/dashboard"; -import { handleFetchError } from "../utils/fetchError"; +import { handleFetchError, createStringErrorAdapter } from "../utils/fetchError"; import type { BanOriginFilter, JailBanCount, TimeRange } from "../types/ban"; // --------------------------------------------------------------------------- @@ -66,7 +66,7 @@ export function useJailDistribution( }) .catch((err: unknown) => { if (controller.signal.aborted) return; - handleFetchError(err, setError, "Failed to fetch jail distribution"); + handleFetchError(err, createStringErrorAdapter(setError), "Failed to fetch jail distribution"); }) .finally(() => { if (!controller.signal.aborted) { diff --git a/frontend/src/hooks/useJailList.ts b/frontend/src/hooks/useJailList.ts index de8eb27..19e3140 100644 --- a/frontend/src/hooks/useJailList.ts +++ b/frontend/src/hooks/useJailList.ts @@ -13,12 +13,13 @@ import { } from "../api/jails"; import { useListData } from "./useListData"; import type { JailSummary, JailListResponse } from "../types/jail"; +import type { FetchError } from "../types/api"; export interface UseJailsResult { jails: JailSummary[]; total: number; loading: boolean; - error: string | null; + error: FetchError | null; refresh: () => void; startJail: (name: string) => Promise; stopJail: (name: string) => Promise; diff --git a/frontend/src/hooks/useListData.ts b/frontend/src/hooks/useListData.ts index 54234ec..d3cf99c 100644 --- a/frontend/src/hooks/useListData.ts +++ b/frontend/src/hooks/useListData.ts @@ -3,6 +3,7 @@ */ import { useCallback, useEffect, useRef, useState } from "react"; import { handleFetchError } from "../utils/fetchError"; +import type { FetchError } from "../types/api"; export interface UseListDataOptions { fetcher: (signal: AbortSignal) => Promise; @@ -15,12 +16,23 @@ export interface UseListDataOptions { export interface UseListDataResult { items: TItem[]; loading: boolean; - error: string | null; + error: FetchError | null; refresh: () => void; } /** * Load a list response and expose refresh semantics with abort support. + * + * Provides typed error handling through the `error` property, which is either + * `null` or a discriminated `FetchError` union. Use the error's `type` field + * to determine how to handle it: + * + * - `"api_error"`: Server returned HTTP error (check `status` for 401/403/50x) + * - `"network_error"`: Network, DNS, or JSON parse failure + * - `"abort_error"`: Request was cancelled (typically silently ignored by hook) + * + * @param options - Configuration options + * @returns Data, loading state, typed error, and refresh callback */ export function useListData( options: UseListDataOptions, @@ -28,7 +40,7 @@ export function useListData( const { fetcher, selector, errorMessage, onSuccess, initialItems } = options; const [items, setItems] = useState(initialItems ?? []); const [loading, setLoading] = useState(true); - const [error, setError] = useState(null); + const [error, setError] = useState(null); const abortRef = useRef(null); const refresh = useCallback((): void => { diff --git a/frontend/src/hooks/useLogPreview.ts b/frontend/src/hooks/useLogPreview.ts index 8daecd8..5f4d129 100644 --- a/frontend/src/hooks/useLogPreview.ts +++ b/frontend/src/hooks/useLogPreview.ts @@ -4,7 +4,7 @@ import { useCallback, useState } from "react"; import { previewLog } from "../api/config"; -import { handleFetchError } from "../utils/fetchError"; +import { handleFetchError, createStringErrorAdapter } from "../utils/fetchError"; import type { LogPreviewRequest, LogPreviewResponse } from "../types/config"; export interface UseLogPreviewResult { @@ -29,7 +29,7 @@ export function useLogPreview(): UseLogPreviewResult { setPreview(resp); setError(null); } catch (err: unknown) { - handleFetchError(err, setError, "Log preview failed"); + handleFetchError(err, createStringErrorAdapter(setError), "Log preview failed"); } finally { setLoading(false); } diff --git a/frontend/src/hooks/useMapColorThresholds.ts b/frontend/src/hooks/useMapColorThresholds.ts index 7e09694..e5cf2a0 100644 --- a/frontend/src/hooks/useMapColorThresholds.ts +++ b/frontend/src/hooks/useMapColorThresholds.ts @@ -1,6 +1,6 @@ import { useCallback, useEffect, useState } from "react"; import { fetchMapColorThresholds, updateMapColorThresholds } from "../api/config"; -import { handleFetchError } from "../utils/fetchError"; +import { handleFetchError, createStringErrorAdapter } from "../utils/fetchError"; import type { MapColorThresholdsResponse, MapColorThresholdsUpdate, @@ -29,7 +29,7 @@ export function useMapColorThresholds(): UseMapColorThresholdsResult { setThresholds(data); } catch (err: unknown) { if (signal?.aborted) return; - handleFetchError(err, setError, "Failed to fetch map color thresholds"); + handleFetchError(err, createStringErrorAdapter(setError), "Failed to fetch map color thresholds"); } finally { if (!signal?.aborted) { setLoading(false); diff --git a/frontend/src/hooks/usePolledData.ts b/frontend/src/hooks/usePolledData.ts index 08d13ed..884ae61 100644 --- a/frontend/src/hooks/usePolledData.ts +++ b/frontend/src/hooks/usePolledData.ts @@ -6,6 +6,7 @@ */ import { useCallback, useEffect, useRef, useState } from "react"; import { handleFetchError } from "../utils/fetchError"; +import type { FetchError } from "../types/api"; export interface UsePolledDataOptions { fetcher: (signal: AbortSignal) => Promise; @@ -20,15 +21,23 @@ export interface UsePolledDataOptions { export interface UsePolledDataResult { data: TData | null; loading: boolean; - error: string | null; + error: FetchError | null; refresh: () => void; } /** * Load a single-item response and expose refresh semantics with polling support. * + * Provides typed error handling through the `error` property, which is either + * `null` or a discriminated `FetchError` union. Use the error's `type` field + * to determine how to handle it: + * + * - `"api_error"`: Server returned HTTP error (check `status` for 401/403/50x) + * - `"network_error"`: Network, DNS, or JSON parse failure + * - `"abort_error"`: Request was cancelled (typically silently ignored by hook) + * * @param options - Configuration options - * @returns Data, loading state, error, and refresh callback + * @returns Data, loading state, typed error, and refresh callback */ export function usePolledData( options: UsePolledDataOptions, @@ -45,7 +54,7 @@ export function usePolledData( const [data, setData] = useState(initialData ?? null); const [loading, setLoading] = useState(true); - const [error, setError] = useState(null); + const [error, setError] = useState(null); const abortRef = useRef(null); const fetchRef = useRef<() => void>((): void => undefined); diff --git a/frontend/src/hooks/useRegexTester.ts b/frontend/src/hooks/useRegexTester.ts index 286aea4..a4ecb92 100644 --- a/frontend/src/hooks/useRegexTester.ts +++ b/frontend/src/hooks/useRegexTester.ts @@ -4,7 +4,7 @@ import { useCallback, useState } from "react"; import { testRegex } from "../api/config"; -import { handleFetchError } from "../utils/fetchError"; +import { handleFetchError, createStringErrorAdapter } from "../utils/fetchError"; import type { RegexTestRequest, RegexTestResponse } from "../types/config"; export interface UseRegexTesterResult { @@ -29,7 +29,7 @@ export function useRegexTester(): UseRegexTesterResult { setResult(resp); setError(null); } catch (err: unknown) { - handleFetchError(err, setError, "Regex test failed"); + handleFetchError(err, createStringErrorAdapter(setError), "Regex test failed"); } finally { setTesting(false); } diff --git a/frontend/src/hooks/useRunImport.ts b/frontend/src/hooks/useRunImport.ts index 9c918fa..8e49e6d 100644 --- a/frontend/src/hooks/useRunImport.ts +++ b/frontend/src/hooks/useRunImport.ts @@ -4,7 +4,7 @@ import { useCallback, useState } from "react"; import { runImportNow } from "../api/blocklist"; -import { handleFetchError } from "../utils/fetchError"; +import { handleFetchError, createStringErrorAdapter } from "../utils/fetchError"; import type { ImportRunResult } from "../types/blocklist"; export interface UseRunImportReturn { @@ -29,7 +29,7 @@ export function useRunImport(): UseRunImportReturn { const result = await runImportNow(); setLastResult(result); } catch (err: unknown) { - handleFetchError(err, setError, "Import failed"); + handleFetchError(err, createStringErrorAdapter(setError), "Import failed"); } finally { setRunning(false); } diff --git a/frontend/src/hooks/useSchedule.ts b/frontend/src/hooks/useSchedule.ts index aa1c5e6..68c926f 100644 --- a/frontend/src/hooks/useSchedule.ts +++ b/frontend/src/hooks/useSchedule.ts @@ -4,7 +4,7 @@ import { useCallback, useEffect, useRef, useState } from "react"; import { fetchSchedule, updateSchedule } from "../api/blocklist"; -import { handleFetchError } from "../utils/fetchError"; +import { handleFetchError, createStringErrorAdapter } from "../utils/fetchError"; import type { ScheduleConfig, ScheduleInfo } from "../types/blocklist"; export interface UseScheduleReturn { @@ -39,7 +39,7 @@ export function useSchedule(): UseScheduleReturn { }) .catch((err: unknown) => { if (controller.signal.aborted) return; - handleFetchError(err, setError, "Failed to load schedule"); + handleFetchError(err, createStringErrorAdapter(setError), "Failed to load schedule"); }) .finally(() => { if (!controller.signal.aborted) { diff --git a/frontend/src/hooks/useServerSettings.ts b/frontend/src/hooks/useServerSettings.ts index 2b43c07..d4ef8fd 100644 --- a/frontend/src/hooks/useServerSettings.ts +++ b/frontend/src/hooks/useServerSettings.ts @@ -5,7 +5,7 @@ import { useCallback, useEffect, useRef, useState } from "react"; import { fetchServerSettings, flushLogs, updateServerSettings } from "../api/server"; import { reloadConfig, restartFail2Ban } from "../api/config"; -import { handleFetchError } from "../utils/fetchError"; +import { handleFetchError, createStringErrorAdapter } from "../utils/fetchError"; import type { ServerSettings, ServerSettingsUpdate } from "../types/config"; export interface UseServerSettingsResult { @@ -43,7 +43,7 @@ export function useServerSettings(): UseServerSettingsResult { }) .catch((err: unknown) => { if (!ctrl.signal.aborted) { - handleFetchError(err, setError, "Failed to fetch server settings"); + handleFetchError(err, createStringErrorAdapter(setError), "Failed to fetch server settings"); } }) .finally(() => { diff --git a/frontend/src/hooks/useServerStatus.ts b/frontend/src/hooks/useServerStatus.ts index 0d33433..7f48cac 100644 --- a/frontend/src/hooks/useServerStatus.ts +++ b/frontend/src/hooks/useServerStatus.ts @@ -10,6 +10,7 @@ import { useCallback } from "react"; import { fetchServerStatus } from "../api/dashboard"; import { usePolledData } from "./usePolledData"; import type { ServerStatus, ServerStatusResponse } from "../types/server"; +import type { FetchError } from "../types/api"; /** How often to poll the status endpoint (milliseconds). */ const POLL_INTERVAL_MS = 30_000; @@ -21,7 +22,7 @@ export interface UseServerStatusResult { /** Whether a fetch is currently in flight. */ loading: boolean; /** Error message string when the last fetch failed, otherwise `null`. */ - error: string | null; + error: FetchError | null; /** Manually trigger a refresh immediately. */ refresh: () => void; } diff --git a/frontend/src/hooks/useTimezoneData.ts b/frontend/src/hooks/useTimezoneData.ts index c77d228..2918a79 100644 --- a/frontend/src/hooks/useTimezoneData.ts +++ b/frontend/src/hooks/useTimezoneData.ts @@ -1,6 +1,6 @@ import { useCallback, useEffect, useState } from "react"; import { fetchTimezone } from "../api/setup"; -import { handleFetchError } from "../utils/fetchError"; +import { handleFetchError, createStringErrorAdapter } from "../utils/fetchError"; export interface UseTimezoneDataResult { timezone: string; @@ -24,7 +24,7 @@ export function useTimezoneData(): UseTimezoneDataResult { setTimezone(resp.timezone); } catch (err: unknown) { if (signal?.aborted) return; - handleFetchError(err, setError, "Failed to fetch timezone"); + handleFetchError(err, createStringErrorAdapter(setError), "Failed to fetch timezone"); setTimezone("UTC"); } finally { if (!signal?.aborted) { diff --git a/frontend/src/types/api.ts b/frontend/src/types/api.ts new file mode 100644 index 0000000..dacac6b --- /dev/null +++ b/frontend/src/types/api.ts @@ -0,0 +1,106 @@ +/** + * Typed API error contracts and models. + * + * Provides discriminated error types for standardized error handling across + * API calls and hooks, enabling actionable error reporting and diagnostics. + */ + +/** + * Discriminated error type representing an HTTP API error response. + * + * Thrown when the server returns a non-2xx HTTP status code. + * Use the `type` discriminator to handle different error categories. + */ +export interface ApiErrorPayload { + type: "api_error"; + /** HTTP status code returned by the server. */ + status: number; + /** Raw response body text as returned by the server. */ + body: string; + /** User-friendly error message derived from status and body. */ + message: string; +} + +/** + * Discriminated error type representing a network error. + * + * Thrown when the request fails due to network issues (DNS lookup failure, + * connection timeout, offline, CORS error, etc.) or when parsing JSON fails. + */ +export interface NetworkErrorPayload { + type: "network_error"; + /** Underlying error message (network stack or JSON parse error). */ + message: string; +} + +/** + * Discriminated error type representing a request abort. + * + * Thrown when a fetch request is aborted (e.g., component unmounts, user cancels). + * These are expected and should typically be silently ignored. + */ +export interface AbortErrorPayload { + type: "abort_error"; + message: string; +} + +/** + * Union of all possible fetch error types. + * + * Use the `type` discriminator to narrow and handle each error case: + * + * ```ts + * const error: FetchError = ...; + * if (error.type === "api_error") { + * // Handle HTTP error — check status for 401/403/50x etc. + * if (error.status === 401) redirectToLogin(); + * } else if (error.type === "network_error") { + * // Handle network/connectivity issues + * showNetworkMessage(); + * } else if (error.type === "abort_error") { + * // Request was cancelled — typically silent + * return; + * } + * ``` + */ +export type FetchError = ApiErrorPayload | NetworkErrorPayload | AbortErrorPayload; + +/** + * Type guard to check if an error is an authentication error (401/403). + * + * @param error - The error to check (typically a FetchError) + * @returns `true` if the error is a 401 or 403 API error + */ +export function isAuthError(error: FetchError): error is ApiErrorPayload { + return error.type === "api_error" && (error.status === 401 || error.status === 403); +} + +/** + * Type guard to check if an error is an abort error (request cancelled). + * + * @param error - The error to check (typically a FetchError) + * @returns `true` if the error is an abort error + */ +export function isAbortError(error: FetchError): error is AbortErrorPayload { + return error.type === "abort_error"; +} + +/** + * Type guard to check if an error is a network error. + * + * @param error - The error to check (typically a FetchError) + * @returns `true` if the error is a network error + */ +export function isNetworkError(error: FetchError): error is NetworkErrorPayload { + return error.type === "network_error"; +} + +/** + * Type guard to check if an error is an API error. + * + * @param error - The error to check (typically a FetchError) + * @returns `true` if the error is an API error + */ +export function isApiError(error: FetchError): error is ApiErrorPayload { + return error.type === "api_error"; +} diff --git a/frontend/src/utils/__tests__/fetchError.test.ts b/frontend/src/utils/__tests__/fetchError.test.ts index 395f9bc..0f0121e 100644 --- a/frontend/src/utils/__tests__/fetchError.test.ts +++ b/frontend/src/utils/__tests__/fetchError.test.ts @@ -1,6 +1,6 @@ import { describe, expect, it, vi } from "vitest"; import { ApiError } from "../../api/client"; -import { handleFetchError } from "../fetchError"; +import { handleFetchError, createStringErrorAdapter } from "../fetchError"; describe("utils/fetchError", () => { it("ignores AbortError errors", () => { @@ -17,7 +17,7 @@ describe("utils/fetchError", () => { it("sets fallback for normal errors", () => { const setError = vi.fn(); - handleFetchError(new Error("Oops"), setError, "fallback"); + handleFetchError(new Error("Oops"), createStringErrorAdapter(setError), "fallback"); expect(setError).toHaveBeenCalledWith("Oops"); }); }); diff --git a/frontend/src/utils/fetchError.ts b/frontend/src/utils/fetchError.ts index 77408fb..34783c9 100644 --- a/frontend/src/utils/fetchError.ts +++ b/frontend/src/utils/fetchError.ts @@ -1,41 +1,142 @@ -import { isAuthError } from "../api/client"; +import type { FetchError } from "../types/api"; +import { isAuthError, isAbortError } from "../types/api"; /** - * Normalize fetch error handling across hooks. + * Extract user-friendly message from a FetchError. + * + * - abort_error → empty string (typically ignored) + * - auth_error (401/403) → empty string (handled globally) + * - api_error → HTTP status and body + * - network_error → error message + * + * @param error - The FetchError to extract message from + * @returns User-friendly error message string + */ +export function getErrorMessage(error: FetchError): string { + if (isAbortError(error)) { + return ""; + } + if (isAuthError(error)) { + return ""; + } + return error.message; +} + +/** + * Normalize typed fetch error handling across hooks. * * Handles three error cases: * 1. Request was aborted — silently ignored (expected cleanup) * 2. Auth error (401/403) — silently handled by AuthProvider (do not display) * 3. Other error — stored in component state or notified via callback * - * @param err - The caught error - * @param setError - State setter to store error message (used when no notification callback) + * Supports both typed FetchError and backward-compatible string error setters. + * + * @param err - The caught error (any value caught from Promise.catch) + * @param setError - State setter (accepts either FetchError | null or string | null) * @param fallback - Default error message if err is not an Error instance - * @param onError - Optional callback to notify of errors instead of using setError */ export function handleFetchError( err: unknown, - setError: (value: string | null) => void, + setError: (value: any) => void, fallback: string = "Unknown error", - onError?: (message: string) => void, ): void { + // Convert to FetchError + const fetchError = normalizeFetchError(err, fallback); + // Abort errors are expected during cleanup — ignore silently - if (err instanceof DOMException && err.name === "AbortError") { + if (isAbortError(fetchError)) { return; } // Auth errors are handled globally by AuthProvider — do not display locally - if (isAuthError(err)) { + if (isAuthError(fetchError)) { return; } - const message = err instanceof Error ? err.message : fallback; - - // Use notification callback if provided; otherwise use local state setter - if (onError) { - onError(message); - } else { - setError(message); - } + // Determine if setError expects FetchError or string by checking current behavior + // For now, always pass FetchError; consuming code can extract message as needed + setError(fetchError); +} + +/** + * Create a string-based error setter adapter for handleFetchError. + * + * Use this when you have a `string | null` error state but need to call handleFetchError. + * It automatically converts FetchError to displayable messages. + * + * @param setStringError - State setter that expects string | null + * @returns A setter function compatible with handleFetchError + * + * @example + * const [error, setError] = useState(null); + * handleFetchError(err, createStringErrorAdapter(setError), "Failed to load"); + */ +export function createStringErrorAdapter( + setStringError: (value: string | null) => void, +): (value: FetchError | null) => void { + return (fetchError: FetchError | null) => { + if (fetchError === null) { + setStringError(null); + return; + } + setStringError(getErrorMessage(fetchError)); + }; +} + +/** + * Normalize any caught error into a FetchError discriminated union. + * + * Handles three cases: + * 1. DOMException with name "AbortError" → abort_error + * 2. ApiError with status → api_error + * 3. Anything else → network_error + * + * @param err - The caught error value + * @param fallback - Default message for unknown errors + * @returns Typed FetchError + * @internal + */ +export function normalizeFetchError(err: unknown, fallback: string = "Unknown error"): FetchError { + // Handle abort errors + if (err instanceof DOMException && err.name === "AbortError") { + return { + type: "abort_error", + message: "Request aborted", + }; + } + + // Handle errors that are already typed objects from type guards + if (typeof err === "object" && err !== null && "type" in err) { + const fetchError = err as FetchError; + if (fetchError.type === "api_error" || fetchError.type === "network_error" || fetchError.type === "abort_error") { + return fetchError; + } + } + + // Handle ApiError instances (for backward compatibility) + if (err instanceof Error && err.name === "ApiError" && "status" in err) { + const apiError = err as any; + return { + type: "api_error", + status: apiError.status, + body: apiError.body, + message: apiError.message, + }; + } + + // Handle generic Error instances + if (err instanceof Error) { + return { + type: "network_error", + message: err.message, + }; + } + + // Fallback for unknown types + return { + type: "network_error", + message: fallback, + }; }