fix(client): реальный обрыв SSE → reconnect-лесенка, не терминальный error (#488)
Браузерный QA поймал баг, который юнит-тесты пропускали из-за вакуумной SDK-формы.
Премиса (подтверждена по ai@6.0.207 AbstractChat.makeRequest, catch ~13763): при
сетевом дропе SDK ставит isError=true БЕЗУСЛОВНО, а isError=true → isDisconnect=true
ставится РЯДОМ (только для fetch/network TypeError). Т.е. реальный обрыв ВСЕГДА
даёт { isError:true, isDisconnect:true }; форма { isDisconnect:true, isError:false }
SDK'ом НЕ эмитится.
Баг: в onFinish проверка `if (isError) return` стояла РАНЬШЕ ветки isDisconnect →
реальный дроп уходил в терминальный error-баннер, а FINISH_DISCONNECT (единственный
вход в reconnect-лесенку) не диспатчился НИКОГДА. Сценарии 1/2 (commit 2 «обрыв до
первого кадра» и commit 3 «два обрыва») в браузере не работали.
Фикс: маршрутизация по disconnect ПЕРВЫМ: isDisconnect → FINISH_DISCONNECT
(reconnect); НЕ-disconnect error (isError && !isDisconnect, напр. провайдерский 500)
→ FINISH_ERROR (терминал); затем isAbort; затем clean. Порядок веток supersede-
блока тоже disconnect-first (для консистентности; там всё равно всё дропается I1).
Инварианты сохранены: epoch-штамп turnEpochRef на всех FINISH_*; honor-in-stopping
в редьюсере честит ЛЮБОЙ финиш в фазе stopping → на пути Stop дроп-финиш уходит в
idle, а не в ложный reconnect (новый тест). F1 supersede-drop чужого поколения цел.
Тесты сделаны НЕ вакуумными: дроп подаётся реальной формой { isError:true,
isDisconnect:true }, терминальная ошибка — { isError:true, isDisconnect:false }.
MUTATION-VERIFY: багованный isError-first порядок → 6 reconnect-тестов краснеют
(нет баннера «reconnecting»); с фиксом зелены. Полный ai-chat 35 файлов / 377 / 0.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -248,14 +248,15 @@ describe("ChatThread — send now", () => {
|
||||
startLocalStreamWithRun();
|
||||
fireEvent.click(screen.getByTestId("queue-btn"));
|
||||
fireEvent.click(screen.getByLabelText("Send now")); // -> superseding, aborts A
|
||||
// A ends via isDisconnect (the server CAS closed it). The OLD overlap bug would
|
||||
// route the LIVE new run into a false reconnect banner.
|
||||
// A ends via a REAL network drop { isError:true, isDisconnect:true } (the server
|
||||
// CAS closed it). The OLD overlap bug would route the LIVE new run into a false
|
||||
// reconnect banner.
|
||||
await act(async () => {
|
||||
h.state.onFinish?.({
|
||||
message: { id: "a1", role: "assistant", parts: [{ type: "text", text: "x" }] },
|
||||
isAbort: false,
|
||||
isDisconnect: true,
|
||||
isError: false,
|
||||
isError: true,
|
||||
});
|
||||
await Promise.resolve();
|
||||
});
|
||||
@@ -328,6 +329,23 @@ describe("ChatThread — send now", () => {
|
||||
expect(screen.getAllByLabelText("Remove queued message")).toHaveLength(1);
|
||||
});
|
||||
|
||||
it("Stop then a REAL network-drop finish exits to idle (honor-in-stopping), NOT a false reconnect", () => {
|
||||
// Regression for the disconnect-first reorder: on the STOP path, even a drop-
|
||||
// form finish { isError:true, isDisconnect:true } arriving in `stopping` must be
|
||||
// HONORED (reducer) and exit to idle — it must NOT enter the reconnect ladder.
|
||||
startLocalStreamWithRun(); // live local stream, autonomous
|
||||
fireEvent.click(screen.getByLabelText("Stop")); // STOP_REQUESTED -> stopping
|
||||
act(() => {
|
||||
h.state.onFinish?.({
|
||||
message: { id: "a1", role: "assistant", parts: [] },
|
||||
isAbort: false,
|
||||
isDisconnect: true,
|
||||
isError: true,
|
||||
});
|
||||
});
|
||||
expect(screen.queryByText(/reconnecting/i)).toBeNull();
|
||||
});
|
||||
|
||||
it("Send now is HIDDEN while observing a resumed run and VISIBLE on a local stream", () => {
|
||||
// Resumed (mount attach) -> observer -> hidden.
|
||||
renderThread({ autonomousRunsEnabled: true, initialRows: streamingTail() });
|
||||
@@ -419,15 +437,18 @@ describe("ChatThread — turn-end decision (onFinish)", () => {
|
||||
expect(screen.getByText("Response stopped.")).toBeTruthy();
|
||||
});
|
||||
|
||||
it("ENDS — keeps the queue on a disconnect (non-autonomous) and shows the notice", () => {
|
||||
finishWith({ isDisconnect: true });
|
||||
it("ENDS — keeps the queue on a REAL disconnect (non-autonomous) and shows the notice", () => {
|
||||
// Real SDK drop form: { isError:true, isDisconnect:true }. With the buggy
|
||||
// isError-first order this would fall to the terminal error branch (no notice).
|
||||
finishWith({ isDisconnect: true, isError: true });
|
||||
expect(h.state.sendMessage).not.toHaveBeenCalled();
|
||||
expect(
|
||||
screen.getByText("Connection lost — the answer was interrupted."),
|
||||
).toBeTruthy();
|
||||
});
|
||||
|
||||
it("ENDS — keeps the queue on a stream error (no notice)", () => {
|
||||
it("ENDS — keeps the queue on a NON-disconnect stream error (no notice)", () => {
|
||||
// A provider error is { isError:true, isDisconnect:false } — terminal.
|
||||
finishWith({ isError: true });
|
||||
expect(h.state.sendMessage).not.toHaveBeenCalled();
|
||||
expect(screen.queryByText("Response stopped.")).toBeNull();
|
||||
@@ -437,7 +458,7 @@ describe("ChatThread — turn-end decision (onFinish)", () => {
|
||||
for (const flags of [
|
||||
{},
|
||||
{ isAbort: true },
|
||||
{ isDisconnect: true },
|
||||
{ isDisconnect: true, isError: true },
|
||||
{ isError: true },
|
||||
]) {
|
||||
cleanup();
|
||||
@@ -787,13 +808,18 @@ describe("ChatThread — live reconnect + stalled", () => {
|
||||
cleanup();
|
||||
});
|
||||
|
||||
// A REAL live SSE drop. ai@6.0.207 emits BOTH { isError:true, isDisconnect:true }
|
||||
// for a network TypeError — NOT the { isError:false } form the old tests fed. This
|
||||
// is the form browser QA hit; with the buggy isError-first routing these tests go
|
||||
// red (a real drop would surface the terminal error banner, not the reconnect
|
||||
// ladder). MUTATION-VERIFY of the disconnect-first fix.
|
||||
function disconnect(message: unknown = liveMsg) {
|
||||
act(() => {
|
||||
h.state.onFinish?.({
|
||||
message,
|
||||
isAbort: false,
|
||||
isDisconnect: true,
|
||||
isError: false,
|
||||
isError: true,
|
||||
});
|
||||
});
|
||||
}
|
||||
|
||||
@@ -626,10 +626,13 @@ export default function ChatThread({
|
||||
machineRef.current.phase.name === "superseding" &&
|
||||
pendingSupersedeTextRef.current !== null
|
||||
) {
|
||||
if (isError) dispatch({ type: "FINISH_ERROR", kind: "stream", epoch: stampEpoch });
|
||||
else if (isAbort) dispatch({ type: "FINISH_ABORT", epoch: stampEpoch });
|
||||
else if (isDisconnect)
|
||||
// Disconnect-first (see the routing note below): a real drop is
|
||||
// { isError:true, isDisconnect:true }. All of these are dropped by I1 here
|
||||
// (superseding bumped the epoch) — the order only mirrors the main routing.
|
||||
if (isDisconnect)
|
||||
dispatch({ type: "FINISH_DISCONNECT", hasVisibleContent: msgHasVisible, epoch: stampEpoch });
|
||||
else if (isError) dispatch({ type: "FINISH_ERROR", kind: "stream", epoch: stampEpoch });
|
||||
else if (isAbort) dispatch({ type: "FINISH_ABORT", epoch: stampEpoch });
|
||||
else dispatch({ type: "FINISH_CLEAN", epoch: stampEpoch });
|
||||
const text = pendingSupersedeTextRef.current;
|
||||
pendingSupersedeTextRef.current = null;
|
||||
@@ -642,18 +645,14 @@ export default function ChatThread({
|
||||
return;
|
||||
}
|
||||
|
||||
if (isError) {
|
||||
dispatch({ type: "FINISH_ERROR", kind: "stream", epoch: stampEpoch });
|
||||
setStopNotice(null);
|
||||
return;
|
||||
}
|
||||
if (isAbort) {
|
||||
// A user Stop / an interrupt abort finished. The FSM stopping/idle exit is
|
||||
// by DATA (this terminal outcome, I4).
|
||||
dispatch({ type: "FINISH_ABORT", epoch: stampEpoch });
|
||||
setStopNotice("manual");
|
||||
return;
|
||||
}
|
||||
// #488 (browser QA): route DISCONNECT FIRST. In ai@6.0.207 a real network drop
|
||||
// yields BOTH `{ isError:true, isDisconnect:true }` — the SDK sets `isError`
|
||||
// unconditionally in its catch and `isDisconnect` only ALONGSIDE it (for a
|
||||
// fetch/network TypeError). Checking `isError` first therefore sent EVERY real
|
||||
// drop to the terminal error banner and NEVER entered the reconnect ladder
|
||||
// (`FINISH_DISCONNECT` is its only entry). A disconnect — the detached run
|
||||
// keeps executing server-side — must win; only a NON-disconnect error (a
|
||||
// provider 500, `{ isError:true, isDisconnect:false }`) is terminal.
|
||||
if (isDisconnect) {
|
||||
if (wasObserver) {
|
||||
// A resumed/attached OBSERVER stream dropped. Recover via the degraded
|
||||
@@ -722,6 +721,19 @@ export default function ChatThread({
|
||||
}
|
||||
return;
|
||||
}
|
||||
// A NON-disconnect stream error (a provider 500 etc.) -> terminal error banner.
|
||||
if (isError) {
|
||||
dispatch({ type: "FINISH_ERROR", kind: "stream", epoch: stampEpoch });
|
||||
setStopNotice(null);
|
||||
return;
|
||||
}
|
||||
if (isAbort) {
|
||||
// A user Stop / an interrupt abort finished. The FSM stopping/idle exit is
|
||||
// by DATA (this terminal outcome, I4 — honored in `stopping` by the reducer).
|
||||
dispatch({ type: "FINISH_ABORT", epoch: stampEpoch });
|
||||
setStopNotice("manual");
|
||||
return;
|
||||
}
|
||||
// Clean finish.
|
||||
if (wasObserver) {
|
||||
if (mountedRef.current) {
|
||||
|
||||
Reference in New Issue
Block a user