From 174af3cb61ea398f8cfba025cd87965388248bf2 Mon Sep 17 00:00:00 2001 From: dwelle <5153846+dwelle@users.noreply.github.com> Date: Sat, 18 Jul 2026 21:18:24 +0200 Subject: [PATCH] tta: drain the SSE body to EOF after terminal chunks MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Reads the trailing [DONE] sentinel + EOF (detached, 2s-bounded) so the response body completes instead of being abandoned mid-stream, letting the keep-alive connection be reused. Note: this does NOT remove the net::ERR_ABORTED annotation DevTools attaches to these requests — verified via A/B that Chrome logs it for cross-origin SSE fetches even when the body is fully read; that annotation is a reporting quirk, not an actual abort. Co-Authored-By: Claude Fable 5 --- packages/excalidraw/TTA/client.test.ts | 43 ++++++++++++++++++++++++++ packages/excalidraw/TTA/client.ts | 40 ++++++++++++++++++++++++ tta_rewrite_final.md | 2 +- 3 files changed, 84 insertions(+), 1 deletion(-) diff --git a/packages/excalidraw/TTA/client.test.ts b/packages/excalidraw/TTA/client.test.ts index c9ecbda129..bb21b6544b 100644 --- a/packages/excalidraw/TTA/client.test.ts +++ b/packages/excalidraw/TTA/client.test.ts @@ -335,6 +335,49 @@ describe("TTAStreamFetch", () => { expect(result.error?.message).toMatch(/ended the stream/i); }); + it("drains the stream to EOF after the done chunk so the fetch completes cleanly", async () => { + const encoder = new TextEncoder(); + const chunks = [ + createChunk({ + type: "done", + lifecycleStatus: "completed", + finishReason: "stop", + skeletons: [], + }), + "data: [DONE]\n\n", + ]; + let index = 0; + let fullyRead = false; + const body = new ReadableStream({ + pull(controller) { + if (index >= chunks.length) { + fullyRead = true; + controller.close(); + return; + } + controller.enqueue(encoder.encode(chunks[index])); + index++; + }, + }); + + global.fetch = vi.fn().mockResolvedValue({ + ok: true, + headers: new Headers(), + body, + }); + + const result = await TTAStreamFetch({ + payload: { prompt: "hello" }, + fetch: createTransportFetch(), + }); + + // the result resolves at the `done` frame, before the trailing sentinel… + expect(result.error).toBeNull(); + // …and the detached drain then reads the body to EOF (no ERR_ABORTED) + await new Promise((resolve) => setTimeout(resolve, 300)); + expect(fullyRead).toBe(true); + }); + it("returns an abort error when the request is cancelled", async () => { const abortController = new AbortController(); diff --git a/packages/excalidraw/TTA/client.ts b/packages/excalidraw/TTA/client.ts index 452adc77af..b0693d38c2 100644 --- a/packages/excalidraw/TTA/client.ts +++ b/packages/excalidraw/TTA/client.ts @@ -179,6 +179,44 @@ export interface TTADefaultTransportAdapterConfig { truncate: TTATruncateResponseFetch; } +/** + * After a terminal `done`/`error` chunk the response body still holds a few + * unread frames (the `[DONE]` sentinel, then EOF). Read them so the stream + * completes instead of being abandoned mid-body — an abandoned body forces + * the browser to tear down the connection, while a drained one can return to + * the keep-alive pool. Detached and bounded: the result is resolved before + * this runs, and a server that doesn't close promptly gets cancelled after + * the deadline (the old behavior). + * + * NOTE this does NOT silence the `net::ERR_ABORTED` annotation DevTools + * attaches to these requests — verified empirically that Chrome logs it for + * cross-origin SSE fetches even when the body is read fully to EOF. + */ +const drainStreamToEof = (body: ReadableStream) => { + void (async () => { + try { + // the for-await teardown releases the SSE parser's reader lock + // asynchronously — wait briefly for it + for (let i = 0; i < 10 && body.locked; i++) { + await new Promise((resolve) => setTimeout(resolve, 10)); + } + if (body.locked) { + return; + } + const reader = body.getReader(); + const deadline = setTimeout(() => { + reader.cancel().catch(() => {}); + }, 2_000); + while (!(await reader.read()).done) { + // discard trailing frames + } + clearTimeout(deadline); + } catch { + // teardown races are fine — nothing user-visible depends on the drain + } + })(); +}; + const toStreamError = ( message: string, code?: AI_ERROR_CODE | number, @@ -275,6 +313,7 @@ export const TTAStreamFetch = async ( }); break; case "done": + drainStreamToEof(stream); return { ...rateLimitInfo, finalPayload: { @@ -294,6 +333,7 @@ export const TTAStreamFetch = async ( error: null, }; case "error": + drainStreamToEof(stream); return { ...rateLimitInfo, error: toStreamError( diff --git a/tta_rewrite_final.md b/tta_rewrite_final.md index 733fdb677e..b256f8212e 100644 --- a/tta_rewrite_final.md +++ b/tta_rewrite_final.md @@ -267,7 +267,7 @@ Steps 1–2 clear every 🔴 client bug with no data-model change. Steps 3–5 a **Progress log** -- ✅ **Live QA of steps 0–2 (2026-07-18, localhost:3005 + dev oss-ai-server)**: basic generation + follow-up replacement ✓; C1 Enter-while-streaming is a no-op keeping composer text ✓; Stop mid-stream commits the partial (flag stripped, "Stopped after", To canvas/Retry) ✓; C2 via spliced stream (clean EOF, no `done`) → "Connection to the AI service was interrupted" + Retry, previous generation preserved (failed gen never rendered → M8 sub-case), replacement queue survived the failure and the retry's success replaced correctly ✓; N1 retry payload verified on the wire: `retry.reason: "generation_error"` with **no** `retryAssistantMessageId` for a never-succeeded turn, retry completed (no 400) ✓; N2 chat switch mid-stream auto-stops ("Generation interrupted", partial committed) ✓. Note: successful SSE requests show cosmetic `net::ERR_ABORTED` in DevTools because the client returns at `done` without draining the `[DONE]` sentinel — pre-existing, harmless. +- ✅ **Live QA of steps 0–2 (2026-07-18, localhost:3005 + dev oss-ai-server)**: basic generation + follow-up replacement ✓; C1 Enter-while-streaming is a no-op keeping composer text ✓; Stop mid-stream commits the partial (flag stripped, "Stopped after", To canvas/Retry) ✓; C2 via spliced stream (clean EOF, no `done`) → "Connection to the AI service was interrupted" + Retry, previous generation preserved (failed gen never rendered → M8 sub-case), replacement queue survived the failure and the retry's success replaced correctly ✓; N1 retry payload verified on the wire: `retry.reason: "generation_error"` with **no** `retryAssistantMessageId` for a never-succeeded turn, retry completed (no 400) ✓; N2 chat switch mid-stream auto-stops ("Generation interrupted", partial committed) ✓. Note: successful SSE requests show cosmetic `net::ERR_ABORTED` in DevTools. **Correction after follow-up investigation (2026-07-18):** this is a Chromium/CDP reporting quirk for cross-origin SSE fetches, _not_ caused by abandoning the body — verified by A/B comparison (a fully-drained-to-EOF stream logs the same annotation). Unfixable client-side; a bounded detached post-terminal drain was kept anyway (honest win: the keep-alive connection can be reused instead of torn down) with a regression test pinning full consumption. - ✅ **Step 2 landed 2026-07-05**: new `useGenerationSlot` hook (ownership-checked single-flight slot) + a `runGeneration` owner in TTADialog — the sole entry point for send and retry — that reserves the slot synchronously before any chat mutation (C1: a send while streaming is a no-op that keeps the composer draft), with one shared catch that patches the generation's own bubble or appends an error bubble (C3). Retry goes through the owner with `replaceActive` (cancel-and-replace, preserved semantics incl. `MIN_RETRYING_VISIBLE_MS` and reuse-vs-replace bubbles). **N1 fix**: new persisted `lastCompletedMessageId` field (stamped from every `done`, surviving failed retries, seeded onto regenerate's replacement bubble, carried through the turns round-trip) is now the retry target — with a legacy-data fallback to `messageId` for cleanly-completed bubbles — and is omitted when the turn never succeeded (server starts a fresh turn). **N2 fix**: chat switch and active-chat delete now run `stopActiveGeneration("interrupted")` (extracted full-Stop semantics: abort + commit draft + free slot + mark bubble) before proceeding; message-delete keeps its no-commit teardown but now cancels pending renders and frees the slot. Lifecycle: `generateResponse`'s `finally` is ownership-checked (a canceled predecessor can no longer clobber the successor's controller/throttle/stop state — a live race today) and each stream starts from a reset throttle state. `isSendingChat` state + all 8 scattered `setIsSendingChat`/`setStopRequested` call sites collapsed into the owner. Tests: 3 `useGenerationSlot` unit tests (double-acquire no-op, release, stale-release ignored); dialog-level interaction tests (switch-mid-stream, retry payload) deferred to the step-7 `useTTAChatActions` extraction where the actions become testable without mounting the full dialog — cover via live QA meanwhile. 39 passing, typecheck clean. - ✅ **Step 1 landed 2026-07-05** (commit follows step 0's): success now requires a `done` frame — EOF/`[DONE]`-without-`done` returns `STREAM_INTERRUPTED` (1002, two log-level message flavors, user copy = existing connection/offline keys); on-error canvas policy switched from wipe to commit-like-Stop (both the transport-error branch and the thrown-`INVALID_RESULT` catch), with error-retry now queueing the failed generation's tag for replacement; salvage UI included (errored bubbles with partial skeletons render their thumbnail + "To canvas" — formerly tta_c2.md Phase 2, made mandatory by the commit policy so bubble/canvas agree); `partial.isComplete` forwarded (the lifecycle explicitly renders partials as non-final to avoid double-committing on the server's final partial); `done.finishReason` carried and `length`/`content_filter` surface as a `truncatedResponse` status line (full warning treatment arrives with the step-3 union); truncate response validated with the real `{ ok, chatId, updatedAt? }` shape (M4); deleted the dead `[ai-server]` filter and changed-chatId re-`onStarted` branch; N5 spinner-forever hole closed. Deliberate deviation from Appendix A: `lifecycleStatus` optionality on client chunk types stays defensive until the shared-types work (§7.5). Tests: `client.test.ts` EOF-as-success case inverted + 2 new interrupt cases + isComplete-forwarding case, 2 new `sse.test.ts` sentinel cases, 1 new lifecycle interrupted-stream case — 36 passing, typecheck clean, TTD untouched.