mirror of
https://github.com/excalidraw/excalidraw.git
synced 2026-10-21 08:36:05 +00:00
tta: drain the SSE body to EOF after terminal chunks
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 <noreply@anthropic.com>
This commit is contained in:
@@ -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<Uint8Array>({
|
||||
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();
|
||||
|
||||
|
||||
@@ -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<Uint8Array>) => {
|
||||
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(
|
||||
|
||||
@@ -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.
|
||||
|
||||
Reference in New Issue
Block a user