diff --git a/.changeset/cool-streams-close.md b/.changeset/cool-streams-close.md new file mode 100644 index 0000000000..1d1a12a95f --- /dev/null +++ b/.changeset/cool-streams-close.md @@ -0,0 +1,9 @@ +--- +'@tanstack/router-core': patch +--- + +fix(router-core): stop `createRequestHandler` from cleaning up SSR state while the response body is still streaming + +When a handler callback returned a plain `Response` wrapping a stream produced by `transformStreamWithRouter` — the contract of `renderRouterToStream` up to v1.169.0 — `createRequestHandler`'s `finally` block ran `serverSsr.cleanup()` immediately, while the body was still streaming. That wiped the render-finished and serialization-finished listeners mid-flight: integrations like `@tanstack/react-router-ssr-query` never closed their dehydration query stream, the transform never received its completion signals, and the response hung until the ~60s serialization timeout (#7529). + +The stream transform now claims cleanup ownership when it attaches (`serverSsr.claimCleanup()`), and `createRequestHandler` skips its fallback cleanup when claimed — the transformed stream already cleans up when consumed, cancelled, errored, or when its lifetime expires. `onRenderFinished` listeners registered after the stream fast path is reserved are also no longer dropped. diff --git a/packages/router-core/src/router.ts b/packages/router-core/src/router.ts index 9ff7b82b05..0651b83c8d 100644 --- a/packages/router-core/src/router.ts +++ b/packages/router-core/src/router.ts @@ -799,6 +799,15 @@ export interface ServerSsr { isSerializationFinished: () => boolean /** Framework-only: atomically reserves the pass-through stream path if safe. */ reserveStreamFastPath: () => boolean + /** + * Framework-only: called by the stream transform to take ownership of + * cleanup. Once claimed, `createRequestHandler` must not run its + * fallback cleanup — the transformed stream cleans up when it is + * consumed, cancelled, or its lifetime expires. + */ + claimCleanup: () => void + /** Framework-only. */ + isCleanupClaimed: () => boolean /** Framework-only. */ onInjectedHtml: (listener: () => void) => () => void /** Framework-only. */ diff --git a/packages/router-core/src/ssr/createRequestHandler.ts b/packages/router-core/src/ssr/createRequestHandler.ts index 32377b2bea..0b4035ca49 100644 --- a/packages/router-core/src/ssr/createRequestHandler.ts +++ b/packages/router-core/src/ssr/createRequestHandler.ts @@ -67,10 +67,13 @@ export function createRequestHandler({ responseOwnsCleanup = ssrResponse.serverSsrCleanup === 'stream' return ssrResponse.response } finally { - if (!responseOwnsCleanup) { + if (!responseOwnsCleanup && !router.serverSsr?.isCleanupClaimed()) { // Clean up router SSR state if the callback won't handle it // (e.g., if an error occurred before the callback was invoked). - // Transformed streaming response bodies clean up when consumed/cancelled. + // Transformed streaming response bodies clean up when + // consumed/cancelled — both when wrapped via createSsrStreamResponse + // and when a transform claimed cleanup ownership directly (a plain + // Response wrapping a transformed stream, the pre-1.170 contract). router.serverSsr?.cleanup() } } diff --git a/packages/router-core/src/ssr/ssr-server.ts b/packages/router-core/src/ssr/ssr-server.ts index 31db2f5e41..e0e29a5051 100644 --- a/packages/router-core/src/ssr/ssr-server.ts +++ b/packages/router-core/src/ssr/ssr-server.ts @@ -445,6 +445,7 @@ export function attachRouterServerSsrUtils({ let _dehydrated = false let _serializationFinished = false let streamFastPathReserved = false + let cleanupClaimed = false const renderFinishedListeners: Array<() => void> = [] const injectedHtmlListeners: Array<() => void> = [] const serializationFinishedListeners: Array<() => void> = [] @@ -617,6 +618,12 @@ export function attachRouterServerSsrUtils({ isSerializationFinished() { return _serializationFinished }, + claimCleanup() { + cleanupClaimed = true + }, + isCleanupClaimed() { + return cleanupClaimed + }, reserveStreamFastPath() { if ( !cleanupStarted && @@ -637,7 +644,10 @@ export function attachRouterServerSsrUtils({ return () => removeListener(injectedHtmlListeners, listener) }, onRenderFinished: (listener) => { - if (cleanupStarted || streamFastPathReserved) return + if (cleanupStarted) return + // Register even when the fast path is reserved: it still calls + // setRenderFinished() at the end of the app stream. Dropping listeners + // here left router-ssr-query's query stream open, hanging SSR (#7529). renderFinishedListeners.push(listener) }, onSerializationFinished: (listener) => { diff --git a/packages/router-core/src/ssr/transformStreamWithRouter.ts b/packages/router-core/src/ssr/transformStreamWithRouter.ts index 4393f955b3..3715030046 100644 --- a/packages/router-core/src/ssr/transformStreamWithRouter.ts +++ b/packages/router-core/src/ssr/transformStreamWithRouter.ts @@ -205,6 +205,14 @@ export function transformStreamWithRouter( if (!serverSsr) { throw new Error('Invariant failed: router.serverSsr is required') } + // The transformed stream owns cleanup from here on: it cleans up when + // consumed, cancelled, errored, or when its lifetime expires. Claiming + // stops createRequestHandler's fallback cleanup from tearing down SSR + // state while the response body is still streaming (#7529) — that wiped + // the render-finished + serialization-finished listeners mid-flight, so + // integrations like router-ssr-query never closed their query stream and + // the response hung until the serialization timeout. + serverSsr.claimCleanup() if (serverSsr.reserveStreamFastPath()) { return makeFastPathStream(appStream, opts, serverSsr) } diff --git a/packages/router-core/tests/ssr-server-cleanup.test.ts b/packages/router-core/tests/ssr-server-cleanup.test.ts index 0cc8832abd..76e7ba9e93 100644 --- a/packages/router-core/tests/ssr-server-cleanup.test.ts +++ b/packages/router-core/tests/ssr-server-cleanup.test.ts @@ -190,6 +190,36 @@ describe('serverSsr.cleanup', () => { router.serverSsr?.cleanup() }) + test('onRenderFinished listener registered after fast path reserve still fires', async () => { + // Regression test for #7529: when the fast path is reserved before an + // integration (e.g. router-ssr-query) registers its onRenderFinished + // listener, the listener must not be dropped - otherwise the query stream + // is never closed and the response hangs until the serialization timeout. + // The fast path still calls setRenderFinished() at the end of the app + // stream, so the listener fires at that point. + const router = buildRouter() + attachRouterServerSsrUtils({ router, manifest: undefined }) + + await router.load() + await router.serverSsr!.dehydrate() + router.serverSsr!.takeBufferedScripts() + + expect(router.serverSsr!.reserveStreamFastPath()).toBe(true) + + let renderFinishedCalls = 0 + router.serverSsr!.onRenderFinished(() => { + renderFinishedCalls++ + }) + // Not invoked at registration time - the fast path defers to the app + // stream end, mirrored here by an explicit setRenderFinished(). + expect(renderFinishedCalls).toBe(0) + + router.serverSsr!.setRenderFinished() + expect(renderFinishedCalls).toBe(1) + + router.serverSsr?.cleanup() + }) + test('stream fast path rejects while SSR work is pending', async () => { const value = deferred() const router = buildRouter({ value: value.promise }) @@ -377,4 +407,63 @@ describe('serverSsr.cleanup', () => { expect(cleanupCalls).toBe(1) expect(router.serverSsr).toBeUndefined() }) + + test('request handler defers cleanup for a plain Response wrapping a transformed stream (#7529)', async () => { + // Regression test for the production path of #7529: an entry server that + // returns `new Response(transformStreamWithRouter(...))` directly — the + // contract of v1.169.0's own renderRouterToStream. Without the cleanup + // claim, the handler's `finally` tore down SSR state while the body was + // still streaming: render-finished + serialization-finished listeners + // were wiped, router-ssr-query's close listener never fired, and the + // response hung until the serialization timeout. + const router = buildRouter() + let cleanupCalls = 0 + let renderFinishedCalls = 0 + let controller!: ReadableStreamDefaultController + const handler = createRequestHandler({ + createRouter: () => router, + request: new Request('http://localhost/'), + }) + + const response = await handler(({ router: requestRouter }) => { + const serverSsr = requestRouter.serverSsr! + const cleanup = serverSsr.cleanup + serverSsr.cleanup = () => { + cleanupCalls++ + cleanup() + } + // Simulates router-ssr-query's dehydration-stream close listener, + // registered before the transform attaches. + serverSsr.onRenderFinished(() => { + renderFinishedCalls++ + }) + const appStream = new ReadableStream({ + start(c) { + controller = c + }, + }) + const responseStream = transformStreamWithRouter( + requestRouter, + appStream as any, + ) + + // Plain Response — NOT wrapped in createSsrStreamResponse. + return Promise.resolve(new Response(responseStream as any)) + }) + + // The transform claimed cleanup: nothing may tear down SSR state while + // the body is still streaming. + expect(cleanupCalls).toBe(0) + expect(renderFinishedCalls).toBe(0) + + controller.enqueue(new TextEncoder().encode('ok')) + controller.close() + const body = await response.text() + + expect(body).toContain('') + expect(renderFinishedCalls).toBe(1) + expect(cleanupCalls).toBe(1) + expect(router.serverSsr).toBeUndefined() + }) + }) diff --git a/packages/router-core/tests/transformStreamWithRouter.test.ts b/packages/router-core/tests/transformStreamWithRouter.test.ts index 13b0072e24..74dd4774f6 100644 --- a/packages/router-core/tests/transformStreamWithRouter.test.ts +++ b/packages/router-core/tests/transformStreamWithRouter.test.ts @@ -25,6 +25,7 @@ const MAX_ROUTER_HTML_CHARS = 16 * 1024 * 1024 type FakeServerSsr = { isSerializationFinished: () => boolean + claimCleanup: () => void reserveStreamFastPath: () => boolean onInjectedHtml: (listener: () => void) => () => void onSerializationFinished: (listener: () => void) => () => void @@ -53,6 +54,7 @@ function makeRouter(opts: Partial = {}): { const router: FakeRouter = { serverSsr: { isSerializationFinished: () => false, + claimCleanup: () => {}, reserveStreamFastPath: () => false, onInjectedHtml: (cb) => { injectedListeners.push(cb)