From 8762f86ebddb9f9226cb3a976973ac49118f0595 Mon Sep 17 00:00:00 2001 From: Valtora Date: Fri, 24 Jul 2026 13:57:51 +0000 Subject: [PATCH 1/3] fix(frontend): replace the fixed-interval backend health poller with a passive-first connectivity state machine that derives reachability from real request outcomes (via the axios interceptor), browser online/offline and visibility events, and demotes active probing to an idle fallback, eliminating false Server Unreachable alerts when a backgrounded tab is throttled during a recording --- .../src/components/MeetingControls.test.tsx | 7 +- frontend/src/components/MeetingControls.tsx | 5 +- .../components/ServiceStatusAlerts.test.tsx | 89 +++--- .../src/components/ServiceStatusAlerts.tsx | 206 ++++++------ frontend/src/lib/api/client.ts | 19 +- frontend/src/lib/connectivity/monitor.test.ts | 110 +++++++ frontend/src/lib/connectivity/monitor.ts | 292 ++++++++++++++++++ frontend/src/lib/connectivity/reducer.test.ts | 125 ++++++++ frontend/src/lib/connectivity/reducer.ts | 146 +++++++++ frontend/src/lib/serviceStatusStore.test.ts | 141 +++------ frontend/src/lib/serviceStatusStore.ts | 124 +++----- 11 files changed, 926 insertions(+), 338 deletions(-) create mode 100644 frontend/src/lib/connectivity/monitor.test.ts create mode 100644 frontend/src/lib/connectivity/monitor.ts create mode 100644 frontend/src/lib/connectivity/reducer.test.ts create mode 100644 frontend/src/lib/connectivity/reducer.ts diff --git a/frontend/src/components/MeetingControls.test.tsx b/frontend/src/components/MeetingControls.test.tsx index 7d9322f1..3149e10a 100644 --- a/frontend/src/components/MeetingControls.test.tsx +++ b/frontend/src/components/MeetingControls.test.tsx @@ -27,10 +27,9 @@ vi.mock("next/navigation", () => ({ }), })); -vi.mock("@/lib/serviceStatusStore", () => ({ - useServiceStatusStore: () => ({ - backend: true, - }), +vi.mock("@/lib/connectivity/monitor", () => ({ + useConnectivityStore: (selector: (state: { status: string }) => unknown) => + selector({ status: "online" }), })); vi.mock("@/lib/capture/CaptureProvider", () => ({ diff --git a/frontend/src/components/MeetingControls.tsx b/frontend/src/components/MeetingControls.tsx index b29ad10a..28f242b9 100644 --- a/frontend/src/components/MeetingControls.tsx +++ b/frontend/src/components/MeetingControls.tsx @@ -2,7 +2,8 @@ import { Mic } from "lucide-react"; import { useRouter } from "next/navigation"; -import { useServiceStatusStore } from "@/lib/serviceStatusStore"; +import { useConnectivityStore } from "@/lib/connectivity/monitor"; +import { isReachable } from "@/lib/connectivity/reducer"; import { useCapture } from "@/lib/capture/CaptureProvider"; import { useNotificationStore } from "@/lib/notificationStore"; @@ -27,7 +28,7 @@ export default function MeetingControls({ onMeetingEnd, variant = "sidebar", }: MeetingControlsProps) { - const { backend } = useServiceStatusStore(); + const backend = useConnectivityStore((state) => isReachable(state.status)); const { controller, finalizeRetry, diff --git a/frontend/src/components/ServiceStatusAlerts.test.tsx b/frontend/src/components/ServiceStatusAlerts.test.tsx index 4d8f715b..88234530 100644 --- a/frontend/src/components/ServiceStatusAlerts.test.tsx +++ b/frontend/src/components/ServiceStatusAlerts.test.tsx @@ -1,16 +1,18 @@ -import { act, render, waitFor } from "@testing-library/react"; +import { render, waitFor } from "@testing-library/react"; import { beforeEach, describe, expect, it, vi } from "vitest"; import ServiceStatusAlerts from "./ServiceStatusAlerts"; +import type { Reachability } from "@/lib/connectivity/reducer"; const addNotification = vi.fn(); const removeActiveNotification = vi.fn(); -const checkBackend = vi.fn().mockResolvedValue(undefined); +const refreshHealth = vi.fn().mockResolvedValue(undefined); const startPolling = vi.fn(); const stopPolling = vi.fn(); +let connectivityStatus: Reachability = "online"; + const serviceStatusState = { - backend: true, db: true, worker: true, deploymentWarnings: [] as Array<{ @@ -19,8 +21,7 @@ const serviceStatusState = { title: string; message: string; }>, - backendFailCount: 0, - checkBackend, + refreshHealth, startPolling, stopPolling, }; @@ -36,19 +37,25 @@ vi.mock("@/lib/serviceStatusStore", () => ({ useServiceStatusStore: () => serviceStatusState, })); +vi.mock("@/lib/connectivity/monitor", () => ({ + useConnectivityStore: (selector: (state: { status: Reachability }) => unknown) => + selector({ status: connectivityStatus }), + startConnectivityMonitor: vi.fn(), + stopConnectivityMonitor: vi.fn(), +})); + describe("ServiceStatusAlerts", () => { beforeEach(() => { addNotification.mockReset(); removeActiveNotification.mockReset(); - checkBackend.mockClear(); + refreshHealth.mockClear(); startPolling.mockClear(); stopPolling.mockClear(); addNotification.mockReturnValue("placeholder-toast-id"); - serviceStatusState.backend = true; + connectivityStatus = "online"; serviceStatusState.db = true; serviceStatusState.worker = true; serviceStatusState.deploymentWarnings = []; - serviceStatusState.backendFailCount = 0; }); it("creates one persistent placeholder warning toast", async () => { @@ -99,49 +106,51 @@ describe("ServiceStatusAlerts", () => { rerender(); await waitFor(() => { - expect(removeActiveNotification).toHaveBeenCalledWith( - "placeholder-toast-id", - ); + expect(removeActiveNotification).toHaveBeenCalledWith("placeholder-toast-id"); }); }); - it("keeps backend-offline and placeholder-warning notifications separate", async () => { - vi.useFakeTimers(); - addNotification - .mockReturnValueOnce("backend-toast-id") - .mockReturnValueOnce("placeholder-toast-id"); - serviceStatusState.backend = false; - serviceStatusState.backendFailCount = 2; - serviceStatusState.deploymentWarnings = [ - { - code: "placeholder_first_run_password", - key: "FIRST_RUN_PASSWORD", - title: "Placeholder bootstrap password configured", - message: "Update it.", - }, - ]; + it("raises the unreachable alert only in the unreachable state", async () => { + connectivityStatus = "unreachable"; - const { rerender } = render(); + render(); - await act(async () => { - await vi.advanceTimersByTimeAsync(5001); + await waitFor(() => { + expect(addNotification).toHaveBeenCalledWith({ + type: "error", + message: "Server Unreachable: Cannot connect to Nojoin Backend API.", + persistent: true, + }); }); + }); - serviceStatusState.backendFailCount = 3; - rerender(); + it("shows a distinct offline message when the browser is offline", async () => { + connectivityStatus = "offline"; - expect(addNotification).toHaveBeenCalledWith({ - type: "error", - message: "Server Unreachable: Cannot connect to Nojoin Backend API.", - persistent: true, + render(); + + await waitFor(() => { + expect(addNotification).toHaveBeenCalledWith({ + type: "error", + message: "You're offline. Check your network connection.", + persistent: true, + }); }); + }); + + it("does not raise a reachability alert while online or checking", async () => { + connectivityStatus = "checking"; - expect(addNotification).not.toHaveBeenCalledWith({ - type: "warning", - message: expect.stringContaining("placeholder secrets"), - persistent: true, + render(); + + await waitFor(() => { + expect(startPolling).toHaveBeenCalled(); }); - vi.useRealTimers(); + expect(addNotification).not.toHaveBeenCalledWith( + expect.objectContaining({ + message: "Server Unreachable: Cannot connect to Nojoin Backend API.", + }), + ); }); }); diff --git a/frontend/src/components/ServiceStatusAlerts.tsx b/frontend/src/components/ServiceStatusAlerts.tsx index 8f8c81ce..a16378af 100644 --- a/frontend/src/components/ServiceStatusAlerts.tsx +++ b/frontend/src/components/ServiceStatusAlerts.tsx @@ -3,151 +3,135 @@ import { useEffect, useRef } from "react"; import { useNotificationStore } from "@/lib/notificationStore"; import { useServiceStatusStore } from "@/lib/serviceStatusStore"; +import { + startConnectivityMonitor, + stopConnectivityMonitor, + useConnectivityStore, +} from "@/lib/connectivity/monitor"; +import { isReachable } from "@/lib/connectivity/reducer"; export default function ServiceStatusAlerts() { const { addNotification, removeActiveNotification } = useNotificationStore(); - const { - backend, - db, - worker, - deploymentWarnings, - backendFailCount, - checkBackend, - startPolling, - stopPolling, - } = useServiceStatusStore(); + const status = useConnectivityStore((state) => state.status); + const { db, worker, deploymentWarnings, refreshHealth, startPolling, stopPolling } = + useServiceStatusStore(); - // Track active notification IDs const notificationIds = useRef<{ [key: string]: string | null }>({ - backend: null, + reachability: null, db: null, worker: null, placeholderSecrets: null, }); const placeholderWarningSignature = useRef(""); - // Track startup grace period - const isStartupRef = useRef(true); - - useEffect(() => { - const timer = setTimeout(() => { - isStartupRef.current = false; - }, 5000); // 5 seconds grace period - return () => clearTimeout(timer); - }, []); - - // Start polling on mount + // Start reachability monitoring and health-content polling for the session. useEffect(() => { + startConnectivityMonitor(); startPolling(); - return () => stopPolling(); + return () => { + stopPolling(); + stopConnectivityMonitor(); + }; }, [startPolling, stopPolling]); + // Refresh health *content* when the tab returns to the foreground. Reachability + // has its own visibility handling inside the connectivity monitor. useEffect(() => { - const refreshStatuses = () => { - if (document.visibilityState !== "visible") { - return; + const refreshOnForeground = () => { + if (document.visibilityState === "visible") { + void refreshHealth(); } - - void checkBackend(); }; - window.addEventListener("focus", refreshStatuses); - document.addEventListener("visibilitychange", refreshStatuses); + window.addEventListener("focus", refreshOnForeground); + document.addEventListener("visibilitychange", refreshOnForeground); return () => { - window.removeEventListener("focus", refreshStatuses); - document.removeEventListener("visibilitychange", refreshStatuses); + window.removeEventListener("focus", refreshOnForeground); + document.removeEventListener("visibilitychange", refreshOnForeground); }; - }, [checkBackend]); + }, [refreshHealth]); - // Monitor Service Status useEffect(() => { - const updateNotifications = () => { - // Backend - if (!backend && !notificationIds.current.backend) { - // Shows error only after startup grace period and > 2 failures. - if (!isStartupRef.current && backendFailCount > 2) { - notificationIds.current.backend = addNotification({ - type: "error", - message: - "Server Unreachable: Cannot connect to Nojoin Backend API.", - persistent: true, - }); - } - } else if (backend && notificationIds.current.backend) { - removeActiveNotification(notificationIds.current.backend); - notificationIds.current.backend = null; - } - - // DB (only if backend is up) - if (backend && !db && !notificationIds.current.db) { - notificationIds.current.db = addNotification({ - type: "error", - message: "Database Error: Connection to PostgreSQL failed.", - persistent: true, - }); - } else if ((!backend || db) && notificationIds.current.db) { - removeActiveNotification(notificationIds.current.db); - notificationIds.current.db = null; - } + const reachable = isReachable(status); - // Worker (only if backend is up) - if (backend && !worker && !notificationIds.current.worker) { - if (!isStartupRef.current) { - notificationIds.current.worker = addNotification({ - type: "error", - message: "Worker Offline: Background processing is paused.", - persistent: true, - }); - } - } else if ((!backend || worker) && notificationIds.current.worker) { - removeActiveNotification(notificationIds.current.worker); - notificationIds.current.worker = null; - } - - const nextPlaceholderSignature = deploymentWarnings - .map((warning) => `${warning.code}:${warning.key}`) - .sort() - .join("|"); - - if (!backend || deploymentWarnings.length === 0) { - if (notificationIds.current.placeholderSecrets) { - removeActiveNotification(notificationIds.current.placeholderSecrets); - notificationIds.current.placeholderSecrets = null; - } - placeholderWarningSignature.current = ""; - return; - } + // Reachability. "Offline" (your device) and "Unreachable" (the backend) are + // deliberately different messages so users are not misdirected. + if (!reachable && !notificationIds.current.reachability) { + notificationIds.current.reachability = addNotification({ + type: "error", + message: + status === "offline" + ? "You're offline. Check your network connection." + : "Server Unreachable: Cannot connect to Nojoin Backend API.", + persistent: true, + }); + } else if (reachable && notificationIds.current.reachability) { + removeActiveNotification(notificationIds.current.reachability); + notificationIds.current.reachability = null; + } + + // DB and worker status are only meaningful while the backend is reachable. + if (reachable && !db && !notificationIds.current.db) { + notificationIds.current.db = addNotification({ + type: "error", + message: "Database Error: Connection to PostgreSQL failed.", + persistent: true, + }); + } else if ((!reachable || db) && notificationIds.current.db) { + removeActiveNotification(notificationIds.current.db); + notificationIds.current.db = null; + } + + if (reachable && !worker && !notificationIds.current.worker) { + notificationIds.current.worker = addNotification({ + type: "error", + message: "Worker Offline: Background processing is paused.", + persistent: true, + }); + } else if ((!reachable || worker) && notificationIds.current.worker) { + removeActiveNotification(notificationIds.current.worker); + notificationIds.current.worker = null; + } - if (nextPlaceholderSignature === placeholderWarningSignature.current) { - return; - } + const nextPlaceholderSignature = deploymentWarnings + .map((warning) => `${warning.code}:${warning.key}`) + .sort() + .join("|"); + if (!reachable || deploymentWarnings.length === 0) { if (notificationIds.current.placeholderSecrets) { removeActiveNotification(notificationIds.current.placeholderSecrets); + notificationIds.current.placeholderSecrets = null; } - - const affectedKeys = deploymentWarnings - .map((warning) => warning.key) - .sort() - .join(", "); - - notificationIds.current.placeholderSecrets = addNotification({ - type: "warning", - message: - `Security warning: Nojoin is using known placeholder secrets from the deployment templates (${affectedKeys}). Update .env and restart or redeploy Nojoin.`, - persistent: true, - }); - placeholderWarningSignature.current = nextPlaceholderSignature; - }; - - updateNotifications(); + placeholderWarningSignature.current = ""; + return; + } + + if (nextPlaceholderSignature === placeholderWarningSignature.current) { + return; + } + + if (notificationIds.current.placeholderSecrets) { + removeActiveNotification(notificationIds.current.placeholderSecrets); + } + + const affectedKeys = deploymentWarnings + .map((warning) => warning.key) + .sort() + .join(", "); + + notificationIds.current.placeholderSecrets = addNotification({ + type: "warning", + message: `Security warning: Nojoin is using known placeholder secrets from the deployment templates (${affectedKeys}). Update .env and restart or redeploy Nojoin.`, + persistent: true, + }); + placeholderWarningSignature.current = nextPlaceholderSignature; }, [ - backend, + status, db, worker, deploymentWarnings, - backendFailCount, addNotification, removeActiveNotification, ]); diff --git a/frontend/src/lib/api/client.ts b/frontend/src/lib/api/client.ts index 061989e3..ff506355 100644 --- a/frontend/src/lib/api/client.ts +++ b/frontend/src/lib/api/client.ts @@ -1,5 +1,7 @@ import axios from "axios"; +import { recordRequestOutcome } from "@/lib/connectivity/monitor"; + export const API_BASE_URL = process.env.NEXT_PUBLIC_API_URL ? `${process.env.NEXT_PUBLIC_API_URL}/v1` : "https://localhost:14443/api/v1"; @@ -71,8 +73,23 @@ api.interceptors.request.use((config) => { }); api.interceptors.response.use( - (response) => response, + (response) => { + // Passive health: every real response is authoritative proof the backend + // is reachable, so the connectivity monitor never needs to guess. + recordRequestOutcome({ reachedServer: true }); + return response; + }, (error) => { + // Classify the failure mode rather than collapsing everything into "down". + // A 4xx/5xx still means the server answered; only a missing response or a + // 502/503/504 gateway error indicates the backend itself is unreachable. + const status = error.response?.status; + const gateway = status === 502 || status === 503 || status === 504; + recordRequestOutcome({ + reachedServer: Boolean(error.response) && !gateway, + gateway, + }); + if ( error.response?.data && typeof error.response.data === "object" && diff --git a/frontend/src/lib/connectivity/monitor.test.ts b/frontend/src/lib/connectivity/monitor.test.ts new file mode 100644 index 00000000..57388782 --- /dev/null +++ b/frontend/src/lib/connectivity/monitor.test.ts @@ -0,0 +1,110 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; + +import { ConnectivityMonitor, type MonitorDeps } from "./monitor"; +import { + createInitialState, + PROBE_FAILURES_TO_UNREACHABLE, + reduce, + STALE_AFTER_MS, + type ConnectivityEvent, +} from "./reducer"; + +// A local reducer-backed store so the driver exercises the real state machine. +const makeHarness = (probe: () => Promise) => { + let state = createInitialState(); + const deps: MonitorDeps = { + dispatch: (event: ConnectivityEvent) => { + state = reduce(state, event); + }, + getState: () => state, + now: () => Date.now(), + probe, + setTimer: (fn, ms) => setTimeout(fn, ms) as unknown as number, + clearTimer: (handle) => clearTimeout(handle as unknown as ReturnType), + }; + return { monitor: new ConnectivityMonitor(deps), getState: () => state }; +}; + +describe("ConnectivityMonitor driver", () => { + beforeEach(() => { + vi.useFakeTimers(); + }); + afterEach(() => { + vi.useRealTimers(); + vi.restoreAllMocks(); + }); + + it("probes on start and reports online when the backend answers", async () => { + const probe = vi.fn().mockResolvedValue(true); + const { monitor, getState } = makeHarness(probe); + + monitor.start(); + await vi.advanceTimersByTimeAsync(1); + + expect(probe).toHaveBeenCalledTimes(1); + expect(getState().status).toBe("online"); + monitor.stop(); + }); + + it("does not fire synthetic probes while real traffic keeps it fresh", async () => { + const probe = vi.fn().mockResolvedValue(true); + const { monitor, getState } = makeHarness(probe); + + monitor.start(); + await vi.advanceTimersByTimeAsync(1); // initial probe (1) + + // Simulate steady real traffic every 4s for 60s (the recording case). + for (let i = 0; i < 15; i += 1) { + monitor.recordRequestOutcome({ reachedServer: true }); + await vi.advanceTimersByTimeAsync(4_000); + } + + // Only the initial probe should have run; fresh real traffic skips probing. + expect(probe).toHaveBeenCalledTimes(1); + expect(getState().status).toBe("online"); + monitor.stop(); + }); + + it("confirms unreachable only after repeated probe failures once stale", async () => { + const probe = vi.fn().mockResolvedValue(false); + const { monitor, getState } = makeHarness(probe); + + monitor.start(); + // Advance well past staleness; the confirm-cadence prober keeps failing. + await vi.advanceTimersByTimeAsync(STALE_AFTER_MS + 5_000 * PROBE_FAILURES_TO_UNREACHABLE + 100); + + expect(probe.mock.calls.length).toBeGreaterThanOrEqual(PROBE_FAILURES_TO_UNREACHABLE); + expect(getState().status).toBe("unreachable"); + monitor.stop(); + }); + + it("recovers to online once a probe succeeds again", async () => { + const probe = vi.fn().mockResolvedValue(false); + const { monitor, getState } = makeHarness(probe); + + monitor.start(); + await vi.advanceTimersByTimeAsync(STALE_AFTER_MS + 5_000 * PROBE_FAILURES_TO_UNREACHABLE + 100); + expect(getState().status).toBe("unreachable"); + + probe.mockResolvedValue(true); + await vi.advanceTimersByTimeAsync(5_000); + expect(getState().status).toBe("online"); + monitor.stop(); + }); + + it("brings a probe forward when a real request fails", async () => { + const probe = vi.fn().mockResolvedValue(true); + const { monitor, getState } = makeHarness(probe); + + monitor.start(); + await vi.advanceTimersByTimeAsync(1); + expect(probe).toHaveBeenCalledTimes(1); + + monitor.recordRequestOutcome({ reachedServer: false, gateway: true }); + await vi.advanceTimersByTimeAsync(600); // reactive probe (~500ms) + + expect(probe).toHaveBeenCalledTimes(2); + expect(getState().status).toBe("online"); + monitor.stop(); + }); +}); diff --git a/frontend/src/lib/connectivity/monitor.ts b/frontend/src/lib/connectivity/monitor.ts new file mode 100644 index 00000000..7209d79a --- /dev/null +++ b/frontend/src/lib/connectivity/monitor.ts @@ -0,0 +1,292 @@ +import { create } from "zustand"; + +import { + createInitialState, + reduce, + STALE_AFTER_MS, + type ConnectivityEvent, + type ConnectivityState, +} from "./reducer"; + +/** Cadence when we have fresh proof of life: probe lazily, only to fill idle gaps. */ +const FRESH_PROBE_INTERVAL_MS = 15_000; +/** Cadence while suspected-down: confirm the outage (or detect recovery) quickly. */ +const CONFIRM_PROBE_INTERVAL_MS = 5_000; +/** Brief delay used to bring a probe forward after a real request fails. */ +const REACTIVE_PROBE_DELAY_MS = 500; +/** Liveness-probe timeout. Deliberately generous — a slow answer is not an outage. */ +const PROBE_TIMEOUT_MS = 10_000; + +/** + * Cheap, unauthenticated, redirect-free liveness endpoint. Mirrors the axios + * client's base-URL resolution but targets `/health` (outside `/v1`). + */ +const LIVENESS_URL = `${(process.env.NEXT_PUBLIC_API_URL || "/api").replace(/\/$/, "")}/health`; + +type TimerHandle = number; + +interface ConnectivityStore extends ConnectivityState { + dispatch: (event: ConnectivityEvent) => void; +} + +export const useConnectivityStore = create((set) => ({ + ...createInitialState(), + dispatch: (event) => set((state) => reduce(state, event)), +})); + +export interface RequestOutcome { + /** True when the server produced any HTTP response (even 4xx/5xx). */ + reachedServer: boolean; + /** True for 502/503/504 — proxy up, backend not answering. */ + gateway?: boolean; +} + +export interface MonitorScheduler { + now: () => number; + setTimer: (fn: () => void, ms: number) => TimerHandle; + clearTimer: (handle: TimerHandle) => void; +} + +export interface MonitorDeps extends MonitorScheduler { + dispatch: (event: ConnectivityEvent) => void; + getState: () => ConnectivityState; + /** Runs one liveness probe; resolves true when the backend answered ok. */ + probe: () => Promise; +} + +/** + * Drives the pure reducer: schedules idle-fallback probes, reacts to real + * request outcomes and browser connectivity/visibility events. All timing and + * I/O is injected so the scheduling logic is deterministically testable. + */ +export class ConnectivityMonitor { + private timer: TimerHandle | null = null; + + private running = false; + + private readonly listeners: Array<() => void> = []; + + constructor(private readonly deps: MonitorDeps) {} + + start(): void { + if (this.running) { + return; + } + this.running = true; + this.syncEnvironment(); + this.attachBrowserListeners(); + // Defer the first tick through the scheduler so the initial probe is + // observable/controllable and start() never leaves a floating promise. + this.timer = this.deps.setTimer(() => void this.tick(), 0); + } + + stop(): void { + this.running = false; + this.clearTimer(); + this.listeners.splice(0).forEach((detach) => detach()); + } + + recordRequestOutcome(outcome: RequestOutcome): void { + const at = this.deps.now(); + if (outcome.reachedServer) { + this.deps.dispatch({ type: "request-succeeded", at }); + return; + } + + this.deps.dispatch({ + type: "request-failed", + at, + gateway: Boolean(outcome.gateway), + }); + // A real failure is our cue to verify quickly rather than wait for the + // next idle tick — but only the dedicated probe can confirm an outage. + this.bringProbeForward(); + } + + private syncEnvironment(): void { + const at = this.deps.now(); + if (typeof navigator !== "undefined" && navigator.onLine === false) { + this.deps.dispatch({ type: "browser-offline", at }); + } + if (typeof document !== "undefined") { + this.deps.dispatch({ + type: "visibility-changed", + at, + visible: document.visibilityState === "visible", + }); + } + } + + private attachBrowserListeners(): void { + if (typeof window === "undefined") { + return; + } + + const onOnline = () => { + this.deps.dispatch({ type: "browser-online", at: this.deps.now() }); + this.bringProbeForward(); + }; + const onOffline = () => { + this.deps.dispatch({ type: "browser-offline", at: this.deps.now() }); + }; + const onVisibility = () => { + const visible = + typeof document !== "undefined" && + document.visibilityState === "visible"; + this.deps.dispatch({ + type: "visibility-changed", + at: this.deps.now(), + visible, + }); + if (visible) { + // On return to the foreground, re-check — but the machine cannot alarm + // without confirmed probe failures, so a throttled wake-up is safe. + this.bringProbeForward(); + } + }; + + window.addEventListener("online", onOnline); + window.addEventListener("offline", onOffline); + document.addEventListener("visibilitychange", onVisibility); + this.listeners.push( + () => window.removeEventListener("online", onOnline), + () => window.removeEventListener("offline", onOffline), + () => document.removeEventListener("visibilitychange", onVisibility), + ); + } + + private async tick(force = false): Promise { + if (!this.running) { + return; + } + + const at = this.deps.now(); + this.deps.dispatch({ type: "evaluate", at }); + + const state = this.deps.getState(); + // Never probe a hidden or offline tab: those failures are false negatives. + const canProbe = state.browserOnline && state.visible; + // Probe when a real failure forced a verification, or (idle fallback) when + // we lack fresh proof of reachability. Fresh real traffic already answers + // the question, so an untriggered tick skips the synthetic load. + if (canProbe && (force || this.isStale(state, at))) { + await this.runProbe(); + } + + this.scheduleNext(); + } + + private isStale(state: ConnectivityState, now: number): boolean { + return ( + state.lastReachableAt === null || + now - state.lastReachableAt >= STALE_AFTER_MS + ); + } + + private async runProbe(): Promise { + let ok = false; + try { + ok = await this.deps.probe(); + } catch { + ok = false; + } + const at = this.deps.now(); + this.deps.dispatch( + ok ? { type: "probe-succeeded", at } : { type: "probe-failed", at }, + ); + } + + private scheduleNext(): void { + if (!this.running) { + return; + } + this.clearTimer(); + const status = this.deps.getState().status; + const delay = + status === "online" ? FRESH_PROBE_INTERVAL_MS : CONFIRM_PROBE_INTERVAL_MS; + this.timer = this.deps.setTimer(() => void this.tick(), delay); + } + + private bringProbeForward(): void { + if (!this.running) { + return; + } + const state = this.deps.getState(); + if (!state.browserOnline || !state.visible) { + return; + } + this.clearTimer(); + // Force the probe: a real failure is a live signal something may be wrong + // right now, so verify even if a recent success still looks "fresh". + this.timer = this.deps.setTimer(() => void this.tick(true), REACTIVE_PROBE_DELAY_MS); + } + + private clearTimer(): void { + if (this.timer !== null) { + this.deps.clearTimer(this.timer); + this.timer = null; + } + } +} + +const defaultProbe = async (): Promise => { + if (typeof fetch === "undefined") { + return false; + } + const controller = new AbortController(); + const timeout = setTimeout(() => controller.abort(), PROBE_TIMEOUT_MS); + try { + const response = await fetch(LIVENESS_URL, { + method: "GET", + cache: "no-store", + credentials: "omit", + signal: controller.signal, + }); + return response.ok; + } catch { + return false; + } finally { + clearTimeout(timeout); + } +}; + +let monitor: ConnectivityMonitor | null = null; + +const getMonitor = (): ConnectivityMonitor => { + if (!monitor) { + monitor = new ConnectivityMonitor({ + dispatch: (event) => useConnectivityStore.getState().dispatch(event), + getState: () => useConnectivityStore.getState(), + now: () => Date.now(), + probe: defaultProbe, + setTimer: (fn, ms) => setTimeout(fn, ms) as unknown as TimerHandle, + clearTimer: (handle) => clearTimeout(handle as unknown as ReturnType), + }); + } + return monitor; +}; + +export const startConnectivityMonitor = (): void => { + if (typeof window === "undefined") { + return; + } + getMonitor().start(); +}; + +export const stopConnectivityMonitor = (): void => { + if (typeof window === "undefined") { + return; + } + getMonitor().stop(); +}; + +/** + * Feeds one observed request outcome into the monitor. Called from the single + * axios response interceptor so every real request updates connectivity. + */ +export const recordRequestOutcome = (outcome: RequestOutcome): void => { + if (typeof window === "undefined") { + return; + } + getMonitor().recordRequestOutcome(outcome); +}; diff --git a/frontend/src/lib/connectivity/reducer.test.ts b/frontend/src/lib/connectivity/reducer.test.ts new file mode 100644 index 00000000..c56d7d4c --- /dev/null +++ b/frontend/src/lib/connectivity/reducer.test.ts @@ -0,0 +1,125 @@ +import { describe, expect, it } from "vitest"; + +import { + createInitialState, + isReachable, + PROBE_FAILURES_TO_UNREACHABLE, + reduce, + STALE_AFTER_MS, + type ConnectivityEvent, + type ConnectivityState, +} from "./reducer"; + +const T0 = 1_000_000; // arbitrary fixed clock; the reducer never reads a real clock + +const run = ( + events: ConnectivityEvent[], + start: ConnectivityState = createInitialState(), +): ConnectivityState => events.reduce(reduce, start); + +describe("connectivity reducer", () => { + it("starts optimistic-online so a fresh load never flashes a false alarm", () => { + const state = createInitialState(); + expect(state.status).toBe("online"); + expect(isReachable(state.status)).toBe(true); + }); + + it("treats any server response as proof of life", () => { + const state = run([{ type: "request-succeeded", at: T0 }]); + expect(state.status).toBe("online"); + expect(state.lastReachableAt).toBe(T0); + expect(state.probeFailureStreak).toBe(0); + }); + + it("goes stale to `checking` (never straight to unreachable) after the freshness window", () => { + const state = run([ + { type: "request-succeeded", at: T0 }, + { type: "evaluate", at: T0 + STALE_AFTER_MS + 1 }, + ]); + expect(state.status).toBe("checking"); + expect(isReachable(state.status)).toBe(true); // still usable, just verifying + }); + + it("escalates to `unreachable` only after enough probe failures while stale", () => { + const events: ConnectivityEvent[] = [{ type: "request-succeeded", at: T0 }]; + for (let i = 0; i < PROBE_FAILURES_TO_UNREACHABLE; i += 1) { + events.push({ type: "probe-failed", at: T0 + STALE_AFTER_MS + 1 + i }); + } + const state = run(events); + expect(state.probeFailureStreak).toBe(PROBE_FAILURES_TO_UNREACHABLE); + expect(state.status).toBe("unreachable"); + }); + + it("does not alarm at exactly one below the threshold", () => { + const events: ConnectivityEvent[] = [{ type: "request-succeeded", at: T0 }]; + for (let i = 0; i < PROBE_FAILURES_TO_UNREACHABLE - 1; i += 1) { + events.push({ type: "probe-failed", at: T0 + STALE_AFTER_MS + 1 + i }); + } + const state = run(events); + expect(state.status).toBe("checking"); + }); + + it("recovers instantly on any success after being unreachable", () => { + const events: ConnectivityEvent[] = [{ type: "request-succeeded", at: T0 }]; + for (let i = 0; i < PROBE_FAILURES_TO_UNREACHABLE; i += 1) { + events.push({ type: "probe-failed", at: T0 + STALE_AFTER_MS + 1 + i }); + } + events.push({ type: "probe-succeeded", at: T0 + 100_000 }); + const state = run(events); + expect(state.status).toBe("online"); + expect(state.probeFailureStreak).toBe(0); + }); + + it("browser-offline is its own state, distinct from unreachable", () => { + const state = run([{ type: "browser-offline", at: T0 }]); + expect(state.status).toBe("offline"); + expect(isReachable(state.status)).toBe(false); + }); + + it("a successful request overrides a stale offline flag (we clearly are online)", () => { + const state = run([ + { type: "browser-offline", at: T0 }, + { type: "request-succeeded", at: T0 + 10 }, + ]); + expect(state.status).toBe("online"); + expect(state.browserOnline).toBe(true); + }); + + // The incident: during a recording, segment uploads keep succeeding, so the + // machine must stay online across a long span even with an interleaved miss. + it("stays online while real traffic keeps succeeding (the recording case)", () => { + const events: ConnectivityEvent[] = []; + for (let t = 0; t <= 300_000; t += 4_000) { + // A single blocked socket in the middle must not matter. + events.push( + t === 120_000 + ? { type: "request-failed", at: T0 + t, gateway: false } + : { type: "request-succeeded", at: T0 + t }, + ); + } + const state = run(events); + expect(state.status).toBe("online"); + }); + + // The wake-up burst: a hidden→visible flip plus a couple of failed real + // requests must NOT alarm without confirmed probe failures. + it("a visibility flip with failed requests does not alarm without probe failures", () => { + const state = run([ + { type: "request-succeeded", at: T0 }, + { type: "visibility-changed", at: T0 + 60_000, visible: false }, + { type: "visibility-changed", at: T0 + 90_000, visible: true }, + { type: "request-failed", at: T0 + 90_001, gateway: false }, + { type: "request-failed", at: T0 + 90_002, gateway: true }, + ]); + expect(state.status).toBe("checking"); + expect(state.status).not.toBe("unreachable"); + }); + + it("never probes a hidden tab into unreachable (probe failures still counted only when driver runs them)", () => { + // Sanity: visibility does not itself change reachability derivation. + const visible = run([{ type: "visibility-changed", at: T0, visible: true }]); + const hidden = run([{ type: "visibility-changed", at: T0, visible: false }]); + expect(visible.status).toBe("online"); + expect(hidden.status).toBe("online"); + }); +}); diff --git a/frontend/src/lib/connectivity/reducer.ts b/frontend/src/lib/connectivity/reducer.ts new file mode 100644 index 00000000..a5d7cdc6 --- /dev/null +++ b/frontend/src/lib/connectivity/reducer.ts @@ -0,0 +1,146 @@ +/** + * Pure state machine for backend connectivity. + * + * The previous design equated "a synthetic health probe failed" with "the + * backend is down". That is a category error: a throttled background tab, a + * saturated connection pool, a sleeping laptop or a dropped link all fail + * probes while saying nothing about the server. This machine instead treats + * every observed success — a real API response, a health-content poll, or an + * idle liveness probe — as authoritative proof of reachability, and only + * escalates to `unreachable` once the browser is online, no success has been + * seen for a while, AND a dedicated probe has failed repeatedly. + * + * Reachability is a derived function of the inputs, so the reducer is a pure + * function of (state, event) and is exhaustively unit-testable without mocking + * timers or the network. All timing lives in the driver (monitor.ts). + */ + +export type Reachability = "online" | "checking" | "offline" | "unreachable"; + +export interface ConnectivityState { + status: Reachability; + /** Timestamp (ms) of the last proof the backend answered. */ + lastReachableAt: number | null; + /** Consecutive dedicated-probe failures; only the prober confirms outages. */ + probeFailureStreak: number; + /** navigator.onLine, tracked via the browser's online/offline events. */ + browserOnline: boolean; + /** document.visibilityState === "visible"; hidden tabs are not probed. */ + visible: boolean; +} + +export type ConnectivityEvent = + | { type: "request-succeeded"; at: number } + | { type: "request-failed"; at: number; gateway: boolean } + | { type: "probe-succeeded"; at: number } + | { type: "probe-failed"; at: number } + | { type: "browser-online"; at: number } + | { type: "browser-offline"; at: number } + | { type: "visibility-changed"; at: number; visible: boolean } + | { type: "evaluate"; at: number }; + +/** + * A success (real traffic or probe) keeps the machine `online` for this long + * before staleness forces a re-check. Longer than any single poll interval so + * one missed beat never destales a healthy backend. + */ +export const STALE_AFTER_MS = 30_000; + +/** Dedicated-probe failures required, once stale, to declare `unreachable`. */ +export const PROBE_FAILURES_TO_UNREACHABLE = 3; + +export const createInitialState = (): ConnectivityState => ({ + // Optimistic: assume reachable until the first probe establishes ground + // truth, so a fresh load never flashes a false alarm. + status: "online", + lastReachableAt: null, + probeFailureStreak: 0, + browserOnline: true, + visible: true, +}); + +const deriveStatus = ( + state: Omit, + now: number, +): Reachability => { + if (!state.browserOnline) { + return "offline"; + } + + const reachableRecently = + state.lastReachableAt !== null && + now - state.lastReachableAt < STALE_AFTER_MS; + + // Before any probe has run and with no failures observed, stay optimistic. + const optimisticStartup = + state.lastReachableAt === null && state.probeFailureStreak === 0; + + if (reachableRecently || optimisticStartup) { + return "online"; + } + + return state.probeFailureStreak >= PROBE_FAILURES_TO_UNREACHABLE + ? "unreachable" + : "checking"; +}; + +const withStatus = ( + state: Omit, + now: number, +): ConnectivityState => ({ ...state, status: deriveStatus(state, now) }); + +export const reduce = ( + state: ConnectivityState, + event: ConnectivityEvent, +): ConnectivityState => { + switch (event.type) { + case "request-succeeded": + case "probe-succeeded": + // Any answer from the server is proof of life; clear the failure streak + // and refresh the reachability clock. A success also means we are online + // regardless of a stale navigator.onLine flag. + return withStatus( + { + ...state, + browserOnline: true, + lastReachableAt: event.at, + probeFailureStreak: 0, + }, + event.at, + ); + + case "probe-failed": + // Only the dedicated prober increments the confirmation streak. + return withStatus( + { ...state, probeFailureStreak: state.probeFailureStreak + 1 }, + event.at, + ); + + case "request-failed": + // A real request without a server response is ambiguous (timeout, tab + // throttle, one blocked socket). It never alarms on its own — it only + // lets time advance so the driver can schedule a confirming probe. + return withStatus(state, event.at); + + case "browser-offline": + return { ...state, browserOnline: false, status: "offline" }; + + case "browser-online": + return withStatus({ ...state, browserOnline: true }, event.at); + + case "visibility-changed": + return withStatus({ ...state, visible: event.visible }, event.at); + + case "evaluate": + return withStatus(state, event.at); + + default: { + const _exhaustive: never = event; + return _exhaustive; + } + } +}; + +/** True when the backend should be treated as usable by the UI. */ +export const isReachable = (status: Reachability): boolean => + status === "online" || status === "checking"; diff --git a/frontend/src/lib/serviceStatusStore.test.ts b/frontend/src/lib/serviceStatusStore.test.ts index c37d1ad2..08c3b742 100644 --- a/frontend/src/lib/serviceStatusStore.test.ts +++ b/frontend/src/lib/serviceStatusStore.test.ts @@ -1,47 +1,48 @@ import { beforeEach, describe, expect, it, vi } from "vitest"; -describe("serviceStatusStore backend health", () => { +const recordRequestOutcome = vi.fn(); + +vi.mock("@/lib/connectivity/monitor", () => ({ + recordRequestOutcome: (...args: unknown[]) => recordRequestOutcome(...args), +})); + +describe("serviceStatusStore health content", () => { beforeEach(() => { vi.resetModules(); vi.restoreAllMocks(); + recordRequestOutcome.mockReset(); }); - it("uses the detailed health endpoint when it is available", async () => { - const fetchMock = vi - .fn() - .mockResolvedValueOnce({ - ok: true, - status: 200, - json: async () => ({ - version: "1.2.3", - deployment_warnings: [ - { - code: "placeholder_first_run_password", - key: "FIRST_RUN_PASSWORD", - title: "Placeholder bootstrap password configured", - message: "Update it.", - }, - ], - components: { - db: "connected", - worker: "inactive", + it("populates db/worker/version/warnings from the detailed health endpoint", async () => { + const fetchMock = vi.fn().mockResolvedValueOnce({ + ok: true, + status: 200, + json: async () => ({ + version: "1.2.3", + deployment_warnings: [ + { + code: "placeholder_first_run_password", + key: "FIRST_RUN_PASSWORD", + title: "Placeholder bootstrap password configured", + message: "Update it.", }, - }), - } as Response) - .mockResolvedValueOnce({ - ok: true, - status: 200, - } as Response); + ], + components: { + db: "connected", + worker: "inactive", + }, + }), + } as Response); vi.stubGlobal("fetch", fetchMock); const { useServiceStatusStore } = await import("./serviceStatusStore"); - - await useServiceStatusStore.getState().checkBackend(); + await useServiceStatusStore.getState().refreshHealth(); expect(fetchMock).toHaveBeenCalledTimes(1); + // Reaching the endpoint at all reports reachability to the monitor. + expect(recordRequestOutcome).toHaveBeenCalledWith({ reachedServer: true }); expect(useServiceStatusStore.getState()).toMatchObject({ - backend: true, db: true, worker: false, backendVersion: "1.2.3", @@ -53,36 +54,10 @@ describe("serviceStatusStore backend health", () => { message: "Update it.", }, ], - backendFailCount: 0, }); }); - it("falls back to the public health probe when the detailed endpoint fails", async () => { - const fetchMock = vi - .fn() - .mockRejectedValueOnce(new TypeError("Failed to fetch")) - .mockResolvedValueOnce({ - ok: true, - status: 200, - } as Response); - - vi.stubGlobal("fetch", fetchMock); - - const { useServiceStatusStore } = await import("./serviceStatusStore"); - - await useServiceStatusStore.getState().checkBackend(); - - expect(fetchMock).toHaveBeenCalledTimes(2); - expect(useServiceStatusStore.getState()).toMatchObject({ - backend: true, - db: true, - worker: true, - deploymentWarnings: [], - backendFailCount: 0, - }); - }); - - it("does not probe or accrue failures while the tab is hidden", async () => { + it("does not fetch while the tab is hidden", async () => { const fetchMock = vi.fn(); vi.stubGlobal("fetch", fetchMock); @@ -93,17 +68,10 @@ describe("serviceStatusStore backend health", () => { try { const { useServiceStatusStore } = await import("./serviceStatusStore"); + await useServiceStatusStore.getState().refreshHealth(); - await useServiceStatusStore.getState().checkBackend(); - - // A backgrounded tab is throttled by the browser, so a timed-out probe - // would be a false negative. We must not fetch, and must leave the - // previously-known-good status untouched. expect(fetchMock).not.toHaveBeenCalled(); - expect(useServiceStatusStore.getState()).toMatchObject({ - backend: true, - backendFailCount: 0, - }); + expect(recordRequestOutcome).not.toHaveBeenCalled(); } finally { Object.defineProperty(document, "visibilityState", { configurable: true, @@ -112,7 +80,7 @@ describe("serviceStatusStore backend health", () => { } }); - it("preserves deployment warnings when falling back to the public health probe", async () => { + it("reports a transport failure to the monitor and keeps last-known content", async () => { const fetchMock = vi .fn() .mockResolvedValueOnce({ @@ -120,40 +88,25 @@ describe("serviceStatusStore backend health", () => { status: 200, json: async () => ({ version: "1.2.3", - deployment_warnings: [ - { - code: "placeholder_data_encryption_key", - key: "DATA_ENCRYPTION_KEY", - title: "Placeholder data encryption key configured", - message: "Update it.", - }, - ], - components: { - db: "connected", - worker: "active", - }, + deployment_warnings: [], + components: { db: "connected", worker: "active" }, }), } as Response) - .mockRejectedValueOnce(new TypeError("Failed to fetch")) - .mockResolvedValueOnce({ - ok: true, - status: 200, - } as Response); + .mockRejectedValueOnce(new TypeError("Failed to fetch")); vi.stubGlobal("fetch", fetchMock); const { useServiceStatusStore } = await import("./serviceStatusStore"); + await useServiceStatusStore.getState().refreshHealth(); + await useServiceStatusStore.getState().refreshHealth(); - await useServiceStatusStore.getState().checkBackend(); - await useServiceStatusStore.getState().checkBackend(); - - expect(useServiceStatusStore.getState().deploymentWarnings).toEqual([ - { - code: "placeholder_data_encryption_key", - key: "DATA_ENCRYPTION_KEY", - title: "Placeholder data encryption key configured", - message: "Update it.", - }, - ]); + expect(recordRequestOutcome).toHaveBeenCalledWith({ reachedServer: true }); + expect(recordRequestOutcome).toHaveBeenCalledWith({ reachedServer: false }); + // Content from the last good poll is preserved, not wiped to a false state. + expect(useServiceStatusStore.getState()).toMatchObject({ + db: true, + worker: true, + backendVersion: "1.2.3", + }); }); }); diff --git a/frontend/src/lib/serviceStatusStore.ts b/frontend/src/lib/serviceStatusStore.ts index 37269b13..8ce67834 100644 --- a/frontend/src/lib/serviceStatusStore.ts +++ b/frontend/src/lib/serviceStatusStore.ts @@ -1,5 +1,6 @@ import { create } from "zustand"; import type { DeploymentWarning } from "@/types"; +import { recordRequestOutcome } from "@/lib/connectivity/monitor"; interface DetailedHealthStatus { status: string; @@ -11,85 +12,63 @@ interface DetailedHealthStatus { }; } +/** + * Health *content* only: the subsystem status the backend reports about itself + * (db, worker, version, deployment warnings). Backend *reachability* is a + * separate concern owned by the connectivity monitor — a failure to load this + * content must never assert "unreachable", it only means the content is stale. + */ interface ServiceStatusState { - backend: boolean; db: boolean; worker: boolean; backendVersion: string | null; deploymentWarnings: DeploymentWarning[]; isPolling: boolean; - backendFailCount: number; - checkBackend: () => Promise; + refreshHealth: () => Promise; startPolling: () => void; stopPolling: () => void; } -const BACKOFF_DELAYS = [1000, 2000, 4000, 8000, 16000, 32000, 60000]; -const NORMAL_INTERVAL = 10000; -const BACKEND_REQUEST_TIMEOUT_MS = 5000; +const HEALTH_POLL_INTERVAL_MS = 15_000; +const HEALTH_REQUEST_TIMEOUT_MS = 8_000; export const useServiceStatusStore = create((set, get) => { - let backendTimer: ReturnType | null = null; + let healthTimer: ReturnType | null = null; - const clearBackendTimer = () => { - if (!backendTimer) { - return; + const clearHealthTimer = () => { + if (healthTimer) { + clearTimeout(healthTimer); + healthTimer = null; } - - clearTimeout(backendTimer); - backendTimer = null; }; - const scheduleNextBackend = () => { + const scheduleNext = () => { if (!get().isPolling) { return; } - - const failCount = get().backendFailCount; - const delay = - failCount === 0 - ? NORMAL_INTERVAL - : BACKOFF_DELAYS[Math.min(failCount - 1, BACKOFF_DELAYS.length - 1)]; - - clearBackendTimer(); - backendTimer = setTimeout(() => { - void get().checkBackend(); - }, delay); - }; - - const markBackendUnavailable = () => { - set((state) => ({ - backend: false, - db: false, - worker: false, - deploymentWarnings: [], - backendFailCount: state.backendFailCount + 1, - })); + clearHealthTimer(); + healthTimer = setTimeout( + () => void get().refreshHealth(), + HEALTH_POLL_INTERVAL_MS, + ); }; return { - backend: true, db: true, worker: true, backendVersion: null, deploymentWarnings: [], isPolling: false, - backendFailCount: 0, - - checkBackend: async () => { - // A hidden/backgrounded tab has its timers throttled and network - // requests deprioritised by the browser, so the health probe can time - // out even while the backend is perfectly healthy (active recording - // uploads keep succeeding throughout). Skip probing while hidden so we - // never accrue failures — and never raise a false "Server Unreachable" - // alert — during a call the user is watching in another window. The - // visibilitychange handler in ServiceStatusAlerts re-checks immediately - // on return to the foreground. + + refreshHealth: async () => { + // A hidden tab throttles timers and deprioritises fetches, so a probe can + // falsely fail; stale content for a backgrounded tab is harmless. Skip + // while hidden — the connectivity monitor owns reachability separately. if ( typeof document !== "undefined" && document.visibilityState === "hidden" ) { - scheduleNextBackend(); + scheduleNext(); return; } @@ -102,7 +81,7 @@ export const useServiceStatusStore = create((set, get) => { const controller = new AbortController(); const timeoutId = setTimeout( () => controller.abort(), - BACKEND_REQUEST_TIMEOUT_MS, + HEALTH_REQUEST_TIMEOUT_MS, ); const response = await fetch(`${apiBaseUrl}/v1/system/health`, { @@ -113,53 +92,26 @@ export const useServiceStatusStore = create((set, get) => { clearTimeout(timeoutId); + // Any HTTP answer — even 401/500 — proves the backend is reachable, + // independent of whether the payload was usable. + recordRequestOutcome({ reachedServer: true }); + if (response.ok) { const data: DetailedHealthStatus = await response.json(); set({ - backend: true, db: data.components.db === "connected", worker: data.components.worker === "active", backendVersion: data.version, deploymentWarnings: data.deployment_warnings, - backendFailCount: 0, - }); - scheduleNextBackend(); - return; - } - } catch { - // Fall back to the unauthenticated probe to distinguish backend - // outages from failures in the richer health endpoint. - } - - try { - const controller = new AbortController(); - const timeoutId = setTimeout( - () => controller.abort(), - BACKEND_REQUEST_TIMEOUT_MS, - ); - - const response = await fetch(`${apiBaseUrl}/health`, { - signal: controller.signal, - method: "GET", - }); - - clearTimeout(timeoutId); - - if (response.ok) { - set({ - backend: true, - db: true, - worker: true, - backendFailCount: 0, }); - } else { - markBackendUnavailable(); } } catch { - markBackendUnavailable(); + // Transport failure: hand it to the connectivity monitor, which will + // confirm with a dedicated probe. Keep the last-known content as-is. + recordRequestOutcome({ reachedServer: false }); } - scheduleNextBackend(); + scheduleNext(); }, startPolling: () => { @@ -168,12 +120,12 @@ export const useServiceStatusStore = create((set, get) => { } set({ isPolling: true }); - void get().checkBackend(); + void get().refreshHealth(); }, stopPolling: () => { set({ isPolling: false }); - clearBackendTimer(); + clearHealthTimer(); }, }; }); From 20266f1ffc9051806e38bad6e912c9660e4caf20 Mon Sep 17 00:00:00 2001 From: Valtora Date: Fri, 24 Jul 2026 13:57:51 +0000 Subject: [PATCH 2/3] fix(api): exempt the /health and /api/health liveness endpoints from EnforceCanonicalHttpsMiddleware so container health checks and plain-HTTP uptime monitors get a clean 200 instead of a canonical-HTTPS redirect, and repoint the middleware tests at a guarded path --- backend/main.py | 9 ++++++ backend/tests/test_security_surface.py | 41 ++++++++++++++++++++------ 2 files changed, 41 insertions(+), 9 deletions(-) diff --git a/backend/main.py b/backend/main.py index 31b41295..1871886a 100644 --- a/backend/main.py +++ b/backend/main.py @@ -62,7 +62,16 @@ def _is_private_client_address(host: str | None) -> bool: class EnforceCanonicalHttpsMiddleware(BaseHTTPMiddleware): + # Liveness endpoints must answer over plain HTTP: container/orchestrator + # health checks and internal uptime monitors call them directly, without the + # X-Forwarded-Proto headers that browser traffic carries. Redirecting them + # breaks those checks and, for the frontend probe, defeats the fallback. + _HEALTH_PATHS = frozenset({"/health", "/api/health"}) + async def dispatch(self, request: Request, call_next): + if request.url.path in self._HEALTH_PATHS: + return await call_next(request) + if self._is_request_https(request): return await call_next(request) diff --git a/backend/tests/test_security_surface.py b/backend/tests/test_security_surface.py index c2f67dc0..e531d7bf 100644 --- a/backend/tests/test_security_surface.py +++ b/backend/tests/test_security_surface.py @@ -929,11 +929,14 @@ async def test_safe_http_requests_redirect_to_canonical_https_origin( transport=ASGITransport(app=app), base_url="http://nojoin.example.com", ) as client: - response = await client.get("/api/health?probe=1") + # Health paths are exempt from HTTPS enforcement, so use a guarded path + # to prove plain-HTTP safe requests still redirect to canonical HTTPS. + response = await client.get("/api/v1/system/health?probe=1") assert response.status_code == 307 assert ( - response.headers["location"] == "https://nojoin.example.com/api/health?probe=1" + response.headers["location"] + == "https://nojoin.example.com/api/v1/system/health?probe=1" ) @@ -946,7 +949,8 @@ async def test_unsafe_http_requests_are_rejected(monkeypatch) -> None: transport=ASGITransport(app=app), base_url="http://nojoin.example.com", ) as client: - response = await client.post("/api/health") + # A guarded path (health is exempt) proves unsafe plain-HTTP is rejected. + response = await client.post("/api/v1/system/health") assert response.status_code == 400 assert response.json() == { @@ -963,16 +967,18 @@ async def test_forwarded_https_proxy_requests_are_accepted(monkeypatch) -> None: transport=ASGITransport(app=app), base_url="http://nojoin.example.com", ) as client: + # Health paths bypass HTTPS enforcement, so exercise the proxy-scheme + # gate with a guarded endpoint: a forwarded-https request is accepted + # (reaches auth and 401s) rather than being redirected to canonical HTTPS. response = await client.get( - "/api/health", + "/api/v1/system/health", headers={ "X-Forwarded-Proto": "https", "X-Forwarded-Host": "nojoin.example.com", }, ) - assert response.status_code == 200 - assert response.json() == {"status": "ok"} + assert response.status_code == 401 @pytest.mark.anyio @@ -986,16 +992,33 @@ async def test_forwarded_https_proxy_requests_accept_host_headers_with_ports( transport=ASGITransport(app=app), base_url="http://localhost:14443", ) as client: + # Health paths bypass HTTPS enforcement; use a guarded endpoint to prove + # the host-with-port normalisation still accepts the forwarded request. response = await client.get( - "/api/health", + "/api/v1/system/health", headers={ "X-Forwarded-Proto": "https", "X-Forwarded-Host": "localhost:14443", }, ) - assert response.status_code == 200 - assert response.json() == {"status": "ok"} + assert response.status_code == 401 + + +@pytest.mark.anyio +async def test_liveness_endpoints_bypass_https_enforcement() -> None: + # Container/orchestrator probes and internal uptime monitors call the + # liveness endpoints directly over plain HTTP, without forwarded-proto + # headers. They must answer 200 rather than being redirected to HTTPS. + app, _ = _build_app(initialized=True) + + async with AsyncClient( + transport=ASGITransport(app=app), base_url="http://test" + ) as client: + for path in ("/health", "/api/health"): + response = await client.get(path) + assert response.status_code == 200, path + assert response.json() == {"status": "ok"} @pytest.mark.anyio From a8e75aea7cb26d7d143b738305919f59c2fd9061 Mon Sep 17 00:00:00 2001 From: Valtora Date: Fri, 24 Jul 2026 14:21:16 +0000 Subject: [PATCH 3/3] fix(deploy): correct the nginx HTTP to HTTPS redirect that pinned the HTTPS target to the plain-HTTP port ($server_port resolves to 80 on the listen 80 block), producing a malformed https://$host:80 location; redirect to standard HTTPS via $host so plain-HTTP health checks and uptime monitors get a well-formed target --- nginx/nginx.conf | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/nginx/nginx.conf b/nginx/nginx.conf index 6c68cd40..fd5ba379 100644 --- a/nginx/nginx.conf +++ b/nginx/nginx.conf @@ -9,8 +9,10 @@ http { server { listen 80; server_name localhost; - # Redirect HTTP to HTTPS dynamically using the server's port - return 301 https://$host:$server_port$request_uri; + # Redirect HTTP to HTTPS. Do not append $server_port: on the :80 listener + # it resolves to 80, producing a malformed https://$host:80/... target. + # External access arrives over standard HTTPS (443) via the reverse proxy. + return 301 https://$host$request_uri; } server {