From e4f459796d9ed8e1d3219feff1dd137548fa26e3 Mon Sep 17 00:00:00 2001 From: Charly Gomez Date: Tue, 22 Apr 2025 09:48:21 +0200 Subject: [PATCH 1/4] feat(react-router): Trace propagation (#16070) - Adds new util function `getMetaTagTransformer` for injecting trace meta tags to the html `` - Adds helper function `createSentryHandleRequest` which is a complete Sentry-instrumented handleRequest implementation that handles both route parametrization and trace meta tag injection. -> this is for users that do not need any modifications in their `handleRequest` function - Renames `sentryHandleRequest` to `wrapSentryHandleRequest` to avoid confusion closes https://github.com/getsentry/sentry-javascript/issues/15515 --- .../app/entry.server.tsx | 67 +--- .../performance/trace-propagation.test.ts | 36 ++ .../src/server/createSentryHandleRequest.tsx | 138 +++++++ packages/react-router/src/server/index.ts | 4 +- ...eRequest.ts => wrapSentryHandleRequest.ts} | 33 +- .../server/createSentryHandleRequest.test.ts | 340 ++++++++++++++++++ .../server/wrapSentryHandleRequest.test.ts | 183 ++++++++++ packages/react-router/tsconfig.json | 3 +- 8 files changed, 742 insertions(+), 62 deletions(-) create mode 100644 dev-packages/e2e-tests/test-applications/react-router-7-framework/tests/performance/trace-propagation.test.ts create mode 100644 packages/react-router/src/server/createSentryHandleRequest.tsx rename packages/react-router/src/server/{sentryHandleRequest.ts => wrapSentryHandleRequest.ts} (61%) create mode 100644 packages/react-router/test/server/createSentryHandleRequest.test.ts create mode 100644 packages/react-router/test/server/wrapSentryHandleRequest.test.ts diff --git a/dev-packages/e2e-tests/test-applications/react-router-7-framework/app/entry.server.tsx b/dev-packages/e2e-tests/test-applications/react-router-7-framework/app/entry.server.tsx index 567edfe4e032..97260755da21 100644 --- a/dev-packages/e2e-tests/test-applications/react-router-7-framework/app/entry.server.tsx +++ b/dev-packages/e2e-tests/test-applications/react-router-7-framework/app/entry.server.tsx @@ -1,68 +1,19 @@ -import { PassThrough } from 'node:stream'; - import { createReadableStreamFromReadable } from '@react-router/node'; import * as Sentry from '@sentry/react-router'; -import { isbot } from 'isbot'; -import type { RenderToPipeableStreamOptions } from 'react-dom/server'; import { renderToPipeableStream } from 'react-dom/server'; -import type { AppLoadContext, EntryContext } from 'react-router'; import { ServerRouter } from 'react-router'; -const ABORT_DELAY = 5_000; - -function handleRequest( - request: Request, - responseStatusCode: number, - responseHeaders: Headers, - routerContext: EntryContext, - loadContext: AppLoadContext, -) { - return new Promise((resolve, reject) => { - let shellRendered = false; - let userAgent = request.headers.get('user-agent'); - - // Ensure requests from bots and SPA Mode renders wait for all content to load before responding - // https://react.dev/reference/react-dom/server/renderToPipeableStream#waiting-for-all-content-to-load-for-crawlers-and-static-generation - let readyOption: keyof RenderToPipeableStreamOptions = - (userAgent && isbot(userAgent)) || routerContext.isSpaMode ? 'onAllReady' : 'onShellReady'; - - const { pipe, abort } = renderToPipeableStream(, { - [readyOption]() { - shellRendered = true; - const body = new PassThrough(); - const stream = createReadableStreamFromReadable(body); - - responseHeaders.set('Content-Type', 'text/html'); - - resolve( - new Response(stream, { - headers: responseHeaders, - status: responseStatusCode, - }), - ); - - pipe(body); - }, - onShellError(error: unknown) { - reject(error); - }, - onError(error: unknown) { - responseStatusCode = 500; - // Log streaming rendering errors from inside the shell. Don't log - // errors encountered during initial shell rendering since they'll - // reject and get logged in handleDocumentRequest. - if (shellRendered) { - console.error(error); - } - }, - }); +import { type HandleErrorFunction } from 'react-router'; - setTimeout(abort, ABORT_DELAY); - }); -} +const ABORT_DELAY = 5_000; -export default Sentry.sentryHandleRequest(handleRequest); +const handleRequest = Sentry.createSentryHandleRequest({ + streamTimeout: ABORT_DELAY, + ServerRouter, + renderToPipeableStream, + createReadableStreamFromReadable, +}); -import { type HandleErrorFunction } from 'react-router'; +export default handleRequest; export const handleError: HandleErrorFunction = (error, { request }) => { // React Router may abort some interrupted requests, don't log those diff --git a/dev-packages/e2e-tests/test-applications/react-router-7-framework/tests/performance/trace-propagation.test.ts b/dev-packages/e2e-tests/test-applications/react-router-7-framework/tests/performance/trace-propagation.test.ts new file mode 100644 index 000000000000..6a9623171236 --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/react-router-7-framework/tests/performance/trace-propagation.test.ts @@ -0,0 +1,36 @@ +import { expect, test } from '@playwright/test'; +import { waitForTransaction } from '@sentry-internal/test-utils'; +import { APP_NAME } from '../constants'; + +test.describe('Trace propagation', () => { + test('should inject metatags in ssr pageload', async ({ page }) => { + await page.goto(`/`); + const sentryTraceContent = await page.getAttribute('meta[name="sentry-trace"]', 'content'); + expect(sentryTraceContent).toBeDefined(); + expect(sentryTraceContent).toMatch(/^[a-f0-9]{32}-[a-f0-9]{16}-[01]$/); + const baggageContent = await page.getAttribute('meta[name="baggage"]', 'content'); + expect(baggageContent).toBeDefined(); + expect(baggageContent).toContain('sentry-environment=qa'); + expect(baggageContent).toContain('sentry-public_key='); + expect(baggageContent).toContain('sentry-trace_id='); + expect(baggageContent).toContain('sentry-transaction='); + expect(baggageContent).toContain('sentry-sampled='); + }); + + test('should have trace connection', async ({ page }) => { + const serverTxPromise = waitForTransaction(APP_NAME, async transactionEvent => { + return transactionEvent.transaction === 'GET *'; + }); + + const clientTxPromise = waitForTransaction(APP_NAME, async transactionEvent => { + return transactionEvent.transaction === '/'; + }); + + await page.goto(`/`); + const serverTx = await serverTxPromise; + const clientTx = await clientTxPromise; + + expect(clientTx.contexts?.trace?.trace_id).toEqual(serverTx.contexts?.trace?.trace_id); + expect(clientTx.contexts?.trace?.parent_span_id).toBe(serverTx.contexts?.trace?.span_id); + }); +}); diff --git a/packages/react-router/src/server/createSentryHandleRequest.tsx b/packages/react-router/src/server/createSentryHandleRequest.tsx new file mode 100644 index 000000000000..662d0b14a93a --- /dev/null +++ b/packages/react-router/src/server/createSentryHandleRequest.tsx @@ -0,0 +1,138 @@ +import React from 'react'; +import type { AppLoadContext, EntryContext, ServerRouter } from 'react-router'; +import type { ReactNode } from 'react'; +import { getMetaTagTransformer, wrapSentryHandleRequest } from './wrapSentryHandleRequest'; +import type { createReadableStreamFromReadable } from '@react-router/node'; +import { PassThrough } from 'stream'; + +type RenderToPipeableStreamOptions = { + [key: string]: unknown; + onShellReady?: () => void; + onAllReady?: () => void; + onShellError?: (error: unknown) => void; + onError?: (error: unknown) => void; +}; + +type RenderToPipeableStreamResult = { + pipe: (destination: NodeJS.WritableStream) => void; + abort: () => void; +}; + +type RenderToPipeableStreamFunction = ( + node: ReactNode, + options: RenderToPipeableStreamOptions, +) => RenderToPipeableStreamResult; + +export interface SentryHandleRequestOptions { + /** + * Timeout in milliseconds after which the rendering stream will be aborted + * @default 10000 + */ + streamTimeout?: number; + + /** + * React's renderToPipeableStream function from 'react-dom/server' + */ + renderToPipeableStream: RenderToPipeableStreamFunction; + + /** + * The component from '@react-router/server' + */ + ServerRouter: typeof ServerRouter; + + /** + * createReadableStreamFromReadable from '@react-router/node' + */ + createReadableStreamFromReadable: typeof createReadableStreamFromReadable; + + /** + * Regular expression to identify bot user agents + * @default /bot|crawler|spider|googlebot|chrome-lighthouse|baidu|bing|google|yahoo|lighthouse/i + */ + botRegex?: RegExp; +} + +/** + * A complete Sentry-instrumented handleRequest implementation that handles both + * route parametrization and trace meta tag injection. + * + * @param options Configuration options + * @returns A Sentry-instrumented handleRequest function + */ +export function createSentryHandleRequest( + options: SentryHandleRequestOptions, +): ( + request: Request, + responseStatusCode: number, + responseHeaders: Headers, + routerContext: EntryContext, + loadContext: AppLoadContext, +) => Promise { + const { + streamTimeout = 10000, + renderToPipeableStream, + ServerRouter, + createReadableStreamFromReadable, + botRegex = /bot|crawler|spider|googlebot|chrome-lighthouse|baidu|bing|google|yahoo|lighthouse/i, + } = options; + + const handleRequest = function handleRequest( + request: Request, + responseStatusCode: number, + responseHeaders: Headers, + routerContext: EntryContext, + _loadContext: AppLoadContext, + ): Promise { + return new Promise((resolve, reject) => { + let shellRendered = false; + const userAgent = request.headers.get('user-agent'); + + // Determine if we should use onAllReady or onShellReady + const isBot = typeof userAgent === 'string' && botRegex.test(userAgent); + const isSpaMode = !!(routerContext as { isSpaMode?: boolean }).isSpaMode; + + const readyOption = isBot || isSpaMode ? 'onAllReady' : 'onShellReady'; + + const { pipe, abort } = renderToPipeableStream(, { + [readyOption]() { + shellRendered = true; + const body = new PassThrough(); + + const stream = createReadableStreamFromReadable(body); + + responseHeaders.set('Content-Type', 'text/html'); + + resolve( + new Response(stream, { + headers: responseHeaders, + status: responseStatusCode, + }), + ); + + // this injects trace data to the HTML head + pipe(getMetaTagTransformer(body)); + }, + onShellError(error: unknown) { + reject(error); + }, + onError(error: unknown) { + // eslint-disable-next-line no-param-reassign + responseStatusCode = 500; + // Log streaming rendering errors from inside the shell. Don't log + // errors encountered during initial shell rendering since they'll + // reject and get logged in handleDocumentRequest. + if (shellRendered) { + // eslint-disable-next-line no-console + console.error(error); + } + }, + }); + + // Abort the rendering stream after the `streamTimeout` + setTimeout(abort, streamTimeout); + }); + }; + + // Wrap the handle request function for request parametrization + return wrapSentryHandleRequest(handleRequest); +} diff --git a/packages/react-router/src/server/index.ts b/packages/react-router/src/server/index.ts index 44acfec7d4f2..67436582aedd 100644 --- a/packages/react-router/src/server/index.ts +++ b/packages/react-router/src/server/index.ts @@ -1,4 +1,6 @@ export * from '@sentry/node'; export { init } from './sdk'; -export { sentryHandleRequest } from './sentryHandleRequest'; +// eslint-disable-next-line deprecation/deprecation +export { wrapSentryHandleRequest, sentryHandleRequest, getMetaTagTransformer } from './wrapSentryHandleRequest'; +export { createSentryHandleRequest, type SentryHandleRequestOptions } from './createSentryHandleRequest'; diff --git a/packages/react-router/src/server/sentryHandleRequest.ts b/packages/react-router/src/server/wrapSentryHandleRequest.ts similarity index 61% rename from packages/react-router/src/server/sentryHandleRequest.ts rename to packages/react-router/src/server/wrapSentryHandleRequest.ts index 9c5f4abf72e8..bc6cc93122bb 100644 --- a/packages/react-router/src/server/sentryHandleRequest.ts +++ b/packages/react-router/src/server/wrapSentryHandleRequest.ts @@ -1,8 +1,10 @@ import { context } from '@opentelemetry/api'; import { RPCType, getRPCMetadata } from '@opentelemetry/core'; import { ATTR_HTTP_ROUTE } from '@opentelemetry/semantic-conventions'; -import { SEMANTIC_ATTRIBUTE_SENTRY_SOURCE, getActiveSpan, getRootSpan } from '@sentry/core'; +import { SEMANTIC_ATTRIBUTE_SENTRY_SOURCE, getActiveSpan, getRootSpan, getTraceMetaTags } from '@sentry/core'; import type { AppLoadContext, EntryContext } from 'react-router'; +import type { PassThrough } from 'stream'; +import { Transform } from 'stream'; type OriginalHandleRequest = ( request: Request, @@ -18,7 +20,7 @@ type OriginalHandleRequest = ( * @param originalHandle - The original handleRequest function to wrap * @returns A wrapped version of the handle request function with Sentry instrumentation */ -export function sentryHandleRequest(originalHandle: OriginalHandleRequest): OriginalHandleRequest { +export function wrapSentryHandleRequest(originalHandle: OriginalHandleRequest): OriginalHandleRequest { return async function sentryInstrumentedHandleRequest( request: Request, responseStatusCode: number, @@ -47,6 +49,33 @@ export function sentryHandleRequest(originalHandle: OriginalHandleRequest): Orig }); } } + return originalHandle(request, responseStatusCode, responseHeaders, routerContext, loadContext); }; } + +/** @deprecated Use `wrapSentryHandleRequest` instead. */ +export const sentryHandleRequest = wrapSentryHandleRequest; + +/** + * Injects Sentry trace meta tags into the HTML response by piping through a transform stream. + * This enables distributed tracing by adding trace context to the HTML document head. + * + * @param body - PassThrough stream containing the HTML response body to modify + */ +export function getMetaTagTransformer(body: PassThrough): Transform { + const headClosingTag = ''; + const htmlMetaTagTransformer = new Transform({ + transform(chunk, _encoding, callback) { + const html = Buffer.isBuffer(chunk) ? chunk.toString() : String(chunk); + if (html.includes(headClosingTag)) { + const modifiedHtml = html.replace(headClosingTag, `${getTraceMetaTags()}${headClosingTag}`); + callback(null, modifiedHtml); + return; + } + callback(null, chunk); + }, + }); + htmlMetaTagTransformer.pipe(body); + return htmlMetaTagTransformer; +} diff --git a/packages/react-router/test/server/createSentryHandleRequest.test.ts b/packages/react-router/test/server/createSentryHandleRequest.test.ts new file mode 100644 index 000000000000..0db84d19ce16 --- /dev/null +++ b/packages/react-router/test/server/createSentryHandleRequest.test.ts @@ -0,0 +1,340 @@ +/* eslint-disable no-console */ +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; +import { PassThrough } from 'stream'; +import * as wrapSentryHandleRequestModule from '../../src/server/wrapSentryHandleRequest'; +import { createSentryHandleRequest } from '../../src/server/createSentryHandleRequest'; +import type { EntryContext } from 'react-router'; + +vi.mock('../../src/server/wrapSentryHandleRequest', () => ({ + wrapSentryHandleRequest: vi.fn(fn => fn), + getMetaTagTransformer: vi.fn(body => { + const transform = new PassThrough(); + transform.pipe(body); + return transform; + }), +})); + +describe('createSentryHandleRequest', () => { + const mockRenderToPipeableStream = vi.fn(); + const mockServerRouter = vi.fn(); + const mockCreateReadableStreamFromReadable = vi.fn(); + + const mockRequest = { + url: 'https://sentry-example.com/test', + headers: { + get: vi.fn(), + }, + } as unknown as Request; + + let mockResponseHeaders: Headers; + + const mockRouterContext: EntryContext = { + manifest: { + entry: { + imports: [], + module: 'test-module', + }, + routes: {}, + url: '/test', + version: '1.0.0', + }, + routeModules: {}, + future: {}, + isSpaMode: false, + staticHandlerContext: { + matches: [ + { + route: { + path: 'test', + id: 'test-route', + }, + params: {}, + pathname: '/test', + pathnameBase: '/test', + }, + ], + loaderData: {}, + actionData: null, + errors: null, + basename: '/', + location: { + pathname: '/test', + search: '', + hash: '', + state: null, + key: 'default', + }, + statusCode: 200, + loaderHeaders: {}, + actionHeaders: {}, + }, + }; + + const mockLoadContext = {}; + + const mockPipe = vi.fn(); + const mockAbort = vi.fn(); + + beforeEach(() => { + vi.clearAllMocks(); + + mockResponseHeaders = new Headers(); + vi.spyOn(mockResponseHeaders, 'set'); + + mockRenderToPipeableStream.mockReturnValue({ + pipe: mockPipe, + abort: mockAbort, + }); + + mockCreateReadableStreamFromReadable.mockImplementation(body => body); + + vi.useFakeTimers(); + }); + + afterEach(() => { + vi.useRealTimers(); + }); + + it('should create a handleRequest function', () => { + const handleRequest = createSentryHandleRequest({ + renderToPipeableStream: mockRenderToPipeableStream, + ServerRouter: mockServerRouter, + createReadableStreamFromReadable: mockCreateReadableStreamFromReadable, + }); + + expect(handleRequest).toBeDefined(); + expect(typeof handleRequest).toBe('function'); + expect(wrapSentryHandleRequestModule.wrapSentryHandleRequest).toHaveBeenCalled(); + }); + + it('should use the default stream timeout if not provided', () => { + const handleRequest = createSentryHandleRequest({ + renderToPipeableStream: mockRenderToPipeableStream, + ServerRouter: mockServerRouter, + createReadableStreamFromReadable: mockCreateReadableStreamFromReadable, + }); + + handleRequest(mockRequest, 200, mockResponseHeaders, mockRouterContext, mockLoadContext); + + vi.advanceTimersByTime(10000); + expect(mockAbort).toHaveBeenCalled(); + }); + + it('should use a custom stream timeout if provided', () => { + const customTimeout = 5000; + + const handleRequest = createSentryHandleRequest({ + streamTimeout: customTimeout, + renderToPipeableStream: mockRenderToPipeableStream, + ServerRouter: mockServerRouter, + createReadableStreamFromReadable: mockCreateReadableStreamFromReadable, + }); + + handleRequest(mockRequest, 200, mockResponseHeaders, mockRouterContext, mockLoadContext); + + vi.advanceTimersByTime(customTimeout - 1); + expect(mockAbort).not.toHaveBeenCalled(); + + vi.advanceTimersByTime(1); + expect(mockAbort).toHaveBeenCalled(); + }); + + it('should use the default bot regex if not provided', () => { + const handleRequest = createSentryHandleRequest({ + renderToPipeableStream: mockRenderToPipeableStream, + ServerRouter: mockServerRouter, + createReadableStreamFromReadable: mockCreateReadableStreamFromReadable, + }); + + (mockRequest.headers.get as ReturnType).mockReturnValue('Googlebot/2.1'); + handleRequest(mockRequest, 200, mockResponseHeaders, mockRouterContext, mockLoadContext); + + expect(mockRenderToPipeableStream).toHaveBeenCalledWith( + expect.anything(), + expect.objectContaining({ + onAllReady: expect.any(Function), + }), + ); + }); + + it('should use a custom bot regex if provided', () => { + const handleRequest = createSentryHandleRequest({ + renderToPipeableStream: mockRenderToPipeableStream, + ServerRouter: mockServerRouter, + createReadableStreamFromReadable: mockCreateReadableStreamFromReadable, + botRegex: /custom-bot/i, + }); + + (mockRequest.headers.get as ReturnType).mockReturnValue('Googlebot/2.1'); + handleRequest(mockRequest, 200, mockResponseHeaders, mockRouterContext, mockLoadContext); + + expect(mockRenderToPipeableStream).toHaveBeenCalledWith( + expect.anything(), + expect.objectContaining({ + onShellReady: expect.any(Function), + }), + ); + + vi.clearAllMocks(); + (mockRequest.headers.get as ReturnType).mockReturnValue('custom-bot/1.0'); + handleRequest(mockRequest, 200, mockResponseHeaders, mockRouterContext, mockLoadContext); + + expect(mockRenderToPipeableStream).toHaveBeenCalledWith( + expect.anything(), + expect.objectContaining({ + onAllReady: expect.any(Function), + }), + ); + }); + + it('should use onAllReady for SPA mode', () => { + const handleRequest = createSentryHandleRequest({ + renderToPipeableStream: mockRenderToPipeableStream, + ServerRouter: mockServerRouter, + createReadableStreamFromReadable: mockCreateReadableStreamFromReadable, + }); + + (mockRequest.headers.get as ReturnType).mockReturnValue('Mozilla/5.0'); + const spaRouterContext = { ...mockRouterContext, isSpaMode: true }; + + handleRequest(mockRequest, 200, mockResponseHeaders, spaRouterContext, mockLoadContext); + + expect(mockRenderToPipeableStream).toHaveBeenCalledWith( + expect.anything(), + expect.objectContaining({ + onAllReady: expect.any(Function), + }), + ); + }); + + it('should use onShellReady for regular browsers', () => { + const handleRequest = createSentryHandleRequest({ + renderToPipeableStream: mockRenderToPipeableStream, + ServerRouter: mockServerRouter, + createReadableStreamFromReadable: mockCreateReadableStreamFromReadable, + }); + + (mockRequest.headers.get as ReturnType).mockReturnValue('Mozilla/5.0'); + + handleRequest(mockRequest, 200, mockResponseHeaders, mockRouterContext, mockLoadContext); + + expect(mockRenderToPipeableStream).toHaveBeenCalledWith( + expect.anything(), + expect.objectContaining({ + onShellReady: expect.any(Function), + }), + ); + }); + + it('should set Content-Type header when shell is ready', async () => { + const handleRequest = createSentryHandleRequest({ + renderToPipeableStream: mockRenderToPipeableStream, + ServerRouter: mockServerRouter, + createReadableStreamFromReadable: mockCreateReadableStreamFromReadable, + }); + + mockRenderToPipeableStream.mockImplementation((jsx, options) => { + if (options.onShellReady) { + options.onShellReady(); + } + return { pipe: mockPipe, abort: mockAbort }; + }); + + await handleRequest(mockRequest, 200, mockResponseHeaders, mockRouterContext, mockLoadContext); + + expect(mockResponseHeaders.set).toHaveBeenCalledWith('Content-Type', 'text/html'); + }); + + it('should pipe to the meta tag transformer', async () => { + const getMetaTagTransformerSpy = vi.spyOn(wrapSentryHandleRequestModule, 'getMetaTagTransformer'); + + const pipeSpy = vi.fn(); + + mockRenderToPipeableStream.mockImplementation((jsx, options) => { + // Call the ready callback synchronously to trigger the code path we want to test + setTimeout(() => { + if (options.onShellReady) { + options.onShellReady(); + } + }, 0); + + return { + pipe: pipeSpy, + abort: mockAbort, + }; + }); + + const handleRequest = createSentryHandleRequest({ + renderToPipeableStream: mockRenderToPipeableStream, + ServerRouter: mockServerRouter, + createReadableStreamFromReadable: mockCreateReadableStreamFromReadable, + }); + + const promise = handleRequest(mockRequest, 200, mockResponseHeaders, mockRouterContext, mockLoadContext); + + // Advance timers to trigger the setTimeout in our mock + await vi.runAllTimersAsync(); + await promise; + + expect(getMetaTagTransformerSpy).toHaveBeenCalled(); + expect(getMetaTagTransformerSpy.mock.calls[0]?.[0]).toBeInstanceOf(PassThrough); + expect(pipeSpy).toHaveBeenCalled(); + }); + + it('should set status code to 500 on error after shell is rendered', async () => { + const handleRequest = createSentryHandleRequest({ + renderToPipeableStream: mockRenderToPipeableStream, + ServerRouter: mockServerRouter, + createReadableStreamFromReadable: mockCreateReadableStreamFromReadable, + }); + + const originalConsoleError = console.error; + console.error = vi.fn(); + + let shellReadyCallback: (() => void) | undefined; + let errorCallback: ((error: Error) => void) | undefined; + + mockRenderToPipeableStream.mockImplementation((jsx, options) => { + shellReadyCallback = options.onShellReady; + errorCallback = options.onError; + return { pipe: mockPipe, abort: mockAbort }; + }); + + const responsePromise = handleRequest(mockRequest, 200, mockResponseHeaders, mockRouterContext, mockLoadContext); + + // First trigger shellReady to set shellRendered = true + // Then trigger onError to cause the error handling + if (shellReadyCallback) { + shellReadyCallback(); + } + + if (errorCallback) { + errorCallback(new Error('Test error')); + } + + await responsePromise; + expect(console.error).toHaveBeenCalled(); + console.error = originalConsoleError; + }); + + it('should reject the promise on shell error', async () => { + const handleRequest = createSentryHandleRequest({ + renderToPipeableStream: mockRenderToPipeableStream, + ServerRouter: mockServerRouter, + createReadableStreamFromReadable: mockCreateReadableStreamFromReadable, + }); + + const testError = new Error('Shell error'); + + mockRenderToPipeableStream.mockImplementation((jsx, options) => { + if (options.onShellError) { + options.onShellError(testError); + } + return { pipe: mockPipe, abort: mockAbort }; + }); + + await expect( + handleRequest(mockRequest, 200, mockResponseHeaders, mockRouterContext, mockLoadContext), + ).rejects.toThrow(testError); + }); +}); diff --git a/packages/react-router/test/server/wrapSentryHandleRequest.test.ts b/packages/react-router/test/server/wrapSentryHandleRequest.test.ts new file mode 100644 index 000000000000..e29e97f14c57 --- /dev/null +++ b/packages/react-router/test/server/wrapSentryHandleRequest.test.ts @@ -0,0 +1,183 @@ +import { describe, test, expect, beforeEach, vi } from 'vitest'; +import { RPCType } from '@opentelemetry/core'; +import { ATTR_HTTP_ROUTE } from '@opentelemetry/semantic-conventions'; +import { SEMANTIC_ATTRIBUTE_SENTRY_SOURCE, getActiveSpan, getRootSpan, getTraceMetaTags } from '@sentry/core'; +import { PassThrough } from 'stream'; +import { wrapSentryHandleRequest, getMetaTagTransformer } from '../../src/server/wrapSentryHandleRequest'; + +vi.mock('@opentelemetry/core', () => ({ + RPCType: { HTTP: 'http' }, + getRPCMetadata: vi.fn(), +})); + +vi.mock('@sentry/core', () => ({ + SEMANTIC_ATTRIBUTE_SENTRY_SOURCE: 'sentry.source', + getActiveSpan: vi.fn(), + getRootSpan: vi.fn(), + getTraceMetaTags: vi.fn(), +})); + +describe('wrapSentryHandleRequest', () => { + beforeEach(() => { + vi.clearAllMocks(); + }); + + test('should call original handler with same parameters', async () => { + const originalHandler = vi.fn().mockResolvedValue('original response'); + const wrappedHandler = wrapSentryHandleRequest(originalHandler); + + const request = new Request('https://taco.burrito'); + const responseStatusCode = 200; + const responseHeaders = new Headers(); + const routerContext = { staticHandlerContext: { matches: [] } } as any; + const loadContext = {} as any; + + const result = await wrappedHandler(request, responseStatusCode, responseHeaders, routerContext, loadContext); + + expect(originalHandler).toHaveBeenCalledWith( + request, + responseStatusCode, + responseHeaders, + routerContext, + loadContext, + ); + expect(result).toBe('original response'); + }); + + test('should set span attributes when parameterized path exists and active span exists', async () => { + const originalHandler = vi.fn().mockResolvedValue('test'); + const wrappedHandler = wrapSentryHandleRequest(originalHandler); + + const mockActiveSpan = { setAttribute: vi.fn() }; + const mockRootSpan = { setAttributes: vi.fn() }; + const mockRpcMetadata = { type: RPCType.HTTP, route: '/some-path' }; + + (getActiveSpan as unknown as ReturnType).mockReturnValue(mockActiveSpan); + (getRootSpan as unknown as ReturnType).mockReturnValue(mockRootSpan); + const getRPCMetadata = vi.fn().mockReturnValue(mockRpcMetadata); + vi.mocked(vi.importActual('@opentelemetry/core')).getRPCMetadata = getRPCMetadata; + + const routerContext = { + staticHandlerContext: { + matches: [{ route: { path: 'some-path' } }], + }, + } as any; + + await wrappedHandler(new Request('https://nacho.queso'), 200, new Headers(), routerContext, {} as any); + + expect(getActiveSpan).toHaveBeenCalled(); + expect(getRootSpan).toHaveBeenCalledWith(mockActiveSpan); + expect(mockRootSpan.setAttributes).toHaveBeenCalledWith({ + [ATTR_HTTP_ROUTE]: '/some-path', + [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'route', + }); + expect(mockRpcMetadata.route).toBe('/some-path'); + }); + + test('should not set span attributes when parameterized path does not exist', async () => { + const originalHandler = vi.fn().mockResolvedValue('test'); + const wrappedHandler = wrapSentryHandleRequest(originalHandler); + + const routerContext = { + staticHandlerContext: { + matches: [], + }, + } as any; + + await wrappedHandler(new Request('https://guapo.chulo'), 200, new Headers(), routerContext, {} as any); + + expect(getActiveSpan).not.toHaveBeenCalled(); + }); + + test('should not set span attributes when active span does not exist', async () => { + const originalHandler = vi.fn().mockResolvedValue('test'); + const wrappedHandler = wrapSentryHandleRequest(originalHandler); + + (getActiveSpan as unknown as ReturnType).mockReturnValue(null); + + const routerContext = { + staticHandlerContext: { + matches: [{ route: { path: 'some-path' } }], + }, + } as any; + + await wrappedHandler(new Request('https://tio.pepe'), 200, new Headers(), routerContext, {} as any); + + expect(getActiveSpan).toHaveBeenCalled(); + expect(getRootSpan).not.toHaveBeenCalled(); + }); +}); + +describe('getMetaTagTransformer', () => { + beforeEach(() => { + vi.clearAllMocks(); + (getTraceMetaTags as unknown as ReturnType).mockReturnValue( + '', + ); + }); + + test('should inject meta tags before closing head tag', done => { + const outputStream = new PassThrough(); + const bodyStream = new PassThrough(); + const transformer = getMetaTagTransformer(bodyStream); + + let outputData = ''; + outputStream.on('data', chunk => { + outputData += chunk.toString(); + }); + + outputStream.on('end', () => { + expect(outputData).toContain(''); + expect(outputData).not.toContain(''); + done(); + }); + + transformer.pipe(outputStream); + + bodyStream.write('Test'); + bodyStream.end(); + }); + + test('should not modify chunks without head closing tag', done => { + const outputStream = new PassThrough(); + const bodyStream = new PassThrough(); + const transformer = getMetaTagTransformer(bodyStream); + + let outputData = ''; + outputStream.on('data', chunk => { + outputData += chunk.toString(); + }); + + outputStream.on('end', () => { + expect(outputData).toBe('Test'); + expect(getTraceMetaTags).toHaveBeenCalled(); + done(); + }); + + transformer.pipe(outputStream); + + bodyStream.write('Test'); + bodyStream.end(); + }); + + test('should handle buffer input', done => { + const outputStream = new PassThrough(); + const bodyStream = new PassThrough(); + const transformer = getMetaTagTransformer(bodyStream); + + let outputData = ''; + outputStream.on('data', chunk => { + outputData += chunk.toString(); + }); + + outputStream.on('end', () => { + expect(outputData).toContain(''); + done(); + }); + + transformer.pipe(outputStream); + + bodyStream.write(Buffer.from('Test')); + bodyStream.end(); + }); +}); diff --git a/packages/react-router/tsconfig.json b/packages/react-router/tsconfig.json index 5f80a125a0dc..aa2dc034c7c3 100644 --- a/packages/react-router/tsconfig.json +++ b/packages/react-router/tsconfig.json @@ -4,6 +4,7 @@ "include": ["src/**/*"], "compilerOptions": { - "moduleResolution": "bundler" + "moduleResolution": "bundler", + "jsx": "react" } } From 1d10238b4e6fce45de7f2871cd5da4389e3e898f Mon Sep 17 00:00:00 2001 From: Francesco Gringl-Novy Date: Tue, 22 Apr 2025 11:15:48 +0200 Subject: [PATCH 2/4] ref(node): Log when incoming request bodies are being captured (#16104) We used to swallow most things when we try to capture request bodies of incoming requests in node. This can make it hard to debug this. This PR adds some log messages here: 1. Logs when we successfully patched `request.on` 2. Logs the errors that happen during patching ref https://github.com/getsentry/sentry-javascript/issues/16090 --- .../http/SentryHttpInstrumentation.ts | 23 ++++++++++++++----- 1 file changed, 17 insertions(+), 6 deletions(-) diff --git a/packages/node/src/integrations/http/SentryHttpInstrumentation.ts b/packages/node/src/integrations/http/SentryHttpInstrumentation.ts index 0e0502b6fd1f..32b97e628d66 100644 --- a/packages/node/src/integrations/http/SentryHttpInstrumentation.ts +++ b/packages/node/src/integrations/http/SentryHttpInstrumentation.ts @@ -30,6 +30,8 @@ import { getRequestInfo } from './vendor/getRequestInfo'; type Http = typeof http; type Https = typeof https; +const INSTRUMENTATION_NAME = '@sentry/instrumentation-http'; + export type SentryHttpInstrumentationOptions = InstrumentationConfig & { /** * Whether breadcrumbs should be recorded for requests. @@ -101,7 +103,7 @@ const MAX_BODY_BYTE_LENGTH = 1024 * 1024; */ export class SentryHttpInstrumentation extends InstrumentationBase { public constructor(config: SentryHttpInstrumentationOptions = {}) { - super('@sentry/instrumentation-http', VERSION, config); + super(INSTRUMENTATION_NAME, VERSION, config); } /** @inheritdoc */ @@ -377,6 +379,10 @@ function patchRequestToCaptureBody(req: IncomingMessage, isolationScope: Scope): apply: (target, thisArg, args: Parameters) => { const [event, listener, ...restArgs] = args; + if (DEBUG_BUILD) { + logger.log(INSTRUMENTATION_NAME, 'Patching request.on', event); + } + if (event === 'data') { const callback = new Proxy(listener, { apply: (target, thisArg, args: Parameters) => { @@ -387,7 +393,8 @@ function patchRequestToCaptureBody(req: IncomingMessage, isolationScope: Scope): chunks.push(chunk); } else if (DEBUG_BUILD) { logger.log( - `Dropping request body chunk because it maximum body length of ${MAX_BODY_BYTE_LENGTH}b is exceeded.`, + INSTRUMENTATION_NAME, + `Dropping request body chunk because maximum body length of ${MAX_BODY_BYTE_LENGTH}b is exceeded.`, ); } @@ -410,8 +417,10 @@ function patchRequestToCaptureBody(req: IncomingMessage, isolationScope: Scope): const normalizedRequest = { data: body } satisfies RequestEventData; isolationScope.setSDKProcessingMetadata({ normalizedRequest }); } - } catch { - // ignore errors here + } catch (error) { + if (DEBUG_BUILD) { + logger.error(INSTRUMENTATION_NAME, 'Error building captured request body', error); + } } return Reflect.apply(target, thisArg, args); @@ -445,8 +454,10 @@ function patchRequestToCaptureBody(req: IncomingMessage, isolationScope: Scope): return Reflect.apply(target, thisArg, args); }, }); - } catch { - // ignore errors if we can't patch stuff + } catch (error) { + if (DEBUG_BUILD) { + logger.error(INSTRUMENTATION_NAME, 'Error patching request to capture body', error); + } } } From 8026b854a1925b581381fadd038e241a6d094d4a Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Tue, 22 Apr 2025 15:49:58 +0000 Subject: [PATCH 3/4] build(deps): Bump fastify from 5.0.0 to 5.3.2 in /dev-packages/e2e-tests/test-applications/node-fastify-5 (#16097) --- .../e2e-tests/test-applications/node-fastify-5/package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dev-packages/e2e-tests/test-applications/node-fastify-5/package.json b/dev-packages/e2e-tests/test-applications/node-fastify-5/package.json index f9f4f726eb0e..08d6245f771e 100644 --- a/dev-packages/e2e-tests/test-applications/node-fastify-5/package.json +++ b/dev-packages/e2e-tests/test-applications/node-fastify-5/package.json @@ -15,7 +15,7 @@ "@sentry/core": "latest || *", "@sentry/opentelemetry": "latest || *", "@types/node": "^18.19.1", - "fastify": "5.0.0", + "fastify": "5.3.2", "typescript": "5.6.3", "ts-node": "10.9.2" }, From 447e62e89dc576995f7ebe791d3a133f7561c708 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Tue, 22 Apr 2025 15:50:29 +0000 Subject: [PATCH 4/4] feat(deps): Bump @prisma/instrumentation from 6.5.0 to 6.6.0 (#16102) --- packages/node/package.json | 2 +- yarn.lock | 9 +++++---- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/packages/node/package.json b/packages/node/package.json index 1d14d411238a..cfce53271da4 100644 --- a/packages/node/package.json +++ b/packages/node/package.json @@ -95,7 +95,7 @@ "@opentelemetry/resources": "^1.30.1", "@opentelemetry/sdk-trace-base": "^1.30.1", "@opentelemetry/semantic-conventions": "^1.30.0", - "@prisma/instrumentation": "6.5.0", + "@prisma/instrumentation": "6.6.0", "@sentry/core": "9.13.0", "@sentry/opentelemetry": "9.13.0", "import-in-the-middle": "^1.13.0" diff --git a/yarn.lock b/yarn.lock index 8895daad18dc..1dc296fdb9c5 100644 --- a/yarn.lock +++ b/yarn.lock @@ -5860,10 +5860,10 @@ resolved "https://registry.yarnpkg.com/@polka/url/-/url-1.0.0-next.28.tgz#d45e01c4a56f143ee69c54dd6b12eade9e270a73" integrity sha512-8LduaNlMZGwdZ6qWrKlfa+2M4gahzFkprZiAt2TF8uS0qQgBizKXpXURqvTJ4WtmupWxaLqjRb2UCTe72mu+Aw== -"@prisma/instrumentation@6.5.0": - version "6.5.0" - resolved "https://registry.yarnpkg.com/@prisma/instrumentation/-/instrumentation-6.5.0.tgz#ce6c160365dfccbe0f4e7c57a4afc4f946fee562" - integrity sha512-morJDtFRoAp5d/KENEm+K6Y3PQcn5bCvpJ5a9y3V3DNMrNy/ZSn2zulPGj+ld+Xj2UYVoaMJ8DpBX/o6iF6OiA== +"@prisma/instrumentation@6.6.0": + version "6.6.0" + resolved "https://registry.yarnpkg.com/@prisma/instrumentation/-/instrumentation-6.6.0.tgz#5b73164c722bcfcd29c43cb883b4735143b65eb2" + integrity sha512-M/a6njz3hbf2oucwdbjNKrSMLuyMCwgDrmTtkF1pm4Nm7CU45J/Hd6lauF2CDACTUYzu3ymcV7P0ZAhIoj6WRw== dependencies: "@opentelemetry/instrumentation" "^0.52.0 || ^0.53.0 || ^0.54.0 || ^0.55.0 || ^0.56.0 || ^0.57.0" @@ -27080,6 +27080,7 @@ stylus@0.59.0, stylus@^0.59.0: sucrase@^3.27.0, sucrase@^3.35.0, sucrase@getsentry/sucrase#es2020-polyfills: version "3.36.0" + uid fd682f6129e507c00bb4e6319cc5d6b767e36061 resolved "https://codeload.github.com/getsentry/sucrase/tar.gz/fd682f6129e507c00bb4e6319cc5d6b767e36061" dependencies: "@jridgewell/gen-mapping" "^0.3.2"