From bf380fab133a606f6e4d837be72444d6b3bfa3e3 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 5 May 2026 01:50:40 +0000 Subject: [PATCH 1/3] feat(admin): sse realtime updates for /admin/errors (Epic #616 Phase 2 / #807) - add in-memory `apiErrorBroadcaster` for fan-out of `api_errors` updates - expose `GET /api/admin/errors/stream` (text/event-stream, adminRequired) with initial `ready` event, periodic keep-alive comments, and subscriber cap - publish from Sentry webhook, GitHub AI callback, and admin PATCH so the UI receives push updates from every mutation path - switch `useApiErrors` to subscribe via `EventSource` with reconnect/cleanup; fallback polling kicks in only when the stream is down - tests: broadcaster unit, SSE route headers + push delivery, hook merge/filter Closes #807 --- admin/src/pages/errors/useApiErrors.test.ts | 222 ++++++++++++++++++ admin/src/pages/errors/useApiErrors.ts | 203 ++++++++++++++-- .../src/__tests__/routes/admin/errors.test.ts | 96 +++++++- .../services/apiErrorBroadcaster.test.ts | 111 +++++++++ server/api/src/routes/admin/errors.ts | 131 ++++++++++- .../src/routes/webhooks/githubAiCallback.ts | 4 + server/api/src/routes/webhooks/sentry.ts | 6 + .../api/src/services/apiErrorBroadcaster.ts | 117 +++++++++ 8 files changed, 869 insertions(+), 21 deletions(-) create mode 100644 admin/src/pages/errors/useApiErrors.test.ts create mode 100644 server/api/src/__tests__/services/apiErrorBroadcaster.test.ts create mode 100644 server/api/src/services/apiErrorBroadcaster.ts diff --git a/admin/src/pages/errors/useApiErrors.test.ts b/admin/src/pages/errors/useApiErrors.test.ts new file mode 100644 index 00000000..0e2d9138 --- /dev/null +++ b/admin/src/pages/errors/useApiErrors.test.ts @@ -0,0 +1,222 @@ +/** + * `useApiErrors` の単体テスト。 + * SSE で push された行を初期取得結果にマージできること、フィルタ非該当な行を + * 無視すること、アンマウント時に EventSource を close することを検証する。 + * + * Unit tests for `useApiErrors`. Verifies that pushed SSE rows merge into the + * REST-bootstrapped list, filter mismatches are ignored, and the EventSource + * is closed on unmount (no fd leaks). + * + * @see ./useApiErrors.ts + * @see https://github.com/otomatty/zedi/issues/807 + */ +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; +import { act, renderHook, waitFor } from "@testing-library/react"; +import type { ApiErrorRow, GetApiErrorsResponse } from "@/api/admin"; + +// `getApiErrors` を mock してネットワーク呼び出しを避ける。 +// Mock the REST helper so the hook's bootstrap fetch returns a fixed payload. +vi.mock("@/api/admin", async (orig) => { + const actual = await orig(); + return { + ...actual, + getApiErrors: vi.fn(), + }; +}); + +const { getApiErrors } = await import("@/api/admin"); +const { useApiErrors } = await import("./useApiErrors"); + +type Listener = (event: MessageEvent) => void; + +interface FakeEventSource { + url: string; + withCredentials: boolean; + closed: boolean; + close: () => void; + onerror: ((ev: Event) => void) | null; + addEventListener: (type: string, cb: Listener) => void; + dispatch: (type: string, payload: unknown) => void; +} + +let lastInstance: FakeEventSource | null = null; + +function createFakeEventSourceCtor(): typeof EventSource { + // テスト用の最小限の EventSource 互換 stub。`new` で生成された各インスタンスを + // `lastInstance` に保存し、テスト本文から `dispatch` で擬似イベントを送れるようにする。 + // Minimal EventSource stub. Each new instance is stashed in `lastInstance` so + // the test body can dispatch events imperatively without touching `this`. + function build(url: string, init?: { withCredentials?: boolean }): FakeEventSource { + const listeners = new Map>(); + const instance: FakeEventSource = { + url, + withCredentials: init?.withCredentials ?? false, + closed: false, + onerror: null, + close: () => { + instance.closed = true; + }, + addEventListener: (type, cb) => { + let set = listeners.get(type); + if (!set) { + set = new Set(); + listeners.set(type, set); + } + set.add(cb); + }, + dispatch: (type, payload) => { + const event = { + type, + data: typeof payload === "string" ? payload : JSON.stringify(payload), + } as MessageEvent; + listeners.get(type)?.forEach((cb) => cb(event)); + }, + }; + lastInstance = instance; + return instance; + } + // EventSource は `new` で呼ばれるので、関数を `new` 互換に見せかけるラッパで返す。 + // EventSource is constructor-called; wrap `build` so `new EventSource(...)` + // forwards to it. + return function EventSourceCtor(url: string, init?: { withCredentials?: boolean }) { + return build(url, init); + } as unknown as typeof EventSource; +} + +function getInstance(): FakeEventSource { + if (!lastInstance) throw new Error("EventSource has not been instantiated"); + return lastInstance; +} + +const FIXED_RESPONSE: GetApiErrorsResponse = { + errors: [ + { + id: "00000000-0000-0000-0000-000000000001", + sentryIssueId: "sentry-1", + fingerprint: null, + title: "old error", + route: "GET /api/foo", + statusCode: 500, + occurrences: 1, + firstSeenAt: "2026-05-01T00:00:00Z", + lastSeenAt: "2026-05-04T00:00:00Z", + severity: "unknown", + status: "open", + aiSummary: null, + aiSuspectedFiles: null, + aiRootCause: null, + aiSuggestedFix: null, + githubIssueNumber: null, + createdAt: "2026-05-01T00:00:00Z", + updatedAt: "2026-05-04T00:00:00Z", + }, + ], + total: 1, + limit: 50, + offset: 0, +}; + +function getBaseRow(): ApiErrorRow { + const base = FIXED_RESPONSE.errors[0]; + if (!base) throw new Error("FIXED_RESPONSE must have at least one row"); + return base; +} + +function makePushed(overrides: Partial = {}): ApiErrorRow { + return { + ...getBaseRow(), + id: "00000000-0000-0000-0000-0000000000aa", + title: "new pushed error", + ...overrides, + }; +} + +beforeEach(() => { + vi.mocked(getApiErrors).mockResolvedValue(FIXED_RESPONSE); + lastInstance = null; + // jsdom には EventSource が無いので global に注入する。 + // jsdom doesn't ship EventSource; install our fake on globalThis. + (globalThis as unknown as { EventSource: typeof EventSource }).EventSource = + createFakeEventSourceCtor(); +}); + +afterEach(() => { + vi.restoreAllMocks(); + // EventSource を消して次のテストの環境を汚染しないようにする。 + // Strip our fake so the next test boots from a clean slate. + delete (globalThis as { EventSource?: unknown }).EventSource; +}); + +describe("useApiErrors", () => { + it("loads via REST and exposes the rows", async () => { + const { result } = renderHook(() => useApiErrors({ intervalMs: 0 })); + await waitFor(() => expect(result.current.loading).toBe(false)); + expect(result.current.errors).toHaveLength(1); + expect(result.current.total).toBe(1); + }); + + it("opens an EventSource on mount and closes it on unmount", async () => { + const { unmount } = renderHook(() => useApiErrors({ intervalMs: 0 })); + await waitFor(() => expect(lastInstance).not.toBeNull()); + const es = getInstance(); + expect(es.url).toContain("/api/admin/errors/stream"); + expect(es.withCredentials).toBe(true); + unmount(); + expect(es.closed).toBe(true); + }); + + it("merges an `update` SSE event by id (replace)", async () => { + const { result } = renderHook(() => useApiErrors({ intervalMs: 0 })); + await waitFor(() => expect(result.current.loading).toBe(false)); + + act(() => { + getInstance().dispatch("ready", ""); + }); + + const replacement = { ...getBaseRow(), title: "replaced" }; + act(() => { + getInstance().dispatch("update", replacement); + }); + + expect(result.current.errors[0]?.title).toBe("replaced"); + expect(result.current.total).toBe(1); + expect(result.current.streamConnected).toBe(true); + }); + + it("prepends a brand-new id and bumps total", async () => { + const { result } = renderHook(() => useApiErrors({ intervalMs: 0 })); + await waitFor(() => expect(result.current.loading).toBe(false)); + + const fresh = makePushed(); + act(() => { + getInstance().dispatch("ready", ""); + getInstance().dispatch("update", fresh); + }); + + expect(result.current.errors[0]?.id).toBe(fresh.id); + expect(result.current.errors).toHaveLength(2); + expect(result.current.total).toBe(2); + }); + + it("ignores rows that don't match the active filter", async () => { + const { result } = renderHook(() => + useApiErrors({ status: "open", severity: "high", intervalMs: 0 }), + ); + await waitFor(() => expect(result.current.loading).toBe(false)); + + const mismatch = makePushed({ status: "resolved", severity: "low" }); + act(() => { + getInstance().dispatch("ready", ""); + getInstance().dispatch("update", mismatch); + }); + + expect(result.current.errors).toHaveLength(1); + expect(result.current.total).toBe(1); + }); + + it("does not open EventSource when enableStream is false", async () => { + renderHook(() => useApiErrors({ intervalMs: 0, enableStream: false })); + await waitFor(() => expect(getApiErrors).toHaveBeenCalled()); + expect(lastInstance).toBeNull(); + }); +}); diff --git a/admin/src/pages/errors/useApiErrors.ts b/admin/src/pages/errors/useApiErrors.ts index 49610eaa..b7f8a8da 100644 --- a/admin/src/pages/errors/useApiErrors.ts +++ b/admin/src/pages/errors/useApiErrors.ts @@ -8,18 +8,31 @@ import type { import { getApiErrors } from "@/api/admin"; /** - * Phase 1 でのポーリング間隔 (ms)。WebSocket / Server-Sent Events を導入する Phase 2 で - * 廃止する。長すぎるとバッジ更新が遅く、短すぎると API 負荷が上がる。 + * SSE が利用できない環境(古いブラウザ、ネットワーク制限)でのフォールバック + * ポーリング間隔 (ms)。EventSource が確立した後はサーバーから push されるため + * このタイマーは抑制される。 * - * Polling interval (ms) used during Phase 1; will be replaced by realtime - * updates in a later phase. Tuned to keep the badge fresh without hammering - * the API. + * Fallback polling interval (ms) used when the SSE stream is unavailable + * (older browsers, network restrictions, server 503 from subscriber cap). Once + * a SSE connection is healthy this timer is suppressed because updates arrive + * via push. */ export const API_ERRORS_POLL_INTERVAL_MS = 30_000; +/** + * SSE 切断時の再接続バックオフの初期値 (ms) と上限 (ms)。 + * 連続失敗で指数バックオフし、上限に到達したらそのまま継続。 + * + * Initial / max backoff (ms) used when the SSE connection drops. We grow the + * delay exponentially up to the cap so a flaky network or server hiccup does + * not cause a reconnect storm. + */ +const SSE_RECONNECT_INITIAL_MS = 1_000; +const SSE_RECONNECT_MAX_MS = 30_000; + /** * `useApiErrors` の入力。 - * Inputs accepted by the polling hook. + * Inputs accepted by the hook. */ export interface UseApiErrorsParams { status?: ApiErrorStatus; @@ -27,10 +40,18 @@ export interface UseApiErrorsParams { limit?: number; offset?: number; /** - * ポーリング間隔 (ms)。0 を渡すとポーリングを無効化する(テスト用途)。 - * Polling interval in ms; pass 0 to disable polling (used by tests). + * フォールバックポーリング間隔 (ms)。0 を渡すとポーリングを完全無効化する + * (テスト用途、もしくは SSE 専用にしたい場合)。 + * Fallback polling interval in ms; pass 0 to disable polling entirely + * (useful for tests or SSE-only environments). */ intervalMs?: number; + /** + * SSE エンドポイントを購読しない場合 false。テストや診断用途。 + * Disable the SSE subscription (default: true). Tests can opt out to keep + * EventSource off the wire. + */ + enableStream?: boolean; } /** @@ -42,33 +63,90 @@ export interface UseApiErrorsResult { total: number; loading: boolean; error: string | null; + /** SSE が確立しているか(テスト・UI 表示用) / Whether the SSE link is up */ + streamConnected: boolean; /** 即時再取得(リクエスト中のレースは内部で処理) / Force refresh (race-safe) */ refetch: () => Promise; } /** - * `GET /api/admin/errors` を取得し、定期ポーリングで同期するフック。 + * SSE で受信した 1 行を既存の `errors` 配列にマージする。 + * 同 ID があれば置換し、無ければ先頭に追加して `total` をインクリメントする。 * - * Phase 1 では realtime 接続が無いため、`API_ERRORS_POLL_INTERVAL_MS` ごとに - * 再フェッチする。タブが非表示 (`document.hidden`) の間は API 負荷を抑えるため - * インターバルをスキップし、可視化された時に即時再取得する。 + * Merge a single SSE-pushed row into the current list state. Matching rows + * are replaced in place; brand-new ids are prepended (newest-first) and the + * total count bumps by one so pagination footers stay accurate. + */ +function mergeRow(prev: GetApiErrorsResponse | null, row: ApiErrorRow): GetApiErrorsResponse { + if (!prev) { + return { errors: [row], total: 1, limit: 1, offset: 0 }; + } + const idx = prev.errors.findIndex((r) => r.id === row.id); + if (idx >= 0) { + const next = prev.errors.slice(); + next[idx] = row; + return { ...prev, errors: next }; + } + return { ...prev, errors: [row, ...prev.errors], total: prev.total + 1 }; +} + +/** + * フィルタ条件と SSE で push された行が合致するかを判定する。 + * 合致しないものは UI に出さない(一覧の意味的な整合を保つ)。 * - * Polls `GET /api/admin/errors` on a fixed interval. Skips ticks while the tab - * is hidden to avoid wasted API traffic, and refetches immediately when the - * tab becomes visible again. + * Decide whether an SSE-pushed row matches the active filter. Mismatches are + * dropped client-side so a status/severity filter doesn't suddenly surface + * rows that wouldn't appear in a fresh REST query. + */ +function matchesFilter( + row: ApiErrorRow, + status: ApiErrorStatus | undefined, + severity: ApiErrorSeverity | undefined, +): boolean { + if (status && row.status !== status) return false; + if (severity && row.severity !== severity) return false; + return true; +} + +/** + * `getApiErrors` で初回・フォールバック取得しつつ、`/api/admin/errors/stream` + * を `EventSource` で購読してリアルタイム更新するフック (Epic #616 Phase 2 / + * issue #807)。 + * + * - 接続成功中は `intervalMs` のポーリングを抑制する。 + * - 切断時は exponential backoff で再接続する。可視タブのみ。 + * - アンマウント時は EventSource を必ず close する(fd リーク防止)。 * - * @see https://github.com/otomatty/zedi/issues/616 - * @see https://github.com/otomatty/zedi/issues/804 + * Bootstrap the list via REST and subscribe to `/api/admin/errors/stream` for + * push updates (Epic #616 Phase 2 / issue #807). While the SSE link is up the + * fallback poller is suppressed; on disconnect we exponentially back off and + * reconnect (visible tabs only). The EventSource is always closed on unmount + * to prevent file-descriptor leaks. */ export function useApiErrors(params: UseApiErrorsParams = {}): UseApiErrorsResult { - const { status, severity, limit, offset, intervalMs = API_ERRORS_POLL_INTERVAL_MS } = params; + const { + status, + severity, + limit, + offset, + intervalMs = API_ERRORS_POLL_INTERVAL_MS, + enableStream = true, + } = params; const [data, setData] = useState(null); const [loading, setLoading] = useState(true); const [error, setError] = useState(null); + const [streamConnected, setStreamConnected] = useState(false); const isMountedRef = useRef(true); const latestRequestRef = useRef(0); + // 最新フィルタを ref に保持して EventSource の onmessage クロージャから参照する。 + // useEffect 依存に含めると接続を貼り直してしまうのを避ける。 + // Keep the latest filter in a ref so the EventSource handler can read the + // current value without forcing the SSE effect to tear down + reconnect on + // every filter change. + const filterRef = useRef({ status, severity }); + filterRef.current = { status, severity }; const load = useCallback( async (showLoading: boolean) => { @@ -99,8 +177,94 @@ export function useApiErrors(params: UseApiErrorsParams = {}): UseApiErrorsResul }; }, [load]); + // SSE 購読。`enableStream` と `EventSource` 利用可否で短絡する。 + // SSE subscription. Short-circuits when EventSource is unavailable (jsdom, + // very old browsers) or when the caller opts out (`enableStream: false`). + useEffect(() => { + if (!enableStream) return; + if (typeof EventSource === "undefined") return; + + let es: EventSource | null = null; + let reconnectTimer: number | null = null; + let cancelled = false; + let backoff = SSE_RECONNECT_INITIAL_MS; + + const open = () => { + if (cancelled) return; + es = new EventSource("/api/admin/errors/stream", { withCredentials: true }); + + es.addEventListener("ready", () => { + backoff = SSE_RECONNECT_INITIAL_MS; + if (isMountedRef.current) setStreamConnected(true); + }); + + es.addEventListener("update", (rawEvent) => { + const ev = rawEvent as MessageEvent; + let row: ApiErrorRow; + try { + row = JSON.parse(ev.data) as ApiErrorRow; + } catch { + return; + } + const { status: curStatus, severity: curSeverity } = filterRef.current; + if (!matchesFilter(row, curStatus, curSeverity)) return; + if (isMountedRef.current) { + setData((prev) => mergeRow(prev, row)); + } + }); + + // EventSource は接続が切れると自動で再接続する。意図しない無限ループを + // 避けるため、`onerror` で一度 close して我々の backoff スケジューラに + // 任せる。 + // EventSource auto-reconnects with a tiny delay, which fights with our + // backoff. Close the socket on error and reschedule ourselves so a + // failing endpoint doesn't hammer the server. + es.onerror = () => { + if (isMountedRef.current) setStreamConnected(false); + es?.close(); + es = null; + if (cancelled) return; + if (typeof document !== "undefined" && document.hidden) { + // 隠しタブでは再接続せず、可視化を待つ。 + // Hidden tabs: defer reconnect until the tab is visible again. + return; + } + reconnectTimer = window.setTimeout(() => { + backoff = Math.min(backoff * 2, SSE_RECONNECT_MAX_MS); + open(); + }, backoff); + }; + }; + + const onVisible = () => { + if (typeof document !== "undefined" && document.hidden) return; + // 可視化されたタイミングで未接続なら即時再接続を試みる。 + // When the tab becomes visible again, eagerly reconnect if we lost the + // stream while hidden. + if (!es) { + backoff = SSE_RECONNECT_INITIAL_MS; + open(); + } + }; + + open(); + document.addEventListener("visibilitychange", onVisible); + + return () => { + cancelled = true; + if (reconnectTimer != null) window.clearTimeout(reconnectTimer); + es?.close(); + es = null; + document.removeEventListener("visibilitychange", onVisible); + if (isMountedRef.current) setStreamConnected(false); + }; + }, [enableStream]); + + // フォールバックポーリング: SSE が確立していれば抑制する。 + // Fallback polling: suppressed while SSE is healthy so we don't double-load. useEffect(() => { if (intervalMs <= 0) return; + if (streamConnected) return; const tick = () => { if (typeof document !== "undefined" && document.hidden) return; void load(false); @@ -116,7 +280,7 @@ export function useApiErrors(params: UseApiErrorsParams = {}): UseApiErrorsResul window.clearInterval(id); document.removeEventListener("visibilitychange", onVisible); }; - }, [intervalMs, load]); + }, [intervalMs, load, streamConnected]); const refetch = useCallback(() => load(false), [load]); @@ -125,6 +289,7 @@ export function useApiErrors(params: UseApiErrorsParams = {}): UseApiErrorsResul total: data?.total ?? 0, loading, error, + streamConnected, refetch, }; } diff --git a/server/api/src/__tests__/routes/admin/errors.test.ts b/server/api/src/__tests__/routes/admin/errors.test.ts index c2150388..b1ba1ce9 100644 --- a/server/api/src/__tests__/routes/admin/errors.test.ts +++ b/server/api/src/__tests__/routes/admin/errors.test.ts @@ -8,9 +8,13 @@ * [0] adminRequired → ADMIN_ROLE_RESULT (or USER_ROLE_RESULT for 403 cases) * [1..] handler queries (depends on the route) */ -import { describe, it, expect, vi } from "vitest"; +import { afterEach, describe, it, expect, vi } from "vitest"; import type { Context, Next } from "hono"; import type { AppEnv } from "../../../types/index.js"; +import { + clearApiErrorSubscribers, + publishApiErrorUpdate, +} from "../../../services/apiErrorBroadcaster.js"; vi.mock("../../../middleware/auth.js", () => ({ authRequired: async (c: Context, next: Next) => { @@ -321,3 +325,93 @@ describe("PATCH /api/admin/errors/:id", () => { expect(res.status).toBe(403); }); }); + +// ── GET /api/admin/errors/stream ──────────────────────────────────────────── + +/** + * SSE のレスポンスは ReadableStream で延々と続くため、テストでは最初のチャンクを + * 1〜2 個読んだら必ず cancel して接続を閉じる。閉じないと vitest がハングする。 + * + * SSE responses keep streaming, so each test reads at most a couple of chunks + * and then cancels the body. Forgetting to cancel hangs the test runner. + */ +async function readUntil( + reader: ReadableStreamDefaultReader, + predicate: (text: string) => boolean, + timeoutMs = 1_000, +): Promise { + const decoder = new TextDecoder(); + let acc = ""; + const start = Date.now(); + while (Date.now() - start < timeoutMs) { + const { done, value } = await reader.read(); + if (done) break; + if (value) acc += decoder.decode(value, { stream: true }); + if (predicate(acc)) return acc; + } + return acc; +} + +describe("GET /api/admin/errors/stream", () => { + afterEach(() => { + // テスト間で broadcaster の購読者が残らないようにクリアする。 + // Reset broadcaster state between tests so a leaked subscriber doesn't + // cross-contaminate later cases. + clearApiErrorSubscribers(); + }); + + it("returns 200 with text/event-stream and the initial ready event", async () => { + const { app } = createAdminTestApp([ADMIN_ROLE_RESULT]); + + const res = await app.request("/api/admin/errors/stream", { + headers: adminAuthHeaders(), + }); + + expect(res.status).toBe(200); + expect(res.headers.get("content-type")).toMatch(/text\/event-stream/i); + const body = res.body; + if (!body) throw new Error("expected SSE body"); + const reader = body.getReader(); + const text = await readUntil(reader, (t) => t.includes("event: ready")); + expect(text).toContain("event: ready"); + expect(text).toContain("retry: 30000"); + await reader.cancel(); + }); + + it("forwards published rows as `update` events to subscribers", async () => { + const { app } = createAdminTestApp([ADMIN_ROLE_RESULT]); + + const res = await app.request("/api/admin/errors/stream", { + headers: adminAuthHeaders(), + }); + expect(res.status).toBe(200); + const body = res.body; + if (!body) throw new Error("expected SSE body"); + const reader = body.getReader(); + // Wait for the initial ready event so the subscriber is registered before + // we publish. + await readUntil(reader, (t) => t.includes("event: ready")); + + const row = makeRow({ id: "00000000-0000-0000-0000-0000000000aa" }); + publishApiErrorUpdate(row); + + const text = await readUntil(reader, (t) => t.includes("event: update")); + expect(text).toContain("event: update"); + expect(text).toContain('"id":"00000000-0000-0000-0000-0000000000aa"'); + await reader.cancel(); + }); + + it("returns 401 without auth", async () => { + const { app } = createAdminTestApp([]); + const res = await app.request("/api/admin/errors/stream", { + headers: { "Content-Type": "application/json" }, + }); + expect(res.status).toBe(401); + }); + + it("returns 403 when user is not admin", async () => { + const { app } = createAdminTestApp([USER_ROLE_RESULT]); + const res = await app.request("/api/admin/errors/stream", { headers: adminAuthHeaders() }); + expect(res.status).toBe(403); + }); +}); diff --git a/server/api/src/__tests__/services/apiErrorBroadcaster.test.ts b/server/api/src/__tests__/services/apiErrorBroadcaster.test.ts new file mode 100644 index 00000000..29a7ad10 --- /dev/null +++ b/server/api/src/__tests__/services/apiErrorBroadcaster.test.ts @@ -0,0 +1,111 @@ +/** + * `apiErrorBroadcaster` 単体テスト。 + * subscribe / publish / capacity / unsubscribe の挙動を検証する。 + * + * Unit tests for the in-memory `apiErrorBroadcaster` (subscribe, publish, + * capacity guard, unsubscribe cleanup). + * + * @see ../../services/apiErrorBroadcaster.ts + * @see https://github.com/otomatty/zedi/issues/807 + */ +import { afterEach, describe, expect, it, vi } from "vitest"; +import { + ApiErrorStreamCapacityExceededError, + API_ERROR_STREAM_MAX_SUBSCRIBERS, + apiErrorSubscriberCount, + clearApiErrorSubscribers, + publishApiErrorUpdate, + subscribeApiErrorUpdates, +} from "../../services/apiErrorBroadcaster.js"; +import type { ApiError } from "../../schema/apiErrors.js"; + +function makeRow(overrides: Partial = {}): ApiError { + return { + id: "00000000-0000-0000-0000-000000000001", + sentryIssueId: "sentry-1", + fingerprint: null, + title: "Test error", + route: null, + statusCode: 500, + occurrences: 1, + firstSeenAt: new Date("2026-05-01T00:00:00Z"), + lastSeenAt: new Date("2026-05-04T00:00:00Z"), + severity: "unknown", + status: "open", + aiSummary: null, + aiSuspectedFiles: null, + aiRootCause: null, + aiSuggestedFix: null, + githubIssueNumber: null, + createdAt: new Date("2026-05-01T00:00:00Z"), + updatedAt: new Date("2026-05-04T00:00:00Z"), + ...overrides, + }; +} + +afterEach(() => { + clearApiErrorSubscribers(); +}); + +describe("apiErrorBroadcaster", () => { + it("delivers published rows to every subscriber", () => { + const a = vi.fn(); + const b = vi.fn(); + subscribeApiErrorUpdates(a); + subscribeApiErrorUpdates(b); + + const row = makeRow(); + publishApiErrorUpdate(row); + + expect(a).toHaveBeenCalledTimes(1); + expect(a).toHaveBeenCalledWith(row); + expect(b).toHaveBeenCalledTimes(1); + expect(b).toHaveBeenCalledWith(row); + }); + + it("stops delivering after unsubscribe", () => { + const listener = vi.fn(); + const unsubscribe = subscribeApiErrorUpdates(listener); + unsubscribe(); + + publishApiErrorUpdate(makeRow()); + expect(listener).not.toHaveBeenCalled(); + expect(apiErrorSubscriberCount()).toBe(0); + }); + + it("isolates a throwing subscriber from the rest", () => { + const consoleErr = vi.spyOn(console, "error").mockImplementation(() => {}); + const bad = vi.fn(() => { + throw new Error("boom"); + }); + const good = vi.fn(); + subscribeApiErrorUpdates(bad); + subscribeApiErrorUpdates(good); + + publishApiErrorUpdate(makeRow()); + + expect(bad).toHaveBeenCalledTimes(1); + expect(good).toHaveBeenCalledTimes(1); + expect(consoleErr).toHaveBeenCalled(); + consoleErr.mockRestore(); + }); + + it("rejects subscribe past the cap", () => { + for (let i = 0; i < API_ERROR_STREAM_MAX_SUBSCRIBERS; i++) { + subscribeApiErrorUpdates(() => {}); + } + expect(() => subscribeApiErrorUpdates(() => {})).toThrow(ApiErrorStreamCapacityExceededError); + expect(apiErrorSubscriberCount()).toBe(API_ERROR_STREAM_MAX_SUBSCRIBERS); + }); + + it("counts active subscribers", () => { + expect(apiErrorSubscriberCount()).toBe(0); + const u1 = subscribeApiErrorUpdates(() => {}); + const u2 = subscribeApiErrorUpdates(() => {}); + expect(apiErrorSubscriberCount()).toBe(2); + u1(); + expect(apiErrorSubscriberCount()).toBe(1); + u2(); + expect(apiErrorSubscriberCount()).toBe(0); + }); +}); diff --git a/server/api/src/routes/admin/errors.ts b/server/api/src/routes/admin/errors.ts index d9f13a40..7731bf01 100644 --- a/server/api/src/routes/admin/errors.ts +++ b/server/api/src/routes/admin/errors.ts @@ -12,6 +12,7 @@ * @see https://github.com/otomatty/zedi/issues/803 */ import { Hono } from "hono"; +import { streamSSE } from "hono/streaming"; import { ALLOWED_API_ERROR_STATUS_TRANSITIONS, ApiErrorInvalidTransitionError, @@ -22,7 +23,12 @@ import { listApiErrors, updateApiErrorStatus, } from "../../services/apiErrorService.js"; -import type { ApiErrorSeverity, ApiErrorStatus } from "../../schema/apiErrors.js"; +import { + ApiErrorStreamCapacityExceededError, + publishApiErrorUpdate, + subscribeApiErrorUpdates, +} from "../../services/apiErrorBroadcaster.js"; +import type { ApiError, ApiErrorSeverity, ApiErrorStatus } from "../../schema/apiErrors.js"; import type { AppEnv } from "../../types/index.js"; const app = new Hono(); @@ -119,6 +125,124 @@ app.get("/", async (c) => { return c.json({ errors: rows, total, limit, offset }); }); +/** + * SSE で送信するイベントの payload を ApiError 行から組み立てる。 + * timestamps は Date のまま JSON.stringify すると ISO 文字列になる。クライアントは + * `ApiErrorRow` として消費する。 + * + * Build the SSE event payload from an `ApiError` row. Date instances + * stringify to ISO 8601 so the wire shape matches the REST `ApiErrorRow`. + */ +function serializeRowEvent(row: ApiError): string { + return JSON.stringify(row); +} + +/** + * SSE 接続のキープアライブ間隔 (ms)。プロキシや LB がアイドル接続を切る前に + * コメント行を送って TCP を温存する。 + * + * Keep-alive interval (ms) for SSE connections. Proxies / load balancers tend + * to drop idle TCP after 30–60 s; emit a comment line periodically so the + * stream stays open without producing visible events on the client. + */ +const SSE_KEEPALIVE_MS = 25_000; + +/** + * GET /api/admin/errors/stream — `text/event-stream` で `api_errors` の更新を + * リアルタイム配信する (Epic #616 Phase 2 / issue #807)。`adminRequired` は + * 親ルート (`/api/admin`) で適用済み。 + * + * クライアントは `EventSource('/api/admin/errors/stream')` で接続し、 + * `update` イベントの `data` を `ApiErrorRow` としてパースする。受信側は同 ID + * の行を最新値で置換し、未知の ID なら一覧の先頭に追加する想定。 + * + * GET /api/admin/errors/stream — Server-Sent Events feed of `api_errors` + * updates (Epic #616 Phase 2 / issue #807). Browsers consume it via + * `EventSource`; each `update` event carries one `ApiErrorRow` JSON payload so + * the admin UI can replace the row in place (or prepend on first sight) + * without a full refetch. + * + * - 503: 同時接続数の上限に達している。クライアントは backoff 後に再接続する。 + * - 200: 接続確立後、最初に `: connected` コメント行と `retry: 30000` を返す。 + * + * - 503: subscriber cap reached; client must back off before reconnecting. + * - 200: on connect, emits a `: connected` comment plus a `retry: 30000` hint + * so the browser's auto-reconnect waits 30 s instead of the default ~3 s. + */ +app.get("/stream", (c) => { + return streamSSE( + c, + async (stream) => { + // EventSource は接続成功直後の最初のイベントを待つので、空っぽの応答を + // 返さないよう必ずコメントと retry ヒントを送る。 + // EventSource holds the request "pending" until the first event lands; + // emit a comment + retry hint so the browser commits to the connection + // and uses our preferred reconnect delay. + await stream.writeSSE({ data: "", event: "ready", retry: 30_000 }); + + let unsubscribe: (() => void) | null = null; + try { + // すべての更新を購読。送信は async なのでイベントを内部キューに溜め、 + // 順序を保ったまま flush する。 + // Subscribe and queue events; SSE writes are async but listener + // callbacks must be sync, so we serialize through a microtask chain. + let writeChain: Promise = Promise.resolve(); + unsubscribe = subscribeApiErrorUpdates((row) => { + writeChain = writeChain + .then(async () => { + if (stream.aborted || stream.closed) return; + await stream.writeSSE({ event: "update", data: serializeRowEvent(row) }); + }) + .catch((err) => { + const message = err instanceof Error ? err.message : String(err); + console.error(`[admin-errors-stream] write failed: ${message}`); + }); + }); + } catch (err) { + if (err instanceof ApiErrorStreamCapacityExceededError) { + await stream.writeSSE({ + event: "error", + data: JSON.stringify({ error: "subscriber cap reached" }), + }); + await stream.close(); + return; + } + throw err; + } + + stream.onAbort(() => { + unsubscribe?.(); + }); + + // クライアントが切断するまでキープアライブを送り続ける。 + // sleep が abort で resolve すると while を抜けて handler が終了し、 + // ここまで来たら `onAbort` で unsubscribe 済み。 + // Heartbeat loop: emit a comment line every SSE_KEEPALIVE_MS so idle + // proxies (and our own server) don't tear down the TCP connection. + // Exits naturally when the client disconnects (sleep resolves on abort). + while (!stream.aborted && !stream.closed) { + await stream.sleep(SSE_KEEPALIVE_MS); + if (stream.aborted || stream.closed) break; + // SSE コメント行(`:` で始まる)はクライアントには配信されない。 + // SSE comment lines (leading `:`) are ignored by the EventSource API + // but keep the underlying connection alive. + await stream.writeSSE({ data: "", event: "ping" }); + } + + unsubscribe?.(); + }, + async (err, stream) => { + const message = err instanceof Error ? err.message : String(err); + console.error(`[admin-errors-stream] handler error: ${message}`); + try { + await stream.writeSSE({ event: "error", data: JSON.stringify({ error: "stream error" }) }); + } catch { + /* swallow — connection likely closed */ + } + }, + ); +}); + /** * GET /api/admin/errors/:id — 詳細取得。 * GET /api/admin/errors/:id — fetch a single row by primary key. @@ -185,6 +309,11 @@ app.patch("/:id", async (c) => { if (!row) { return c.json({ error: "Not found" }, 404); } + // SSE 購読者へ最新行を配信。失敗してもステータス更新は成功扱いにする + // (broadcaster は in-memory なので例外時は購読者側で polling fallback を期待)。 + // Notify SSE subscribers; errors here must not flip the PATCH response from + // 200 to 5xx because the DB write already succeeded. + publishApiErrorUpdate(row); return c.json({ error: row }); } catch (err) { if (err instanceof ApiErrorStatusConflictError) { diff --git a/server/api/src/routes/webhooks/githubAiCallback.ts b/server/api/src/routes/webhooks/githubAiCallback.ts index 90b2288f..fc30f8c2 100644 --- a/server/api/src/routes/webhooks/githubAiCallback.ts +++ b/server/api/src/routes/webhooks/githubAiCallback.ts @@ -24,6 +24,7 @@ import { updateAiAnalysis, type UpdateAiAnalysisInput, } from "../../services/apiErrorService.js"; +import { publishApiErrorUpdate } from "../../services/apiErrorBroadcaster.js"; import { verifyInstallationToken } from "../../lib/githubAppAuth.js"; import type { AppEnv } from "../../types/index.js"; @@ -157,6 +158,9 @@ app.put("/:id", async (c) => { console.log( `[github-ai-callback] updated api_error=${updated.id} severity=${updated.severity}`, ); + // SSE 購読者へ AI 解析結果を配信 (Phase 2 / issue #807)。 + // Notify SSE subscribers so the admin UI updates without a page reload. + publishApiErrorUpdate(updated); // 外部 (GitHub Actions) 向けの webhook なので、`error` キーを成功時に流用する // 内部 admin API の慣習ではなく、`data` キーで返して "error 有無で失敗判定" // できる素直な形にする(admin/src/api/admin.ts と異なり消費者がまだ存在しない)。 diff --git a/server/api/src/routes/webhooks/sentry.ts b/server/api/src/routes/webhooks/sentry.ts index ed739f8a..2fd8b5d8 100644 --- a/server/api/src/routes/webhooks/sentry.ts +++ b/server/api/src/routes/webhooks/sentry.ts @@ -28,6 +28,7 @@ import { getApiErrorBySentryIssueId, upsertFromSentrySummary, } from "../../services/apiErrorService.js"; +import { publishApiErrorUpdate } from "../../services/apiErrorBroadcaster.js"; import { readDispatchRepository, triggerRepositoryDispatch } from "../../lib/githubAppAuth.js"; import type { AppEnv } from "../../types/index.js"; @@ -410,6 +411,11 @@ app.post("/", async (c) => { console.log( `[sentry-webhook] resource=${resource} upserted issue=${row.sentryIssueId} occurrences=${row.occurrences} isNew=${isNew}`, ); + // SSE 購読者へ最新行を配信 (Phase 2 / issue #807)。broadcaster は in-memory + // なので Webhook の応答性に影響しない。 + // Fan out to SSE subscribers (Phase 2 / issue #807). Broadcaster is + // synchronous + in-memory, so it doesn't slow down the webhook response. + publishApiErrorUpdate(row); if (isNew) { // fire-and-forget: dispatch のレスポンスを await せずに 200 を返す。 diff --git a/server/api/src/services/apiErrorBroadcaster.ts b/server/api/src/services/apiErrorBroadcaster.ts new file mode 100644 index 00000000..1bf2cf5b --- /dev/null +++ b/server/api/src/services/apiErrorBroadcaster.ts @@ -0,0 +1,117 @@ +/** + * `api_errors` 行の更新を SSE 購読者へブロードキャストする in-memory pub/sub。 + * + * Phase 2 (Epic #616 / issue #807) では管理画面の `/admin/errors` を SSE で + * リアルタイム更新する。Sentry Webhook / GitHub AI コールバック / 管理画面の + * PATCH の各経路で `publishApiErrorUpdate` を呼ぶと、`subscribeApiErrorUpdates` + * で購読中のすべてのリスナーへ最新行が配信される。 + * + * In-memory pub/sub used by `/admin/errors` SSE streaming (Epic #616 / issue + * #807). Producers (Sentry webhook, GitHub AI callback, admin PATCH) call + * `publishApiErrorUpdate` after persisting a row; the SSE route subscribes via + * `subscribeApiErrorUpdates` and forwards events to connected admins. + * + * 単一プロセス前提のため、PoC 規模では十分。スケール時 (複数 worker) は + * Redis Pub/Sub または Postgres LISTEN/NOTIFY に差し替える設計。 + * + * Single-process only — fine for the PoC. When the API scales horizontally, + * swap the implementation for Redis Pub/Sub or Postgres LISTEN/NOTIFY without + * changing the call sites. + * + * @see ../routes/admin/errors.ts + * @see https://github.com/otomatty/zedi/issues/616 + * @see https://github.com/otomatty/zedi/issues/807 + */ +import type { ApiError } from "../schema/apiErrors.js"; + +/** + * SSE 購読者の同時接続数上限。`adminRequired` で admin だけがアクセスできるため + * 不正利用は限定的だが、メモリ・ファイル記述子のフェイルセーフとして上限を設ける。 + * + * Hard cap on simultaneous SSE subscribers. Admin-gated already, but enforce + * a defensive ceiling so a buggy client cannot exhaust memory or file + * descriptors by opening unbounded EventSource connections. + */ +export const API_ERROR_STREAM_MAX_SUBSCRIBERS = 64; + +/** + * `subscribeApiErrorUpdates` のリスナー型。`undefined` は該当行が削除された + * 場合の通知用に予約しているが、現状は常に最新行を渡す。 + * + * Listener signature passed to `subscribeApiErrorUpdates`. The optional + * `undefined` slot is reserved for a future delete notification; today we + * always emit the latest row. + */ +export type ApiErrorUpdateListener = (row: ApiError) => void; + +const listeners = new Set(); + +/** + * `publishApiErrorUpdate` などが上限超過などで投げるエラー型のシグナル。 + * Returned by `subscribeApiErrorUpdates` when the subscription cap is reached. + */ +export class ApiErrorStreamCapacityExceededError extends Error { + /** + * 上限値を含むメッセージで例外を生成する。 + * Build the error with a message that includes the configured cap. + */ + constructor() { + super(`api_error stream subscriber cap reached (${API_ERROR_STREAM_MAX_SUBSCRIBERS})`); + this.name = "ApiErrorStreamCapacityExceededError"; + } +} + +/** + * 新しい SSE 購読者を登録する。返り値の `unsubscribe` で解除する。 + * 上限を超える場合は `ApiErrorStreamCapacityExceededError` を投げ、 + * SSE ルート側で 503 にマップする。 + * + * Register a new SSE subscriber. Returns an `unsubscribe` callback that the + * route layer invokes from its abort handler. Exceeding + * `API_ERROR_STREAM_MAX_SUBSCRIBERS` throws `ApiErrorStreamCapacityExceededError` + * so the SSE route can answer 503 instead of silently dropping events. + */ +export function subscribeApiErrorUpdates(listener: ApiErrorUpdateListener): () => void { + if (listeners.size >= API_ERROR_STREAM_MAX_SUBSCRIBERS) { + throw new ApiErrorStreamCapacityExceededError(); + } + listeners.add(listener); + return () => { + listeners.delete(listener); + }; +} + +/** + * 全購読者へ更新行を配信する。リスナーが throw しても他リスナーへ影響しないよう + * 例外は console.error でログするだけにする。 + * + * Fan out a row update to every subscriber. A listener that throws is logged + * and skipped so a single buggy connection does not break broadcast for the + * rest of the admins. + */ +export function publishApiErrorUpdate(row: ApiError): void { + for (const listener of listeners) { + try { + listener(row); + } catch (err) { + const message = err instanceof Error ? err.message : String(err); + console.error(`[api-errors-broadcaster] subscriber threw: ${message}`); + } + } +} + +/** + * 現在の購読者数。テスト・運用監視用。 + * Number of currently-registered subscribers. Used by tests and ops dashboards. + */ +export function apiErrorSubscriberCount(): number { + return listeners.size; +} + +/** + * 全購読者を強制解除する。テスト用に提供。 + * Drop every subscriber. Test-only helper to keep state clean between cases. + */ +export function clearApiErrorSubscribers(): void { + listeners.clear(); +} From 428f31f0026263875334f3da0dd0721c0e82ec0e Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 5 May 2026 02:20:53 +0000 Subject: [PATCH 2/3] fix(admin): address PR #816 review feedback for SSE realtime - route SSE URL through `getApiUrl` so split-origin admin builds reach the API host instead of the Cloudflare Pages origin - pre-check subscriber cap before `streamSSE` so capacity exhaustion returns a real 503 instead of 200 + SSE error event - clear pending `reconnectTimer` in the visibilitychange handler to avoid spawning a duplicate `EventSource` - move SSE-updated rows to the front of the list (matches the server's `last_seen_at DESC` ordering on a fresh REST fetch) --- admin/src/api/client.ts | 17 ++++++- admin/src/pages/errors/useApiErrors.test.ts | 36 ++++++++++--- admin/src/pages/errors/useApiErrors.ts | 51 ++++++++++++++----- .../src/__tests__/routes/admin/errors.test.ts | 18 +++++++ server/api/src/routes/admin/errors.ts | 44 +++++++++++----- 5 files changed, 132 insertions(+), 34 deletions(-) diff --git a/admin/src/api/client.ts b/admin/src/api/client.ts index 5d439aee..df55cadc 100644 --- a/admin/src/api/client.ts +++ b/admin/src/api/client.ts @@ -5,7 +5,22 @@ const baseUrl = import.meta.env.VITE_API_BASE_URL ?? ""; -function getApiUrl(path: string): string { +/** + * パスを `VITE_API_BASE_URL` と結合して絶対 URL(または相対パス)を返す。 + * 本番では admin が Cloudflare Pages、API が `https://api.zedi-note.app` + * など別オリジンになるため、`fetch` だけでなく `EventSource` などの + * リクエスト URL もこの関数で組み立てる必要がある。 + * + * Resolve a path against `VITE_API_BASE_URL`. Production splits the admin + * origin (Cloudflare Pages) from the API origin + * (`https://api.zedi-note.app`), so EventSource and other non-`adminFetch` + * consumers must use this helper or they'll hit the wrong host. + * + * @param path - API パス(先頭の `/` は任意)/ API path (leading `/` optional) + * @returns 絶対 URL(base 設定時)または同一オリジンのパス / + * Absolute URL when a base is set, otherwise the same-origin path + */ +export function getApiUrl(path: string): string { const normalized = path.startsWith("/") ? path : `/${path}`; return baseUrl ? `${baseUrl.replace(/\/$/, "")}${normalized}` : normalized; } diff --git a/admin/src/pages/errors/useApiErrors.test.ts b/admin/src/pages/errors/useApiErrors.test.ts index 0e2d9138..0eafe740 100644 --- a/admin/src/pages/errors/useApiErrors.test.ts +++ b/admin/src/pages/errors/useApiErrors.test.ts @@ -155,17 +155,38 @@ describe("useApiErrors", () => { expect(result.current.total).toBe(1); }); - it("opens an EventSource on mount and closes it on unmount", async () => { + it("opens an EventSource on mount (URL routed through getApiUrl) and closes it on unmount", async () => { const { unmount } = renderHook(() => useApiErrors({ intervalMs: 0 })); await waitFor(() => expect(lastInstance).not.toBeNull()); const es = getInstance(); + // `getApiUrl` 経由なので、`VITE_API_BASE_URL` が空文字でもフルパスは + // `/api/admin/errors/stream` を含む。 + // `getApiUrl` resolves the path; with an empty `VITE_API_BASE_URL` it + // stays same-origin, so the URL still ends with the SSE path. expect(es.url).toContain("/api/admin/errors/stream"); expect(es.withCredentials).toBe(true); unmount(); expect(es.closed).toBe(true); }); - it("merges an `update` SSE event by id (replace)", async () => { + it("merges an `update` SSE event by id and moves the row to the front", async () => { + // 初期一覧に 2 件目の行を返すように mockResolvedValue を上書きし、 + // 2 番目の行を更新したらリストの先頭へ移動することを確認する。 + // Bootstrap with two rows so we can verify the second row is *moved* to + // the front (matching the server's `last_seen_at DESC` ordering) rather + // than replaced in place. + const second: ApiErrorRow = { + ...getBaseRow(), + id: "00000000-0000-0000-0000-0000000000bb", + title: "second", + }; + vi.mocked(getApiErrors).mockResolvedValueOnce({ + errors: [getBaseRow(), second], + total: 2, + limit: 50, + offset: 0, + }); + const { result } = renderHook(() => useApiErrors({ intervalMs: 0 })); await waitFor(() => expect(result.current.loading).toBe(false)); @@ -173,13 +194,16 @@ describe("useApiErrors", () => { getInstance().dispatch("ready", ""); }); - const replacement = { ...getBaseRow(), title: "replaced" }; + const updatedSecond = { ...second, title: "second (updated)" }; act(() => { - getInstance().dispatch("update", replacement); + getInstance().dispatch("update", updatedSecond); }); - expect(result.current.errors[0]?.title).toBe("replaced"); - expect(result.current.total).toBe(1); + expect(result.current.errors).toHaveLength(2); + expect(result.current.errors[0]?.id).toBe(second.id); + expect(result.current.errors[0]?.title).toBe("second (updated)"); + expect(result.current.errors[1]?.id).toBe(getBaseRow().id); + expect(result.current.total).toBe(2); expect(result.current.streamConnected).toBe(true); }); diff --git a/admin/src/pages/errors/useApiErrors.ts b/admin/src/pages/errors/useApiErrors.ts index b7f8a8da..7aecdac7 100644 --- a/admin/src/pages/errors/useApiErrors.ts +++ b/admin/src/pages/errors/useApiErrors.ts @@ -6,6 +6,7 @@ import type { GetApiErrorsResponse, } from "@/api/admin"; import { getApiErrors } from "@/api/admin"; +import { getApiUrl } from "@/api/client"; /** * SSE が利用できない環境(古いブラウザ、ネットワーク制限)でのフォールバック @@ -71,23 +72,33 @@ export interface UseApiErrorsResult { /** * SSE で受信した 1 行を既存の `errors` 配列にマージする。 - * 同 ID があれば置換し、無ければ先頭に追加して `total` をインクリメントする。 * - * Merge a single SSE-pushed row into the current list state. Matching rows - * are replaced in place; brand-new ids are prepended (newest-first) and the - * total count bumps by one so pagination footers stay accurate. + * - 既存の ID が見つかった場合は元の位置から取り除いて先頭へ移動する。 + * サーバー側のデフォルト順序が `last_seen_at DESC` なので、更新(再発生・ + * AI 解析完了・状態変更)があった行を先頭に置く方が REST 再取得時の順序 + * と整合する。 + * - 未知の ID なら先頭に追加し、`total` をインクリメントする。 + * - `offset > 0` のページではこの単純マージが完全には正しくないが、 + * 現状 UI は単一ページのみ表示するため許容(将来ページネーション導入時に + * 再考)。 + * + * Merge an SSE-pushed row into the list. Existing ids are *moved* to the + * front (matching the server's `last_seen_at DESC` ordering on a re-fetch), + * and brand-new ids are prepended with `total` bumping by one. Pagination + * (`offset > 0`) isn't fully consistent under this merge, but the current + * UI shows a single page so we accept the trade-off. */ function mergeRow(prev: GetApiErrorsResponse | null, row: ApiErrorRow): GetApiErrorsResponse { if (!prev) { return { errors: [row], total: 1, limit: 1, offset: 0 }; } - const idx = prev.errors.findIndex((r) => r.id === row.id); - if (idx >= 0) { - const next = prev.errors.slice(); - next[idx] = row; - return { ...prev, errors: next }; - } - return { ...prev, errors: [row, ...prev.errors], total: prev.total + 1 }; + const filtered = prev.errors.filter((r) => r.id !== row.id); + const isUpdate = filtered.length < prev.errors.length; + return { + ...prev, + errors: [row, ...filtered], + total: isUpdate ? prev.total : prev.total + 1, + }; } /** @@ -191,7 +202,12 @@ export function useApiErrors(params: UseApiErrorsParams = {}): UseApiErrorsResul const open = () => { if (cancelled) return; - es = new EventSource("/api/admin/errors/stream", { withCredentials: true }); + // 本番では admin と API のオリジンが分かれているため、相対パスではなく + // `VITE_API_BASE_URL` を解決した絶対 URL を使う必要がある (PR #816 review)。 + // Production splits the admin origin (Cloudflare Pages) from the API + // origin, so the EventSource URL must go through `getApiUrl` to hit + // the correct host instead of falling back to admin's origin. + es = new EventSource(getApiUrl("/api/admin/errors/stream"), { withCredentials: true }); es.addEventListener("ready", () => { backoff = SSE_RECONNECT_INITIAL_MS; @@ -239,9 +255,18 @@ export function useApiErrors(params: UseApiErrorsParams = {}): UseApiErrorsResul const onVisible = () => { if (typeof document !== "undefined" && document.hidden) return; // 可視化されたタイミングで未接続なら即時再接続を試みる。 + // 既存の `reconnectTimer` を必ずクリアしてから接続を張ることで、 + // タイマー fire と immediate-reopen が競合して EventSource が二重に + // 生成されるのを防ぐ。 // When the tab becomes visible again, eagerly reconnect if we lost the - // stream while hidden. + // stream while hidden. Cancel any pending backoff timer first so a + // racing `setTimeout` callback can't open a second EventSource on top + // of this one (which would leak the older connection). if (!es) { + if (reconnectTimer != null) { + window.clearTimeout(reconnectTimer); + reconnectTimer = null; + } backoff = SSE_RECONNECT_INITIAL_MS; open(); } diff --git a/server/api/src/__tests__/routes/admin/errors.test.ts b/server/api/src/__tests__/routes/admin/errors.test.ts index b1ba1ce9..7742384b 100644 --- a/server/api/src/__tests__/routes/admin/errors.test.ts +++ b/server/api/src/__tests__/routes/admin/errors.test.ts @@ -12,8 +12,10 @@ import { afterEach, describe, it, expect, vi } from "vitest"; import type { Context, Next } from "hono"; import type { AppEnv } from "../../../types/index.js"; import { + API_ERROR_STREAM_MAX_SUBSCRIBERS, clearApiErrorSubscribers, publishApiErrorUpdate, + subscribeApiErrorUpdates, } from "../../../services/apiErrorBroadcaster.js"; vi.mock("../../../middleware/auth.js", () => ({ @@ -414,4 +416,20 @@ describe("GET /api/admin/errors/stream", () => { const res = await app.request("/api/admin/errors/stream", { headers: adminAuthHeaders() }); expect(res.status).toBe(403); }); + + it("returns 503 (not 200) when subscriber cap is reached", async () => { + // 上限まで購読者を埋めてから接続を試みると、`streamSSE` 開始前に 503 で + // 即時拒否されることを確認する (PR #816 review fix)。 + // Pre-fill the broadcaster up to the cap so the next connection hits the + // pre-check inside the `/stream` route and gets a real 503 response + // instead of a 200 + SSE error event. + for (let i = 0; i < API_ERROR_STREAM_MAX_SUBSCRIBERS; i++) { + subscribeApiErrorUpdates(() => {}); + } + const { app } = createAdminTestApp([ADMIN_ROLE_RESULT]); + const res = await app.request("/api/admin/errors/stream", { headers: adminAuthHeaders() }); + expect(res.status).toBe(503); + const body = (await res.json()) as { error: string }; + expect(body.error).toMatch(/subscriber cap/i); + }); }); diff --git a/server/api/src/routes/admin/errors.ts b/server/api/src/routes/admin/errors.ts index 7731bf01..69dafafc 100644 --- a/server/api/src/routes/admin/errors.ts +++ b/server/api/src/routes/admin/errors.ts @@ -24,7 +24,8 @@ import { updateApiErrorStatus, } from "../../services/apiErrorService.js"; import { - ApiErrorStreamCapacityExceededError, + apiErrorSubscriberCount, + API_ERROR_STREAM_MAX_SUBSCRIBERS, publishApiErrorUpdate, subscribeApiErrorUpdates, } from "../../services/apiErrorBroadcaster.js"; @@ -170,6 +171,17 @@ const SSE_KEEPALIVE_MS = 25_000; * so the browser's auto-reconnect waits 30 s instead of the default ~3 s. */ app.get("/stream", (c) => { + // 上限チェックは streamSSE を呼ぶ前に行う。streamSSE が走り出すと既に + // ステータス 200 + `text/event-stream` のレスポンスが開始されてしまい、 + // 503 へ降格できないため。クライアント側は 503 を見て backoff する想定。 + // Reject capacity exhaustion BEFORE entering streamSSE — once that helper + // commits the response (200 + text/event-stream), we can no longer change + // the status to 503. The pre-check lets clients see a real 503 and apply + // the documented backoff strategy instead of treating it as a transient + // SSE-level error. + if (apiErrorSubscriberCount() >= API_ERROR_STREAM_MAX_SUBSCRIBERS) { + return c.json({ error: `subscriber cap reached (${API_ERROR_STREAM_MAX_SUBSCRIBERS})` }, 503); + } return streamSSE( c, async (stream) => { @@ -180,12 +192,17 @@ app.get("/stream", (c) => { // and uses our preferred reconnect delay. await stream.writeSSE({ data: "", event: "ready", retry: 30_000 }); + // すべての更新を購読。送信は async なのでイベントを内部キューに溜め、 + // 順序を保ったまま flush する。 + // Subscribe and queue events; SSE writes are async but listener + // callbacks must be sync, so we serialize through a microtask chain. + // 上限はルート冒頭で確認済みのため通常 throw しないが、競合で同時に + // 限界を超えた場合に備えて catch して接続を畳む。 + // Capacity is verified at the top of the route, but a concurrent + // subscribe could still exceed the cap; close the stream cleanly if + // that race fires instead of leaking an unhandled rejection. let unsubscribe: (() => void) | null = null; try { - // すべての更新を購読。送信は async なのでイベントを内部キューに溜め、 - // 順序を保ったまま flush する。 - // Subscribe and queue events; SSE writes are async but listener - // callbacks must be sync, so we serialize through a microtask chain. let writeChain: Promise = Promise.resolve(); unsubscribe = subscribeApiErrorUpdates((row) => { writeChain = writeChain @@ -199,15 +216,14 @@ app.get("/stream", (c) => { }); }); } catch (err) { - if (err instanceof ApiErrorStreamCapacityExceededError) { - await stream.writeSSE({ - event: "error", - data: JSON.stringify({ error: "subscriber cap reached" }), - }); - await stream.close(); - return; - } - throw err; + const message = err instanceof Error ? err.message : String(err); + console.error(`[admin-errors-stream] subscribe failed: ${message}`); + await stream.writeSSE({ + event: "error", + data: JSON.stringify({ error: "subscribe failed" }), + }); + await stream.close(); + return; } stream.onAbort(() => { From 2c06ab69128b99ee1f05468aab1f8ba087f42874 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 5 May 2026 02:25:42 +0000 Subject: [PATCH 3/3] fix(admin): use raw SSE comment line for keep-alive ping MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `stream.writeSSE({ event: "ping" })` always emits an `event:` field, so the client receives a named ping event — not the invisible keep-alive the comment claimed. Switch to `stream.write(": ping\n\n")` so the heartbeat is a true SSE comment line (ignored by EventSource per the spec) while still keeping idle proxies from tearing down the TCP connection. PR #816 review feedback. --- server/api/src/routes/admin/errors.ts | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/server/api/src/routes/admin/errors.ts b/server/api/src/routes/admin/errors.ts index 69dafafc..18ce8872 100644 --- a/server/api/src/routes/admin/errors.ts +++ b/server/api/src/routes/admin/errors.ts @@ -239,10 +239,14 @@ app.get("/stream", (c) => { while (!stream.aborted && !stream.closed) { await stream.sleep(SSE_KEEPALIVE_MS); if (stream.aborted || stream.closed) break; - // SSE コメント行(`:` で始まる)はクライアントには配信されない。 - // SSE comment lines (leading `:`) are ignored by the EventSource API - // but keep the underlying connection alive. - await stream.writeSSE({ data: "", event: "ping" }); + // 生の SSE コメント行(`:` で始まり空行で終わる)を直接書き込む。 + // `writeSSE` は `event:` 行を必ず生成するためコメントにはならず、 + // 名前付きイベントが配信されてしまう (PR #816 review)。 + // Write a raw SSE comment line (`:` prefix, blank-line terminated). + // `writeSSE` always emits an `event:` field, which would be a named + // event the client receives — not the invisible keep-alive we want + // (PR #816 review). + await stream.write(": ping\n\n"); } unsubscribe?.();