From 48aa4f7c1499c76550329bcf07239cf027a7736b Mon Sep 17 00:00:00 2001 From: Renn F Date: Sun, 28 Jun 2026 18:19:50 +0200 Subject: [PATCH] [F021] handle SSE transport errors so the intake composer isn't stuck MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit openStream registered listeners for the server-sent event kinds but not the EventSource's own transport-level error. The 'error' kind IS in LIVE_EVENT_KINDS, so a server-sent event:error (JSON MessageEvent) was handled — but a dropped connection / dead session fires a plain Event with NO data, which JSON.parse(undefined) swallowed in the try/catch, so the stream 'stayed open' (EventSource loop-reconnected a session that no longer existed) and isSending stayed true — the composer was permanently disabled. Fix: route the 'error' event by payload. A MessageEvent with string data is a server-sent error → handleEvent (unchanged). A no-data Event is a transport error → handleTransportError: clear streamingId/activity, set isSending false, add a 'connection lost' error message, keep a draft/batch preview up (so the human can still act on a proposed card) else land on 'chatting', and close the dead stream so EventSource stops loop-reconnecting. Tests: renderHook + a jsdom EventSource double that fires a transport error (plain Event, no data) vs a server-sent error (MessageEvent + JSON). RED: transport error left isSending true; GREEN: resets to false, surfaces the message, closes the stream. The server-sent-JSON path is unchanged. Full panel suite (129) green; eslint/typecheck/prettier clean. --- .../hooks/__tests__/use-prompter-sse.test.tsx | 159 ++++++++++++++++++ panel/src/hooks/use-prompter.ts | 47 +++++- 2 files changed, 205 insertions(+), 1 deletion(-) create mode 100644 panel/src/hooks/__tests__/use-prompter-sse.test.tsx diff --git a/panel/src/hooks/__tests__/use-prompter-sse.test.tsx b/panel/src/hooks/__tests__/use-prompter-sse.test.tsx new file mode 100644 index 00000000..10a0c221 --- /dev/null +++ b/panel/src/hooks/__tests__/use-prompter-sse.test.tsx @@ -0,0 +1,159 @@ +import { describe, expect, it, vi, beforeEach, afterEach } from "vitest"; +import { act, renderHook, waitFor } from "@testing-library/react"; + +// `prompter-live` is mocked so `start` resolves immediately with a fixed session +// id (no network); `streamUrl` + `LIVE_EVENT_KINDS` stay real so the hook wires +// the EventSource exactly as in production. +vi.mock("@/lib/api/prompter-live", async (importOriginal) => { + const actual = + await importOriginal(); + return { + ...actual, + prompterLiveApi: { + ...actual.prompterLiveApi, + start: vi.fn(async () => ({ session_id: "sess-1" })), + stop: vi.fn(async () => undefined), + status: vi.fn(async () => ({ alive: false })), + sendMessage: vi.fn(async () => undefined), + }, + }; +}); + +// `useProjects` would otherwise fire a real fetch on mount. +vi.mock("@/hooks/use-projects", () => ({ + useProjects: () => ({ data: [] }), +})); + +import { usePrompter } from "@/hooks/use-prompter"; + +// --------------------------------------------------------------------------- +// A minimal EventSource double. jsdom has no native EventSource. The double +// records listeners per event kind (as the hook registers them) and lets the +// test dispatch a TRANSPORT-level error — a plain `Event` with NO `data` +// (exactly what a dropped connection / dead session fires in a real browser, +// distinct from a server-sent `event: error` MessageEvent which carries JSON). +// --------------------------------------------------------------------------- +class MockEventSource { + static instances: MockEventSource[] = []; + url: string; + readyState = 1; // OPEN + private listeners: Record void>> = {}; + onerror: ((e: Event) => void) | null = null; + + constructor(url: string) { + this.url = url; + this.readyState = 1; + MockEventSource.instances.push(this); + } + addEventListener(type: string, fn: (e: Event) => void) { + (this.listeners[type] ??= []).push(fn); + } + removeEventListener(type: string, fn: (e: Event) => void) { + this.listeners[type] = (this.listeners[type] ?? []).filter((f) => f !== fn); + } + close() { + this.readyState = 2; // CLOSED + } + /** Fire a transport-level error: a plain Event with no `data` (no JSON). */ + fireTransportError() { + this.readyState = 2; + const ev = new Event("error"); + for (const fn of this.listeners["error"] ?? []) fn(ev); + if (this.onerror) this.onerror(ev); + } + /** Fire a server-sent error event: a MessageEvent carrying JSON data. */ + fireServerSentError(text: string) { + const ev = new MessageEvent("error", { + data: JSON.stringify({ kind: "error", text }), + }); + for (const fn of this.listeners["error"] ?? []) fn(ev); + } +} + +const ORIGINAL_EVENT_SOURCE = globalThis.EventSource; + +describe("usePrompter — SSE transport-error handling (F021)", () => { + beforeEach(() => { + MockEventSource.instances = []; + globalThis.EventSource = MockEventSource as unknown as typeof EventSource; + localStorage.clear(); + }); + afterEach(() => { + globalThis.EventSource = ORIGINAL_EVENT_SOURCE; + }); + + it("isSending is true once a turn is streaming over SSE", async () => { + const { result } = renderHook(() => usePrompter()); + // Scope the chat to a project + opening message so isFormValid() is true. + act(() => { + result.current.setProjectId("proj-1"); + result.current.setInitialMessage("Build me a timestamp footer"); + }); + await act(async () => { + await result.current.start(); + }); + expect(MockEventSource.instances).toHaveLength(1); + await waitFor(() => { + expect(result.current.isSending).toBe(true); + }); + }); + + it("a dropped/dead SSE connection resets isSending so the composer is not stuck", async () => { + const { result } = renderHook(() => usePrompter()); + act(() => { + result.current.setProjectId("proj-1"); + result.current.setInitialMessage("Build me a timestamp footer"); + }); + await act(async () => { + await result.current.start(); + }); + await waitFor(() => { + expect(result.current.isSending).toBe(true); + }); + + // The SSE connection drops (or the server closed the dead session): the + // EventSource fires a transport-level error with no JSON data. Before the + // fix this was swallowed by the JSON-parse try/catch and isSending stayed + // true — the composer was permanently disabled. + act(() => { + MockEventSource.instances[0]!.fireTransportError(); + }); + + await waitFor(() => { + expect(result.current.isSending).toBe(false); + }); + // The user is told the connection died (not a silent hang). + expect(result.current.messages.some((m) => m.role === "error")).toBe(true); + // The dead stream was closed so EventSource doesn't loop-reconnect a + // session that no longer exists. + expect(MockEventSource.instances[0]!.readyState).toBe(2); + }); + + it("a server-sent error event (JSON data) is still handled as before", async () => { + const { result } = renderHook(() => usePrompter()); + act(() => { + result.current.setProjectId("proj-1"); + result.current.setInitialMessage("Build me a timestamp footer"); + }); + await act(async () => { + await result.current.start(); + }); + await waitFor(() => { + expect(result.current.isSending).toBe(true); + }); + + // A server-sent `event: error` carries JSON — the existing handler path. + act(() => { + MockEventSource.instances[0]!.fireServerSentError("the agent blew up"); + }); + + await waitFor(() => { + expect(result.current.isSending).toBe(false); + }); + expect( + result.current.messages.some((m) => + m.content.includes("the agent blew up"), + ), + ).toBe(true); + }); +}); diff --git a/panel/src/hooks/use-prompter.ts b/panel/src/hooks/use-prompter.ts index fd3f19a4..33001047 100644 --- a/panel/src/hooks/use-prompter.ts +++ b/panel/src/hooks/use-prompter.ts @@ -604,11 +604,38 @@ export function usePrompter() { sourceRef.current = null; }, []); + // A dropped connection or a session the server already tore down fires a + // transport-level `error` event on the EventSource itself — a plain Event + // with NO JSON payload. That is distinct from a server-sent `event: error` + // frame (a MessageEvent carrying a LiveEvent). Without handling it the + // transport error was swallowed by the JSON-parse try/catch in openStream + // and `isSending` stayed true — the composer was permanently disabled and + // the dead EventSource loop-reconnected a session that no longer existed. + // Reset the turn, tell the user, and close the dead stream. + const handleTransportError = useCallback(() => { + streamingIdRef.current = null; + setActivity(null); + setIsSending(false); + addMessage({ + role: "error", + content: + "Live connection lost — the chat session is no longer reachable. " + + "Start a new chat to continue.", + }); + // Keep a draft/batch preview up so the human can still act on a proposed + // card; otherwise land on the stable chat state. + setState((s) => + s === "draft_preview" || s === "batch_preview" ? s : "chatting", + ); + closeStream(); + }, [addMessage, closeStream]); + const openStream = useCallback( (sid: string) => { closeStream(); const es = new EventSource(prompterLiveApi.streamUrl(sid)); for (const kind of LIVE_EVENT_KINDS) { + if (kind === "error") continue; // error is dual-purpose — handled below es.addEventListener(kind, (e: MessageEvent) => { try { handleEvent(JSON.parse(e.data) as LiveEvent); @@ -617,9 +644,27 @@ export function usePrompter() { } }); } + // `error` is dual-purpose. A server-sent `event: error` frame carries a + // JSON LiveEvent (dispatched as a MessageEvent with string `data`) → + // route it through handleEvent like any other kind. A transport-level + // error (dropped connection / dead session) fires a plain Event with no + // `data` → JSON.parse would swallow it and leave isSending stuck, so + // route the no-payload case to the transport-error reset instead. + es.addEventListener("error", (e: Event) => { + const data = (e as MessageEvent).data; + if (typeof data === "string") { + try { + handleEvent(JSON.parse(data) as LiveEvent); + } catch { + // A malformed server-sent error frame is dropped; stream stays open. + } + } else { + handleTransportError(); + } + }); sourceRef.current = es; }, - [closeStream, handleEvent], + [closeStream, handleEvent, handleTransportError], ); // Best-effort reap if the user navigates away mid-chat. This cleanup runs on