mirror of
https://github.com/rennf93/roboco.git
synced 2026-08-03 07:23:24 +02:00
[F021] handle SSE transport errors so the intake composer isn't stuck
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.
This commit is contained in:
@@ -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<typeof import("@/lib/api/prompter-live")>();
|
||||||
|
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<string, Array<(e: Event) => 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);
|
||||||
|
});
|
||||||
|
});
|
||||||
@@ -604,11 +604,38 @@ export function usePrompter() {
|
|||||||
sourceRef.current = null;
|
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(
|
const openStream = useCallback(
|
||||||
(sid: string) => {
|
(sid: string) => {
|
||||||
closeStream();
|
closeStream();
|
||||||
const es = new EventSource(prompterLiveApi.streamUrl(sid));
|
const es = new EventSource(prompterLiveApi.streamUrl(sid));
|
||||||
for (const kind of LIVE_EVENT_KINDS) {
|
for (const kind of LIVE_EVENT_KINDS) {
|
||||||
|
if (kind === "error") continue; // error is dual-purpose — handled below
|
||||||
es.addEventListener(kind, (e: MessageEvent) => {
|
es.addEventListener(kind, (e: MessageEvent) => {
|
||||||
try {
|
try {
|
||||||
handleEvent(JSON.parse(e.data) as LiveEvent);
|
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;
|
sourceRef.current = es;
|
||||||
},
|
},
|
||||||
[closeStream, handleEvent],
|
[closeStream, handleEvent, handleTransportError],
|
||||||
);
|
);
|
||||||
|
|
||||||
// Best-effort reap if the user navigates away mid-chat. This cleanup runs on
|
// Best-effort reap if the user navigates away mid-chat. This cleanup runs on
|
||||||
|
|||||||
Reference in New Issue
Block a user