From 166220583a7677f0b9904856f51958bca39c4429 Mon Sep 17 00:00:00 2001 From: JPeer264 Date: Mon, 31 Aug 2026 15:37:21 +0200 Subject: [PATCH] fix(server-utils): Make trace meta tag injection chunk-safe and back-pressured Astro, SolidStart and TanStack Start each carried their own copy of the same head injection, and each copy had the same two defects. This moves one implementation into `@sentry/server-utils`, which all three already depend on, and points them at it. It lives in the shared export surface, so the entry without diagnostics channels carries it too and edge runtimes can use it. The scan ran over every HTML chunk on its own, so a chunk boundary inside a quoted attribute value flipped the quote parity of the next chunk. The regex then paired the value's closing quote with a later one, swallowed `` inside the resulting phantom string, and skipped the injection with no error. React's Fizz writer flushes when its 2048 byte view fills and writes any longer string on its own, so a long attribute on `` puts a boundary right after its opening quote and the trace meta tags disappear. The duplicate guard had the same flaw: `includes('"sentry-trace"')` looked ahead within one chunk, so whether the app's own trace meta tags were seen depended on where the split fell. The shared injector anchors on `` instead, the way `getMetaTagTransformer` does in the react-router SDK, and carries the few characters that could still start either token between chunks. The closing tag cannot be confused with markup inside an attribute value, so the quote scanning goes away entirely, and everything the head contains has been seen by the time the tags are placed, which settles the duplicate guard too. The meta tags now sit at the end of `` rather than at the start, which is where the react-router SDK already puts them. The body was also pumped inside the `start` callback of a `ReadableStream`, with a `for await` loop that never consulted `desiredSize`, so it was drained as fast as it could be produced whether or not anyone was reading the wrapped response. TanStack's `transformStreamWithRouter` throttles itself against `desiredSize` on the stream we consume, so that pause never fired. The shared injector carries the consumer's backpressure through to the body being wrapped. Whether this is what produced the `SSR stream tail exceeded maximum buffer` prerender failure in the report is not established: on the request path React writes `` last and TanStack releases its tail only once the app stream ends, so read pacing alone does not change how much that tail holds. The body is pumped by hand rather than with `pipeTo`, because `pipeTo` rejects for either side failing and a consumer that navigates away would then be reported as an error on every aborted page load. Only a body that fails to read is reported; a consumer going away cancels the body and stays silent. Four smaller fixes come with it: the text decoder was never flushed, so a body ending on an incomplete byte sequence lost its tail; holding characters back between chunks can split a surrogate pair, which encodes each half to U+FFFD; the upstream `content-length` was copied onto a body that had grown, which a client or proxy would truncate; and SolidStart wrapped the tags in newlines, which is the text node in `` that React 19 whole-document hydration rejects. Both TanStack Start E2E apps get a `/split-head-chunk` route whose document carries the long attribute ahead of the head. Verified against the published SDK on workerd: the route renders a `` but no trace meta tags at all. Astro writes each template expression as its own chunk and Solid renders the shell to one string, so neither can split inside an attribute value; SolidStart gets a plain meta tag E2E test and Astro is already covered by its trace continuity test. Co-Authored-By: Claude Opus 5 Co-Authored-By: Claude Fable 5.1 --- .../tests/trace-propagation.test.ts | 11 + .../src/routes/__root.tsx | 10 +- .../src/routes/split-head-chunk.tsx | 14 ++ .../tests/trace-propagation.test.ts | 13 ++ .../tanstackstart-react/src/routes/__root.tsx | 10 +- .../src/routes/split-head-chunk.tsx | 14 ++ .../tests/trace-propagation.test.ts | 16 ++ packages/astro/src/server/middleware.ts | 82 +------ packages/astro/test/server/middleware.test.ts | 8 +- packages/server-utils/src/exports.ts | 1 + .../server-utils/src/utils/htmlInjection.ts | 171 ++++++++++++++ .../test/utils/htmlInjection.test.ts | 214 ++++++++++++++++++ packages/solidstart/src/server/middleware.ts | 25 +- .../src/server/wrapFetchWithSentry.ts | 97 +------- .../test/server/wrapFetchWithSentry.test.ts | 35 ++- 15 files changed, 514 insertions(+), 207 deletions(-) create mode 100644 dev-packages/e2e-tests/test-applications/solidstart-2/tests/trace-propagation.test.ts create mode 100644 dev-packages/e2e-tests/test-applications/tanstackstart-react-cloudflare/src/routes/split-head-chunk.tsx create mode 100644 dev-packages/e2e-tests/test-applications/tanstackstart-react/src/routes/split-head-chunk.tsx create mode 100644 packages/server-utils/src/utils/htmlInjection.ts create mode 100644 packages/server-utils/test/utils/htmlInjection.test.ts diff --git a/dev-packages/e2e-tests/test-applications/solidstart-2/tests/trace-propagation.test.ts b/dev-packages/e2e-tests/test-applications/solidstart-2/tests/trace-propagation.test.ts new file mode 100644 index 000000000000..a3a9ea3ae119 --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/solidstart-2/tests/trace-propagation.test.ts @@ -0,0 +1,11 @@ +import { expect, test } from '@playwright/test'; + +test('injects trace meta tags on pageload', async ({ page }) => { + await page.goto('/'); + + const sentryTraceContent = await page.getAttribute('meta[name="sentry-trace"]', 'content'); + expect(sentryTraceContent).toMatch(/^[a-f0-9]{32}-[a-f0-9]{16}-[01]$/); + + const baggageContent = await page.getAttribute('meta[name="baggage"]', 'content'); + expect(baggageContent).toContain('sentry-trace_id='); +}); diff --git a/dev-packages/e2e-tests/test-applications/tanstackstart-react-cloudflare/src/routes/__root.tsx b/dev-packages/e2e-tests/test-applications/tanstackstart-react-cloudflare/src/routes/__root.tsx index 539af1fa9ace..f5967ea9c750 100644 --- a/dev-packages/e2e-tests/test-applications/tanstackstart-react-cloudflare/src/routes/__root.tsx +++ b/dev-packages/e2e-tests/test-applications/tanstackstart-react-cloudflare/src/routes/__root.tsx @@ -1,5 +1,5 @@ import type { ReactNode } from 'react'; -import { Outlet, createRootRoute, HeadContent, Scripts } from '@tanstack/react-router'; +import { Outlet, createRootRoute, HeadContent, Scripts, useRouterState } from '@tanstack/react-router'; export const Route = createRootRoute({ head: () => ({ @@ -19,6 +19,10 @@ export const Route = createRootRoute({ component: RootComponent, }); +// Long enough that the SSR stream flushes a chunk boundary inside this attribute, ahead of +// the head. See https://github.com/getsentry/sentry-javascript/issues/23468. +const LONG_ATTRIBUTE = 'x'.repeat(3000); + function RootComponent() { return ( @@ -28,8 +32,10 @@ function RootComponent() { } function RootDocument({ children }: Readonly<{ children: ReactNode }>) { + const pathname = useRouterState({ select: state => state.location.pathname }); + return ( - + diff --git a/dev-packages/e2e-tests/test-applications/tanstackstart-react-cloudflare/src/routes/split-head-chunk.tsx b/dev-packages/e2e-tests/test-applications/tanstackstart-react-cloudflare/src/routes/split-head-chunk.tsx new file mode 100644 index 000000000000..e67119a00707 --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/tanstackstart-react-cloudflare/src/routes/split-head-chunk.tsx @@ -0,0 +1,14 @@ +import { createFileRoute } from '@tanstack/react-router'; + +export const Route = createFileRoute('/split-head-chunk')({ + component: SplitHeadChunk, +}); + +function SplitHeadChunk() { + return ( +
+

Split head chunk

+

The root document carries a long attribute ahead of the head, so the SSR stream splits inside it.

+
+ ); +} diff --git a/dev-packages/e2e-tests/test-applications/tanstackstart-react-cloudflare/tests/trace-propagation.test.ts b/dev-packages/e2e-tests/test-applications/tanstackstart-react-cloudflare/tests/trace-propagation.test.ts index f96ea3ce5f71..09d7c63f0c5f 100644 --- a/dev-packages/e2e-tests/test-applications/tanstackstart-react-cloudflare/tests/trace-propagation.test.ts +++ b/dev-packages/e2e-tests/test-applications/tanstackstart-react-cloudflare/tests/trace-propagation.test.ts @@ -17,6 +17,19 @@ test.describe('Trace propagation', () => { expect(baggageContent).toContain('sentry-sampled='); }); + test('should inject metatags when the SSR stream splits ahead of the head', async ({ page }) => { + await page.goto('/split-head-chunk'); + + const sentryTraceContent = await page.getAttribute('meta[name="sentry-trace"]', 'content'); + expect(sentryTraceContent).toMatch(/^[a-f0-9]{32}-[a-f0-9]{16}-[01]$/); + + const baggageContent = await page.getAttribute('meta[name="baggage"]', 'content'); + expect(baggageContent).toContain('sentry-trace_id='); + + // The attribute that forces the chunk boundary must survive the rewrite intact. + expect(await page.getAttribute('html', 'data-long')).toHaveLength(3000); + }); + test('should have trace connection between server and client', async ({ page }) => { const serverTxPromise = waitForTransaction('tanstackstart-react-cloudflare', transactionEvent => { return transactionEvent?.contexts?.trace?.op === 'http.server' && transactionEvent?.transaction === 'GET /'; diff --git a/dev-packages/e2e-tests/test-applications/tanstackstart-react/src/routes/__root.tsx b/dev-packages/e2e-tests/test-applications/tanstackstart-react/src/routes/__root.tsx index 204276e1bdf2..a141b6dbd545 100644 --- a/dev-packages/e2e-tests/test-applications/tanstackstart-react/src/routes/__root.tsx +++ b/dev-packages/e2e-tests/test-applications/tanstackstart-react/src/routes/__root.tsx @@ -1,5 +1,9 @@ import { useEffect, type ReactNode } from 'react'; -import { Outlet, createRootRoute, HeadContent, Scripts } from '@tanstack/react-router'; +import { Outlet, createRootRoute, HeadContent, Scripts, useRouterState } from '@tanstack/react-router'; + +// Long enough that the SSR stream flushes a chunk boundary inside this attribute, ahead of +// the head. See https://github.com/getsentry/sentry-javascript/issues/23468. +const LONG_ATTRIBUTE = 'x'.repeat(3000); export const Route = createRootRoute({ head: () => ({ @@ -36,8 +40,10 @@ function RootComponent() { } function RootDocument({ children }: Readonly<{ children: ReactNode }>) { + const pathname = useRouterState({ select: state => state.location.pathname }); + return ( - + diff --git a/dev-packages/e2e-tests/test-applications/tanstackstart-react/src/routes/split-head-chunk.tsx b/dev-packages/e2e-tests/test-applications/tanstackstart-react/src/routes/split-head-chunk.tsx new file mode 100644 index 000000000000..e67119a00707 --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/tanstackstart-react/src/routes/split-head-chunk.tsx @@ -0,0 +1,14 @@ +import { createFileRoute } from '@tanstack/react-router'; + +export const Route = createFileRoute('/split-head-chunk')({ + component: SplitHeadChunk, +}); + +function SplitHeadChunk() { + return ( +
+

Split head chunk

+

The root document carries a long attribute ahead of the head, so the SSR stream splits inside it.

+
+ ); +} diff --git a/dev-packages/e2e-tests/test-applications/tanstackstart-react/tests/trace-propagation.test.ts b/dev-packages/e2e-tests/test-applications/tanstackstart-react/tests/trace-propagation.test.ts index 88c4be62c120..5afb4fe545f8 100644 --- a/dev-packages/e2e-tests/test-applications/tanstackstart-react/tests/trace-propagation.test.ts +++ b/dev-packages/e2e-tests/test-applications/tanstackstart-react/tests/trace-propagation.test.ts @@ -22,6 +22,22 @@ test.describe('Trace propagation', () => { expect(baggageContent).toContain('sentry-sampled='); }); + // The SSR stream splits inside the long attribute that sits ahead of the head, so the meta + // tags are only injected if the transform carries its state across chunks. + // See https://github.com/getsentry/sentry-javascript/issues/23468. + test('should inject metatags when the SSR stream splits ahead of the head', async ({ page }) => { + await page.goto('/split-head-chunk'); + + const sentryTraceContent = await page.getAttribute('meta[name="sentry-trace"]', 'content'); + expect(sentryTraceContent).toMatch(/^[a-f0-9]{32}-[a-f0-9]{16}-[01]$/); + + const baggageContent = await page.getAttribute('meta[name="baggage"]', 'content'); + expect(baggageContent).toContain('sentry-trace_id='); + + // The attribute that forces the chunk boundary must survive the rewrite. + expect(await page.getAttribute('html', 'data-long')).toHaveLength(3000); + }); + test('should have trace connection between server and client', async ({ page }) => { const serverTxPromise = waitForTransaction('tanstackstart-react', transactionEvent => { return transactionEvent?.contexts?.trace?.op === 'http.server' && transactionEvent?.transaction === 'GET /'; diff --git a/packages/astro/src/server/middleware.ts b/packages/astro/src/server/middleware.ts index 282da5f803ee..d70b4b352c8b 100644 --- a/packages/astro/src/server/middleware.ts +++ b/packages/astro/src/server/middleware.ts @@ -39,7 +39,7 @@ import { winterCGHeadersToDict, withIsolationScope, } from '@sentry/node'; -import { setHttpServerSpanRouteAttribute } from '@sentry/server-utils'; +import { injectHtmlIntoHead, setHttpServerSpanRouteAttribute } from '@sentry/server-utils'; import type { APIContext, MiddlewareHandler, MiddlewareNext, RoutePart } from 'astro'; type MiddlewareOptions = { @@ -279,26 +279,6 @@ async function instrumentRequestStartHttpServerSpan( }); } -/** - * This function optimistically assumes that the HTML coming in chunks will not be split - * within the tag. If this still happens, we simply won't replace anything. - */ -function addMetaTagToHead(htmlChunk: string, metaTagsStr: string): string { - if (typeof htmlChunk !== 'string' || !metaTagsStr) { - return htmlChunk; - } - - // Skip quoted attribute values so we don't match inside e.g. data-code="......" - let replaced = false; - return htmlChunk.replace(/"[^"]*"|'[^']*'|()/g, (match, headTag) => { - if (headTag && !replaced) { - replaced = true; - return `${metaTagsStr}`; - } - return match; - }); -} - function getMetaTagsStr({ injectTraceData, parametrizedRoute, @@ -463,63 +443,5 @@ function getParametrizedRoute(ctx: APIContext & { routePattern?: string }): stri } function injectMetaTagsInResponse(originalResponse: Response, metaTagsStr: string): Response { - try { - const contentType = originalResponse.headers.get('content-type'); - - const isPageloadRequest = contentType?.startsWith('text/html'); - if (!isPageloadRequest) { - return originalResponse; - } - - // Type case necessary b/c the body's ReadableStream type doesn't include - // the async iterator that is actually available in Node - // We later on use the async iterator to read the body chunks - // see https://github.com/microsoft/TypeScript/issues/39051 - const originalBody = originalResponse.body as NodeJS.ReadableStream | null; - if (!originalBody) { - return originalResponse; - } - - const decoder = new TextDecoder(); - - const newResponseStream = new ReadableStream({ - start: async controller => { - // Assign to a new variable to avoid TS losing the narrower type checked above. - const body = originalBody; - - async function* bodyReporter(): AsyncGenerator { - try { - for await (const chunk of body) { - yield chunk; - } - } catch (e) { - // Report stream errors coming from user code or Astro rendering. - sendErrorToSentry(e); - throw e; - } - } - - try { - for await (const chunk of bodyReporter()) { - const html = typeof chunk === 'string' ? chunk : decoder.decode(chunk, { stream: true }); - const modifiedHtml = addMetaTagToHead(html, metaTagsStr); - controller.enqueue(new TextEncoder().encode(modifiedHtml)); - } - } catch (e) { - controller.error(e); - } finally { - controller.close(); - } - }, - }); - - return new Response(newResponseStream, { - status: originalResponse.status, - statusText: originalResponse.statusText, - headers: new Headers(originalResponse.headers), - }); - } catch (e) { - sendErrorToSentry(e); - throw e; - } + return injectHtmlIntoHead(originalResponse, metaTagsStr, sendErrorToSentry); } diff --git a/packages/astro/test/server/middleware.test.ts b/packages/astro/test/server/middleware.test.ts index 9492491b8ddd..b25e04c1f464 100644 --- a/packages/astro/test/server/middleware.test.ts +++ b/packages/astro/test/server/middleware.test.ts @@ -389,9 +389,9 @@ describe('sentryMiddleware', () => { const html = await resultFromNext?.text(); expect(html).toContain(''); - expect(html).toContain(''); - // parametrized route is injected - expect(html).toContain(''); + expect(html).toContain(''); + // parametrized route is injected, directly before the closing head tag + expect(html).toContain(''); // trace data is not injected expect(html).not.toContain(''); + expect(html).toContain(''); expect(html).toContain(''); expect(html).toContain(' string; + /** Returns anything still held back once the body has ended. */ + flush: () => string; +}; + +/** + * Creates an injector that takes the HTML chunks of a single response, in order, and injects + * `html` directly before the closing head tag. + * + * The scan carries its state from one chunk to the next, so the closing tag may be split + * across any number of chunks. Anchoring on the closing tag means everything the head + * contains has already been seen by the time the injection happens, so a page that already + * carries trace meta tags is detected with certainty. The result depends only on the bytes of + * the response, never on where they happen to be split. + */ +function createHeadHtmlInjector(html: string): HeadHtmlInjector { + if (!html) { + return { transformChunk: htmlChunk => htmlChunk, flush: () => '' }; + } + + let done = false; + let carry = ''; + + return { + transformChunk(htmlChunk: string): string { + if (done) { + return htmlChunk; + } + + const chunk = carry + htmlChunk; + const closingIndex = chunk.indexOf(HEAD_CLOSING_TAG); + const existingIndex = chunk.indexOf(EXISTING_META_TAG); + + // The head already carries trace meta tags, e.g. rendered by the app itself. + if (existingIndex !== -1 && (closingIndex === -1 || existingIndex < closingIndex)) { + done = true; + carry = ''; + return chunk; + } + + if (closingIndex !== -1) { + done = true; + carry = ''; + return `${chunk.slice(0, closingIndex)}${html}${chunk.slice(closingIndex)}`; + } + + let keep = Math.min(CARRY_LENGTH, chunk.length); + // The two sides of the cut are encoded separately, and a lone surrogate encodes to + // U+FFFD, so keep a surrogate pair together. + const leadingCharCode = chunk.charCodeAt(chunk.length - keep - 1); + if (leadingCharCode >= 0xd800 && leadingCharCode <= 0xdbff) { + keep++; + } + + carry = chunk.slice(chunk.length - keep); + return chunk.slice(0, chunk.length - keep); + }, + flush(): string { + const buffered = carry; + carry = ''; + return buffered; + }, + }; +} + +/** + * Rewrites an HTML body stream so that `html` sits directly before the closing head tag. + * + * @param body - the HTML body stream to rewrite + * @param html - the markup to inject, e.g. the output of `getTraceMetaTags()` + * @param onError - called if reading the original body fails + */ +export function injectHtmlIntoHeadStream( + body: ReadableStream, + html: string, + onError?: (error: unknown) => void, +): ReadableStream { + const decoder = new TextDecoder(); + const encoder = new TextEncoder(); + const injector = createHeadHtmlInjector(html); + + // A TransformStream carries the consumer's backpressure through to the body it wraps. + // Pumping the body into a ReadableStream instead would read it as fast as it can be + // produced, which keeps our `desiredSize` positive and stops an upstream transform that + // throttles itself against it, such as a framework's own SSR stream, from ever pausing. + const { readable, writable } = new TransformStream({ + transform(chunk, controller) { + const htmlChunk = typeof chunk === 'string' ? chunk : decoder.decode(chunk, { stream: true }); + const modifiedHtml = injector.transformChunk(htmlChunk); + if (modifiedHtml) { + controller.enqueue(encoder.encode(modifiedHtml)); + } + }, + flush(controller) { + // Flush the decoder as well, so that a body ending on an incomplete byte sequence does + // not lose its tail. + const trailingHtml = injector.transformChunk(decoder.decode()) + injector.flush(); + if (trailingHtml) { + controller.enqueue(encoder.encode(trailingHtml)); + } + }, + }); + + const reader = body.getReader(); + const writer = writable.getWriter(); + + // Pumping by hand rather than with `pipeTo` keeps the two failure modes apart: only a body + // that fails to read is reported, while a consumer that goes away is not an error. + async function pump(): Promise { + for (;;) { + let result: ReadableStreamReadResult; + try { + result = await reader.read(); + } catch (error) { + onError?.(error); + await writer.abort(error); + return; + } + + if (result.done) { + await writer.close(); + return; + } + + // Resolves only once the readable side has room, which is what carries backpressure + // through to the body. + await writer.write(result.value); + } + } + + pump().catch((reason: unknown) => { + reader.cancel(reason).catch(() => undefined); + }); + + return readable; +} + +/** + * Returns a copy of `response` whose HTML body carries `html` directly before the closing head + * tag. Responses that are not HTML, responses without a body, and responses with nothing to + * inject are returned untouched. + * + * @param response - the response to rewrite + * @param html - the markup to inject, e.g. the output of `getTraceMetaTags()` + * @param onError - called if reading the original body fails + */ +export function injectHtmlIntoHead(response: Response, html: string, onError?: (error: unknown) => void): Response { + const contentType = response.headers.get('content-type'); + if (!html || !contentType?.startsWith('text/html') || !response.body) { + return response; + } + + const headers = new Headers(response.headers); + // The body grows by the injected markup, so a copied content-length would truncate it. + headers.delete('content-length'); + + return new Response(injectHtmlIntoHeadStream(response.body, html, onError), { + status: response.status, + statusText: response.statusText, + headers, + }); +} diff --git a/packages/server-utils/test/utils/htmlInjection.test.ts b/packages/server-utils/test/utils/htmlInjection.test.ts new file mode 100644 index 000000000000..172a8627e411 --- /dev/null +++ b/packages/server-utils/test/utils/htmlInjection.test.ts @@ -0,0 +1,214 @@ +import { describe, expect, it, vi } from 'vitest'; +import { injectHtmlIntoHead, injectHtmlIntoHeadStream } from '../../src/utils/htmlInjection'; + +const META_TAGS = + ''; + +function streamOf(chunks: (string | Uint8Array)[]): ReadableStream { + const encoder = new TextEncoder(); + return new ReadableStream({ + start(controller) { + for (const chunk of chunks) { + controller.enqueue(typeof chunk === 'string' ? encoder.encode(chunk) : chunk); + } + controller.close(); + }, + }); +} + +function inject(chunks: (string | Uint8Array)[], html: string = META_TAGS): Promise { + return new Response(injectHtmlIntoHeadStream(streamOf(chunks), html)).text(); +} + +function countTraceMetaTags(html: string): number { + return html.split('name="sentry-trace"').length - 1; +} + +describe('injectHtmlIntoHeadStream', () => { + it('injects directly before the closing head tag', async () => { + const html = await inject(['tb']); + + expect(countTraceMetaTags(html)).toBe(1); + // No whitespace text node may appear between the injected tags or before `` — + // React 19 whole-document hydration rejects unexpected text nodes in `` (#21915). + expect(html).toContain(`t${META_TAGS}`); + }); + + it('injects when the closing head tag is split across chunks', async () => { + const html = await inject(['tb']); + + expect(countTraceMetaTags(html)).toBe(1); + expect(html).toContain(`${META_TAGS}`); + }); + + it('produces the same output wherever the response is split', async () => { + const document = + 'App
hi
'; + + const unsplit = await inject([document]); + expect(countTraceMetaTags(unsplit)).toBe(1); + + for (let i = 1; i < document.length; i++) { + expect(await inject([document.slice(0, i), document.slice(i)])).toBe(unsplit); + } + + expect(await inject([...document])).toBe(unsplit); + }); + + it('does not inject when the head already carries trace meta tags', async () => { + const document = 'b'; + + expect(await inject([document])).toBe(document); + expect(await inject([...document])).toBe(document); + }); + + it('injects when sentry-trace appears in the body rather than the head', async () => { + const html = await inject(['t"sentry-trace"']); + + expect(countTraceMetaTags(html)).toBe(1); + expect(html).toContain(`${META_TAGS}`); + }); + + it('keeps multi-byte characters intact when they straddle a chunk boundary', async () => { + const document = 'Grüße 😀日本語'; + const bytes = new TextEncoder().encode(document); + + for (let i = 1; i < bytes.length; i++) { + const html = await inject([bytes.slice(0, i), bytes.slice(i)]); + expect(html.replace(META_TAGS, '')).toBe(document); + } + }); + + it('emits the body unchanged when it has no closing head tag', async () => { + const document = '

fragment

'; + + expect(await inject([document])).toBe(document); + expect(await inject([...document])).toBe(document); + }); + + it('emits the body unchanged when there is nothing to inject', async () => { + const document = 'tb'; + + expect(await inject([document], '')).toBe(document); + }); + + // A framework's own SSR stream can pause while its `desiredSize` is at or below zero. + // Draining it regardless lets its bounded internal buffers overflow. + it('does not read the body until the result is consumed', async () => { + const encoder = new TextEncoder(); + let pulled = 0; + + const body = new ReadableStream({ + pull(controller) { + pulled++; + controller.enqueue(encoder.encode(`

${pulled}

`)); + }, + }); + + injectHtmlIntoHeadStream(body, META_TAGS); + + for (let i = 0; i < 50; i++) { + await Promise.resolve(); + } + + expect(pulled).toBeLessThan(10); + }); + + it('reports a failing body to onError', async () => { + const bodyError = new Error('stream read error'); + const onError = vi.fn(); + + const body = new ReadableStream({ + start(controller) { + controller.error(bodyError); + }, + }); + + await expect(new Response(injectHtmlIntoHeadStream(body, META_TAGS, onError)).text()).rejects.toThrow(); + + expect(onError).toHaveBeenCalledWith(bodyError); + }); + + // A user navigating away cancels the response; that must not surface as an error, and it + // must stop the body being rendered. + it('cancels the body without reporting when the consumer goes away', async () => { + const encoder = new TextEncoder(); + const onError = vi.fn(); + const cancelled = vi.fn(); + + const body = new ReadableStream({ + pull(controller) { + controller.enqueue(encoder.encode('

chunk

')); + }, + cancel: cancelled, + }); + + const reader = injectHtmlIntoHeadStream(body, META_TAGS, onError).getReader(); + await reader.read(); + await reader.cancel('navigated away'); + + // Cancellation reaches the body asynchronously; a later read on a fresh stream is not + // available here, so poll the sentinel spy directly. + await vi.waitFor(() => expect(cancelled).toHaveBeenCalledWith('navigated away')); + expect(onError).not.toHaveBeenCalled(); + }); +}); + +describe('injectHtmlIntoHead', () => { + it('injects into HTML responses and preserves status and headers', async () => { + const response = new Response('b', { + status: 201, + statusText: 'Created', + headers: new Headers({ 'content-type': 'text/html; charset=utf-8', 'X-Custom-Header': 'custom-value' }), + }); + + const injected = injectHtmlIntoHead(response, META_TAGS); + const html = await injected.text(); + + expect(html).toContain(`${META_TAGS}`); + expect(injected.status).toBe(201); + expect(injected.statusText).toBe('Created'); + expect(injected.headers.get('content-type')).toBe('text/html; charset=utf-8'); + expect(injected.headers.get('X-Custom-Header')).toBe('custom-value'); + }); + + it('drops a stale content-length, since the body grows', async () => { + const html = 'b'; + const response = new Response(html, { + headers: new Headers({ 'content-type': 'text/html', 'content-length': String(html.length) }), + }); + + const injected = injectHtmlIntoHead(response, META_TAGS); + + expect(injected.headers.get('content-length')).toBeNull(); + expect((await injected.text()).length).toBe(html.length + META_TAGS.length); + }); + + it('returns non-HTML responses untouched', async () => { + const response = new Response('{"data":"value"}', { + headers: new Headers({ 'content-type': 'application/json' }), + }); + + const injected = injectHtmlIntoHead(response, META_TAGS); + + expect(injected).toBe(response); + expect(await injected.text()).toBe('{"data":"value"}'); + }); + + it('returns responses untouched when there is nothing to inject', () => { + const response = new Response('', { + headers: new Headers({ 'content-type': 'text/html' }), + }); + + expect(injectHtmlIntoHead(response, '')).toBe(response); + }); + + it('returns responses without a body untouched', () => { + const response = new Response(null, { + status: 204, + headers: new Headers({ 'content-type': 'text/html' }), + }); + + expect(injectHtmlIntoHead(response, META_TAGS)).toBe(response); + }); +}); diff --git a/packages/solidstart/src/server/middleware.ts b/packages/solidstart/src/server/middleware.ts index 6419171cc1b8..faf4152107e2 100644 --- a/packages/solidstart/src/server/middleware.ts +++ b/packages/solidstart/src/server/middleware.ts @@ -1,4 +1,5 @@ import { addNonEnumerableProperty, getTraceMetaTags } from '@sentry/core'; +import { injectHtmlIntoHeadStream } from '@sentry/server-utils'; import type { ResponseMiddleware } from '@solidjs/start/middleware'; import type { FetchEvent } from '@solidjs/start/server'; @@ -6,17 +7,6 @@ export type ResponseMiddlewareResponse = Parameters[1] & { __sentry_wrapped__?: boolean; }; -function addMetaTagToHead(html: string): string { - const metaTags = getTraceMetaTags(); - - if (!metaTags) { - return html; - } - - const content = `\n${metaTags}\n`; - return html.replace('', content); -} - /** * Returns an `onBeforeResponse` solid start middleware handler that adds tracing data as * tags to a page on pageload to enable distributed tracing. @@ -38,17 +28,6 @@ export function sentryBeforeResponseMiddleware() { return; } - const body = response.body as NodeJS.ReadableStream; - const decoder = new TextDecoder(); - response.body = new ReadableStream({ - start: async controller => { - for await (const chunk of body) { - const html = typeof chunk === 'string' ? chunk : decoder.decode(chunk, { stream: true }); - const modifiedHtml = addMetaTagToHead(html); - controller.enqueue(new TextEncoder().encode(modifiedHtml)); - } - controller.close(); - }, - }); + response.body = injectHtmlIntoHeadStream(response.body as ReadableStream, getTraceMetaTags()); }; } diff --git a/packages/tanstackstart-react/src/server/wrapFetchWithSentry.ts b/packages/tanstackstart-react/src/server/wrapFetchWithSentry.ts index beddd0c20d48..62be7ae6dd8a 100644 --- a/packages/tanstackstart-react/src/server/wrapFetchWithSentry.ts +++ b/packages/tanstackstart-react/src/server/wrapFetchWithSentry.ts @@ -2,6 +2,7 @@ import { flushIfServerless, getTraceMetaTags } from '@sentry/core'; import { captureException, SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN, startSpan } from '@sentry/node'; import { SENTRY_OP } from '@sentry/conventions/attributes'; import { FUNCTION } from '@sentry/conventions/op'; +import { injectHtmlIntoHead } from '@sentry/server-utils'; import { updateSpanWithRouteParametrization } from './routeParametrization'; declare const __SENTRY_ROUTE_PATTERNS__: string[] | undefined; @@ -14,100 +15,12 @@ export type ServerEntry = { fetch: (request: Request, opts?: any) => Promise | Response; }; -/** - * This function optimistically assumes that the HTML coming in chunks will not be split - * within the tag. If this still happens, we simply won't replace anything. - */ -function addMetaTagToHead(htmlChunk: string, metaTagsStr: string): string { - if (typeof htmlChunk !== 'string' || !metaTagsStr) { - return htmlChunk; - } - - if (htmlChunk.includes('"sentry-trace"')) { - return htmlChunk; - } - - // Skip quoted attribute values so we don't match inside e.g. data-code="......" - let replaced = false; - return htmlChunk.replace(/"[^"]*"|'[^']*'|()/g, (match, headTag) => { - if (headTag && !replaced) { - replaced = true; - return `${metaTagsStr}`; - } - return match; +function reportStreamError(error: unknown): void { + captureException(error, { + mechanism: { type: 'auto.http.tanstackstart', handled: false }, }); } -function injectMetaTagsInResponse(originalResponse: Response): Response { - try { - const contentType = originalResponse.headers.get('content-type'); - - const isPageloadRequest = contentType?.startsWith('text/html'); - if (!isPageloadRequest) { - return originalResponse; - } - - // Type case necessary b/c the body's ReadableStream type doesn't include - // the async iterator that is actually available in Node - // We later on use the async iterator to read the body chunks - // see https://github.com/microsoft/TypeScript/issues/39051 - const originalBody = originalResponse.body as NodeJS.ReadableStream | null; - if (!originalBody) { - return originalResponse; - } - - const metaTagsStr = getTraceMetaTags(); - const decoder = new TextDecoder(); - - const newResponseStream = new ReadableStream({ - start: async controller => { - // Assign to a new variable to avoid TS losing the narrower type checked above. - const body = originalBody; - - async function* bodyReporter(): AsyncGenerator { - try { - for await (const chunk of body) { - yield chunk; - } - } catch (e) { - captureException(e, { - mechanism: { type: 'auto.http.tanstackstart', handled: false }, - }); - throw e; - } - } - - let errored = false; - try { - for await (const chunk of bodyReporter()) { - const html = typeof chunk === 'string' ? chunk : decoder.decode(chunk, { stream: true }); - const modifiedHtml = addMetaTagToHead(html, metaTagsStr); - controller.enqueue(new TextEncoder().encode(modifiedHtml)); - } - } catch (e) { - errored = true; - controller.error(e); - } finally { - if (!errored) { - controller.close(); - } - } - }, - }); - - return new Response(newResponseStream, { - status: originalResponse.status, - statusText: originalResponse.statusText, - headers: new Headers(originalResponse.headers), - }); - } catch (e) { - captureException(e, { - mechanism: { type: 'auto.http.tanstackstart', handled: false }, - }); - throw e; - } -} - /** * This function can be used to wrap the server entry request handler to add tracing to server-side functionality. * You must explicitly define a server entry point in your application for this to work. This is done by passing the request handler to the `createServerEntry` function. @@ -161,7 +74,7 @@ export function wrapFetchWithSentry(serverEntry: ServerEntry): ServerEntry { updateSpanWithRouteParametrization(method, url.pathname, __SENTRY_ROUTE_PATTERNS__); } - return injectMetaTagsInResponse(await target.apply(thisArg, args)); + return injectHtmlIntoHead(await target.apply(thisArg, args), getTraceMetaTags(), reportStreamError); } finally { await flushIfServerless(); } diff --git a/packages/tanstackstart-react/test/server/wrapFetchWithSentry.test.ts b/packages/tanstackstart-react/test/server/wrapFetchWithSentry.test.ts index 5928cefeb9b7..a08427e2865a 100644 --- a/packages/tanstackstart-react/test/server/wrapFetchWithSentry.test.ts +++ b/packages/tanstackstart-react/test/server/wrapFetchWithSentry.test.ts @@ -87,10 +87,10 @@ describe('wrapFetchWithSentry', () => { expect(html).toContain(''); expect(html).toContain(''); - // No whitespace text node may appear directly after `` or between the injected tags — + // No whitespace text node may appear between the injected tags or before `` — // React 19 whole-document hydration rejects unexpected text nodes in `` (#21915). expect(html).toContain( - '', + '', ); }); @@ -149,7 +149,7 @@ describe('wrapFetchWithSentry', () => { expect(response.headers.get('X-Custom-Header')).toBe('custom-value'); }); - it('does not inject meta tags into inside quoted attribute values', async () => { + it('leaves head tags inside quoted attribute values alone', async () => { const mockResponse = new Response('
', { headers: new Headers({ 'content-type': 'text/html' }), }); @@ -161,7 +161,7 @@ describe('wrapFetchWithSentry', () => { const response = await serverEntry.fetch(request); const html = await response.text(); - expect(html).toContain(''); expect(html).toContain('data-content="ignore"'); }); @@ -193,6 +193,33 @@ describe('wrapFetchWithSentry', () => { }); }); + // The chunk boundary handling itself is covered in @sentry/server-utils; this checks that a + // streamed response reaches it at all. + it('injects meta tags into an HTML response arriving in several chunks', async () => { + const encoder = new TextEncoder(); + const body = new ReadableStream({ + start(controller) { + controller.enqueue(encoder.encode('tb')); + controller.close(); + }, + }); + const fetchFn = vi.fn().mockResolvedValue( + new Response(body, { + headers: new Headers({ 'content-type': 'text/html' }), + }), + ); + + const serverEntry = wrapFetchWithSentry({ fetch: fetchFn }); + const response = await serverEntry.fetch(new Request('http://localhost:3000/')); + const html = await response.text(); + + expect(html.split('name="sentry-trace"')).toHaveLength(2); + expect(html).toContain(''); + expect(html).toContain('data-ssr-state="{"theme":"dark"}"'); + }); + it('calls flushIfServerless even if the handler throws', async () => { const fetchFn = vi.fn().mockRejectedValue(new Error('handler error'));