From 6392770922a2f2f1b45ffd62469702f170e5eb3e Mon Sep 17 00:00:00 2001 From: isaacs Date: Tue, 20 Oct 2026 15:24:11 -0700 Subject: [PATCH 1/4] ref(cloudflare): Use the shared request handler wrapper Moves Cloudflare's `wrapRequestHandler` onto `wrapServerRequestHandler`, which `Deno.serve` and `Bun.serve` use since the previous PRs. Cloudflare keeps only what is unique to it: client init, the `cf` and cloud resource contexts, the protocol from `cf.httpProtocol`, the `isStreamingResponse` option, `captureErrors`, and the flush through `waitUntil`, which skips the dispose for a 101 response. The helpers that only the old code used (`captureIncomingRequestBody`, `shouldCreateIncomingRequestSpan`, `getIncomingRequestSpanHooks` and `addRequest`) are gone. `wrapRequestHandler` must run on workers without the `nodejs_compat` flag, such as Shopify Oxygen, and every `@sentry/server-utils` entry loads `node:async_hooks`. So the wrapper moves to a new entry, `@sentry/server-utils/request-handler`, with no Node.js imports. Deno and Bun import it from there too. Cloudflare's module graph test allows this one entry, and a new test in server-utils walks the entry's module graph and fails on any Node.js import. The wrapper gets three optional fields: `isStreamingResponse`, `onDone` (called once when the request is done, with the response if there is one), and a `mechanism` that can be left out to not capture errors. For streamed responses, the shared stream monitor replaces Cloudflare's `TransformStream`. Workers with a compatibility date before 2022-11-30 cannot construct a `ReadableStream` without the `streams_enable_constructors` flag, so the wrapper then ends at handler return, as the `TransformStream` fallback did. The monitor now locks the source body only in `start()`, so the original response stays usable in that case. The wrapper builds the new response from the original one, which keeps `encodeBody: 'manual'`. Visible changes for Cloudflare: - Request spans set `client.address` from `CF-Connecting-IP` (or another forwarding header), as on the other runtimes. This follows `dataCollection.userInfo`, which is `true` by default. - Request spans get the response header attributes, and events get the `response` context. - An error from the source of a streamed response is captured (unless `captureErrors` is `false`), and the span gets an error status. - If the user removed `httpServerIntegration`, request bodies follow `dataCollection.httpBodies`. Before, the SDK attached no body. The minified `withSentry` bundle grows by about 2.4 KB (212,385 to 214,853 bytes), under the 211 KiB limit. Most of it is the request session and client address code, which Cloudflare now shares. Fixes #25577 Fixes JS-3975 Co-Authored-By: Claude Opus 5.5 (1M context) --- .size-limit.js | 1 + CHANGELOG.md | 8 + .../suites/public-api/startSpan/test.ts | 10 + .../subpath-request-handler/test.ts | 13 +- packages/bun/src/integrations/bunserver.ts | 2 +- .../cloudflare/src/integrations/httpServer.ts | 160 +++--------- packages/cloudflare/src/scope-utils.ts | 8 - .../src/wrapRequestHandlerWithInit.ts | 232 +++--------------- packages/cloudflare/test/request.test.ts | 20 +- .../test/requestModuleGraph.test.ts | 7 +- .../deno/src/wrap-deno-request-handler.ts | 2 +- packages/server-utils/package.json | 5 + packages/server-utils/rollup.npm.config.mjs | 1 + packages/server-utils/src/exports.ts | 2 - .../server-utils/src/index.request-handler.ts | 4 + .../server-utils/src/utils/monitorStream.ts | 51 +++- .../src/utils/wrapServerRequestHandler.ts | 104 ++++++-- .../test/requestHandlerEntry.test.ts | 53 ++++ .../test/utils/monitorStream.test.ts | 58 +++++ .../utils/wrapServerRequestHandler.test.ts | 44 ++++ 20 files changed, 406 insertions(+), 379 deletions(-) create mode 100644 packages/server-utils/src/index.request-handler.ts create mode 100644 packages/server-utils/test/requestHandlerEntry.test.ts diff --git a/.size-limit.js b/.size-limit.js index b841667d9d89..075c3fb89a6c 100644 --- a/.size-limit.js +++ b/.size-limit.js @@ -467,6 +467,7 @@ module.exports = [ ignore: [...builtinModules, ...nodePrefixedBuiltinModules], gzip: false, brotli: false, + limit: '521 KiB', disablePlugins: ['@size-limit/webpack'], webpack: false, modifyEsbuildConfig: function (config) { diff --git a/CHANGELOG.md b/CHANGELOG.md index 29fecaa4a5ad..b49e9282b484 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,14 @@ ## Unreleased +- **ref(cloudflare): Use the shared request handler wrapper** + + Cloudflare Workers now use the same request handler wrapper as `Deno.serve` and `Bun.serve`. Changes for Cloudflare users: + + - Request spans now set `client.address` from the `CF-Connecting-IP` header (or another forwarding header), as on the other runtimes. This follows `dataCollection.userInfo`, which is `true` by default. To leave it out, set `dataCollection: { userInfo: false }`. + - Request spans now have the response header attributes, and events have the `response` context with the status code. + - When the source of a streamed response fails, the SDK now captures the error and marks the span as failed. With `captureErrors: false`, it does not capture it. + - "You miss 100 percent of the chances you don't take. — Wayne Gretzky" — Michael Scott Work in this release was contributed by @lazerg and @chiliec. Thank you for your contributions! diff --git a/dev-packages/cloudflare-integration-tests/suites/public-api/startSpan/test.ts b/dev-packages/cloudflare-integration-tests/suites/public-api/startSpan/test.ts index 0fa5d410368b..adf52419022f 100644 --- a/dev-packages/cloudflare-integration-tests/suites/public-api/startSpan/test.ts +++ b/dev-packages/cloudflare-integration-tests/suites/public-api/startSpan/test.ts @@ -1,5 +1,6 @@ import { SDK_VERSION, SEMANTIC_ATTRIBUTE_SENTRY_SAMPLE_RATE } from '@sentry/core'; import { + CLIENT_ADDRESS, SENTRY_SEGMENT_NAME_SOURCE, SENTRY_SDK_NAME, SENTRY_SDK_VERSION, @@ -205,6 +206,15 @@ it('sends a streamed span envelope with correct spans for a manually started spa type: 'string', value: 'localhost', }, + // From the `CF-Connecting-IP` header that `wrangler dev` sets. + [CLIENT_ADDRESS]: { + type: 'string', + value: '127.0.0.1', + }, + 'http.response.header.content-type': { + type: 'array', + value: ['text/plain;charset=UTF-8'], + }, 'url.full': { type: 'string', value: expect.stringMatching(/^http:\/\/localhost:.+$/), diff --git a/dev-packages/cloudflare-integration-tests/suites/tracing/opentelemetry-tracer/subpath-request-handler/test.ts b/dev-packages/cloudflare-integration-tests/suites/tracing/opentelemetry-tracer/subpath-request-handler/test.ts index 596ae6c6ef6d..27e35a07270f 100644 --- a/dev-packages/cloudflare-integration-tests/suites/tracing/opentelemetry-tracer/subpath-request-handler/test.ts +++ b/dev-packages/cloudflare-integration-tests/suites/tracing/opentelemetry-tracer/subpath-request-handler/test.ts @@ -1,16 +1,19 @@ -import type { Event } from '@sentry/core'; +import { HTTP_SERVER } from '@sentry/conventions/op'; import { expect, it } from 'vitest'; import { createRunner } from '../../../../runner'; +import { getSpanOp, getSpansFromEnvelope } from '../../../../spanUtils'; it('does not capture spans emitted through @opentelemetry/api inside wrapRequestHandler from the /request subpath', async ({ signal, }) => { const runner = createRunner(__dirname) .expect(envelope => { - const event = envelope[1]?.[0]?.[1] as Event; - expect(event.transaction).toBe('GET /'); - expect(event.contexts?.trace?.op).toBe('http.server'); - expect(event.spans).toEqual([]); + // Only the request's segment span. The OpenTelemetry span is not captured. + const spans = getSpansFromEnvelope(envelope); + expect(spans).toHaveLength(1); + expect(spans[0]?.is_segment).toBe(true); + expect(spans[0]?.name).toBe('GET /'); + expect(getSpanOp(spans[0]!)).toBe(HTTP_SERVER); }) .start(signal); diff --git a/packages/bun/src/integrations/bunserver.ts b/packages/bun/src/integrations/bunserver.ts index 887f9582466f..cfdca8dca964 100644 --- a/packages/bun/src/integrations/bunserver.ts +++ b/packages/bun/src/integrations/bunserver.ts @@ -2,7 +2,7 @@ import type { Client, Integration, IntegrationFn, MaxRequestBodySize, Span, Span import { debug, defineIntegration, getClient, withIsolationScope } from '@sentry/core'; import type { AssertTrue, CheckServerRequestOptions, ServerRequestOptions } from '@sentry/core/server'; import { URL_PATH_PARAMETER_KEY_BASE } from '@sentry/conventions/attributes'; -import { wrapServerRequestHandler } from '@sentry/server-utils'; +import { wrapServerRequestHandler } from '@sentry/server-utils/request-handler'; import type { Server, ServeOptions } from 'bun'; const INTEGRATION_NAME = 'BunServer' as const; diff --git a/packages/cloudflare/src/integrations/httpServer.ts b/packages/cloudflare/src/integrations/httpServer.ts index 1975f168819e..dbcc4bb839b0 100644 --- a/packages/cloudflare/src/integrations/httpServer.ts +++ b/packages/cloudflare/src/integrations/httpServer.ts @@ -1,15 +1,6 @@ -import type { Client, IntegrationFn, MaxRequestBodySize, Span } from '@sentry/core'; -import { - captureBodyFromWinterCGRequest, - defineIntegration, - getIsolationScope, - hasSpansEnabled, - parseStringToURLObject, - safeCallback, -} from '@sentry/core'; -import type { AssertTrue, CheckServerRequestOptions } from '@sentry/core/server'; -import { isStaticAssetRequest } from '@sentry/core/server'; -import { DEBUG_BUILD } from '../debug-build'; +import type { Client, Integration, IntegrationFn, MaxRequestBodySize, Span } from '@sentry/core'; +import { defineIntegration } from '@sentry/core'; +import type { AssertTrue, CheckServerRequestOptions, ServerRequestOptions } from '@sentry/core/server'; const INTEGRATION_NAME = 'HttpServer' as const; @@ -100,21 +91,45 @@ type _ServerRequestOptionsCheck = AssertTrue< > >; -interface HttpServerIntegrationInstance extends HttpServerIntegrationOptions { - name: string; - maxRequestBodySize: MaxRequestBodySize | undefined; +type SupportedServerRequestOptions = Omit< + ServerRequestOptions, + 'sessions' | 'sessionFlushingDelayMS' +>; + +// Every supported shared option is a required key, so a new one fails to compile below until the +// integration passes it on. +type HttpServerRequestOptions = { + [K in keyof Required]: SupportedServerRequestOptions[K]; +}; + +interface HttpServerIntegration extends Integration { + requestOptions: HttpServerRequestOptions; +} + +/** + * The incoming request options of the client's `httpServerIntegration`. Without the integration, + * the SDK attaches no request body, so users can turn body capture off by removing it. + */ +export function getHttpServerRequestOptions(client: Client): ServerRequestOptions { + return ( + client.getIntegrationByName(INTEGRATION_NAME)?.requestOptions ?? { + maxRequestBodySize: 'none', + } + ); } -const _httpServerIntegration = ((options: HttpServerIntegrationOptions = {}): HttpServerIntegrationInstance => { +const _httpServerIntegration = ((options: HttpServerIntegrationOptions = {}): HttpServerIntegration => { return { name: INTEGRATION_NAME, - maxRequestBodySize: options.maxRequestBodySize, - ignoreRequestBody: options.ignoreRequestBody, - ignoreIncomingRequests: options.ignoreIncomingRequests, - ignoreStaticAssets: options.ignoreStaticAssets ?? true, - spans: options.spans, - onSpanCreated: options.onSpanCreated, - onSpanEnd: options.onSpanEnd, + requestOptions: { + maxRequestBodySize: options.maxRequestBodySize, + ignoreRequestBody: options.ignoreRequestBody, + ignoreIncomingRequests: options.ignoreIncomingRequests, + ignoreStaticAssets: options.ignoreStaticAssets ?? true, + spans: options.spans, + onSpanCreated: options.onSpanCreated, + onSpanEnd: options.onSpanEnd, + }, }; }) satisfies IntegrationFn; @@ -137,102 +152,3 @@ const _httpServerIntegration = ((options: HttpServerIntegrationOptions = {}): Ht * ``` */ export const httpServerIntegration = defineIntegration(_httpServerIntegration); - -/** - * Capture the request body based on the HttpServer integration config. - * Called internally by `wrapRequestHandler`. - */ -export async function captureIncomingRequestBody(client: Client, request: Request): Promise { - const integration = client.getIntegrationByName(INTEGRATION_NAME); - - if (!integration) { - return; - } - - const configuredBodySize = integration.maxRequestBodySize; - const effectiveBodySize: MaxRequestBodySize = - configuredBodySize ?? - (client.getDataCollectionOptions().httpBodies.includes('incomingRequest') ? 'medium' : 'none'); - - if (effectiveBodySize === 'none') { - return; - } - - // Skip GET and HEAD requests - they don't have bodies - // Also skip OPTIONS, even if they may have a body, they might not give a lot of extra value - if (request.method === 'GET' || request.method === 'HEAD' || request.method === 'OPTIONS') { - return; - } - - const ignoreRequestBody = integration.ignoreRequestBody; - if ( - ignoreRequestBody && - // If the filter throws, skip the body so it cannot leak data the filter was meant to hide. - safeCallback( - DEBUG_BUILD ? 'Error in `ignoreRequestBody`, skipping the body:' : '', - () => ignoreRequestBody(request.url, request), - () => true, - ) - ) { - return; - } - - const isolationScope = getIsolationScope(); - await captureBodyFromWinterCGRequest(request, isolationScope, effectiveBodySize); -} - -/** - * Whether to create a span for the incoming request, based on the HttpServer integration config. - * Called internally by `wrapRequestHandler`. - */ -export function shouldCreateIncomingRequestSpan(client: Client | undefined, request: Request): boolean { - if (!client) { - return false; - } - - const integration = client.getIntegrationByName(INTEGRATION_NAME); - if (!(integration?.spans ?? hasSpansEnabled(client.getOptions()))) { - return false; - } - - const urlObject = parseStringToURLObject(request.url); - const urlPath = urlObject ? `${urlObject.pathname}${urlObject.search}` : '/'; - if (integration?.ignoreStaticAssets && request.method === 'GET' && isStaticAssetRequest(urlPath)) { - return false; - } - - const ignoreIncomingRequests = integration?.ignoreIncomingRequests; - return !( - ignoreIncomingRequests && - safeCallback( - DEBUG_BUILD ? 'Error in `ignoreIncomingRequests`, creating the span:' : '', - () => ignoreIncomingRequests(urlPath, request), - () => false, - ) - ); -} - -/** - * The `onSpanCreated` and `onSpanEnd` hooks of the HttpServer integration, guarded so that a hook - * that throws does not fail the request. Called internally by `wrapRequestHandler`. - */ -export function getIncomingRequestSpanHooks(client: Client | undefined): { - onSpanCreated: (span: Span, request: Request) => void; - onSpanEnd: (span: Span, request: Request, response: Response) => void; -} { - const integration = client?.getIntegrationByName(INTEGRATION_NAME); - return { - onSpanCreated: (span, request) => - safeCallback( - DEBUG_BUILD ? 'Error in `onSpanCreated`:' : '', - () => integration?.onSpanCreated?.(span, request), - () => undefined, - ), - onSpanEnd: (span, request, response) => - safeCallback( - DEBUG_BUILD ? 'Error in `onSpanEnd`:' : '', - () => integration?.onSpanEnd?.(span, request, response), - () => undefined, - ), - }; -} diff --git a/packages/cloudflare/src/scope-utils.ts b/packages/cloudflare/src/scope-utils.ts index 540a4589c1a5..22a3e05aebc7 100644 --- a/packages/cloudflare/src/scope-utils.ts +++ b/packages/cloudflare/src/scope-utils.ts @@ -1,7 +1,6 @@ import { CLOUD_PROVIDER } from '@sentry/conventions/attributes'; import type { CfProperties, IncomingRequestCfProperties } from '@cloudflare/workers-types'; import type { Scope } from '@sentry/core'; -import { winterCGRequestToRequestData } from '@sentry/core'; /** * Set cloud resource context on scope. @@ -20,10 +19,3 @@ export function addCultureContext(scope: Scope, cf: IncomingRequestCfProperties timezone: cf.timezone, }); } - -/** - * Set request data on scope - */ -export function addRequest(scope: Scope, request: Request): void { - scope.setSDKProcessingMetadata({ normalizedRequest: winterCGRequestToRequestData(request) }); -} diff --git a/packages/cloudflare/src/wrapRequestHandlerWithInit.ts b/packages/cloudflare/src/wrapRequestHandlerWithInit.ts index 9c2126b9add2..b10797a7023a 100644 --- a/packages/cloudflare/src/wrapRequestHandlerWithInit.ts +++ b/packages/cloudflare/src/wrapRequestHandlerWithInit.ts @@ -1,39 +1,15 @@ import type { CfProperties, IncomingRequestCfProperties } from '@cloudflare/workers-types'; -import { - NETWORK_PROTOCOL_NAME, - NETWORK_PROTOCOL_VERSION, - SENTRY_OP, - SENTRY_SEGMENT_NAME_SOURCE, - HTTP_REQUEST_BODY_SIZE, - USER_AGENT_ORIGINAL, -} from '@sentry/conventions/attributes'; -import { HTTP_SERVER } from '@sentry/conventions/op'; -import type { Span } from '@sentry/core'; -import { - captureException, - continueTrace, - getHttpSpanDetailsFromUrlObject, - hasSpanStreamingEnabled, - httpHeadersToSpanAttributes, - HTTP_SPAN_NAME_FALLBACK, - parseStringToURLObject, - safeCallback, - setHttpStatus, - startSpanManual, - winterCGHeadersToDict, -} from '@sentry/core'; -import { classifyResponseStreaming } from '@sentry/core/server'; +import { NETWORK_PROTOCOL_NAME, NETWORK_PROTOCOL_VERSION } from '@sentry/conventions/attributes'; +import type { SpanAttributes } from '@sentry/core'; +import { safeCallback } from '@sentry/core'; +import { wrapServerRequestHandler } from '@sentry/server-utils/request-handler'; import { DEBUG_BUILD } from './debug-build'; -import { - captureIncomingRequestBody, - getIncomingRequestSpanHooks, - shouldCreateIncomingRequestSpan, -} from './integrations/httpServer'; +import { getHttpServerRequestOptions } from './integrations/httpServer'; import { flushDeferredChannelEvents } from './orchestrion-deferred-channels'; import type { CloudflareClient, CloudflareOptions } from './client'; import type { ExecutionContextCompat } from './executionContext'; import { flushAndDispose, getOriginalWaitUntil } from './flush'; -import { addCloudResourceContext, addCultureContext, addRequest } from './scope-utils'; +import { addCloudResourceContext, addCultureContext } from './scope-utils'; import { getInvocationState, getInvocationWaitUntil } from './utils/invocationContext'; import { withInvocationIsolationScope } from './utils/invocationScope'; @@ -87,7 +63,6 @@ export function wrapRequestHandlerWithInit( : context ? getOriginalWaitUntil(context).bind(context) : undefined; - const errorMechanismType = getRequestErrorMechanismType(context); const client = initSdk({ ...options, ctx: context }); isolationScope.setClient(client); @@ -97,43 +72,8 @@ export function wrapRequestHandlerWithInit( // request, where workerd allows publish, and init() above has registered subscribers. flushDeferredChannelEvents(); - const urlObject = parseStringToURLObject(request.url); - const [rawName, attributes] = getHttpSpanDetailsFromUrlObject( - urlObject, - 'server', - 'auto.http.cloudflare', - request, - undefined, - client, - ); - // With span streaming, span names have to be low cardinality, so we can't fall back to the URL. - // A `route` source means the name already is (e.g. the `/` path), so it is kept as-is. - const name = - attributes[SENTRY_SEGMENT_NAME_SOURCE] === 'route' || !client || !hasSpanStreamingEnabled(client) - ? rawName - : request.method?.toUpperCase() || HTTP_SPAN_NAME_FALLBACK; - - const contentLength = request.headers.get('content-length'); - if (contentLength) { - attributes[HTTP_REQUEST_BODY_SIZE] = parseInt(contentLength, 10); - } - - const userAgentHeader = request.headers.get('user-agent'); - if (userAgentHeader) { - attributes[USER_AGENT_ORIGINAL] = userAgentHeader; - } - - if (client) { - Object.assign( - attributes, - httpHeadersToSpanAttributes(winterCGHeadersToDict(request.headers), client.getDataCollectionOptions()), - ); - } - - attributes[SENTRY_OP] = HTTP_SERVER; - addCloudResourceContext(isolationScope); - addRequest(isolationScope, request); + const attributes: SpanAttributes = {}; if (request.cf) { addCultureContext(isolationScope, request.cf); @@ -144,143 +84,47 @@ export function wrapRequestHandlerWithInit( } } - // Do not capture spans for OPTIONS and HEAD requests - if (request.method === 'OPTIONS' || request.method === 'HEAD') { + if (!client) { try { return await handler(); - } catch (e) { - if (captureErrors) { - captureException(e, { mechanism: { handled: false, type: errorMechanismType } }); - } - throw e; } finally { waitUntil?.(flushAndDispose(client)); } } - if (client) { - await captureIncomingRequestBody(client, request); - } - - const { onSpanCreated, onSpanEnd } = getIncomingRequestSpanHooks(client); - - // Runs the handler. Ends the span (if any) and flushes when the response is done. - const respond = async (span: Span | undefined): Promise => { - let res: Response; - - try { - res = await handler(); - if (span) { - setHttpStatus(span, res.status); - } - - // After the handler runs, the span name might have been updated by nested instrumentation - // (e.g., Remix parameterizing routes). The span should already have the correct name - // from that instrumentation, so we don't need to do anything here. - } catch (e) { - span?.end(); - if (captureErrors) { - captureException(e, { mechanism: { handled: false, type: errorMechanismType } }); - } - waitUntil?.(flushAndDispose(client)); - throw e; - } - - let ended = false; - const endSpan = (): void => { - if (ended) return; - - ended = true; - if (span) { - onSpanEnd(span, request, res); - span.end(); - } - }; - - const userClassification = safeCallback( - DEBUG_BUILD ? 'Error in `isStreamingResponse`, using the default classification:' : '', - () => options.isStreamingResponse?.(res, request), - () => undefined, - ); - // An async callback returns a promise, which would otherwise count as `true`. - const isStreaming = - typeof userClassification === 'boolean' ? userClassification : classifyResponseStreaming(res).isStreaming; - - if (isStreaming && res.body) { - try { - let transformerUsed = false; - - const endSpanOnce = (): void => { - if (ended) return; - - endSpan(); + // Note: The span will not have a duration unless I/O happens in the handler. This is + // because of how the cloudflare workers runtime works. + // See: https://developers.cloudflare.com/workers/runtime-apis/performance/ + return wrapServerRequestHandler( + { + request, + client, + isolationScope, + options: getHttpServerRequestOptions(client), + origin: 'auto.http.cloudflare', + mechanism: captureErrors ? { handled: false, type: getRequestErrorMechanismType(context) } : undefined, + attributes, + isStreamingResponse: response => { + const classification = safeCallback( + DEBUG_BUILD ? 'Error in `isStreamingResponse`, using the default classification:' : '', + () => options.isStreamingResponse?.(response, request), + () => undefined, + ); + // An async callback returns a promise, which would otherwise count as `true`. + return typeof classification === 'boolean' ? classification : undefined; + }, + onDone: response => { + // Don't dispose for protocol upgrades (101 Switching Protocols) - the connection stays alive. + // This includes WebSocket upgrades where webSocketMessage/webSocketClose handlers + // will still be called and may need the client to capture errors. + if (response?.status === 101) { + waitUntil?.(client.flush(2000)); + } else { waitUntil?.(flushAndDispose(client)); - }; - - // Workers with a compatibility date before 2022-11-30 and without the - // `transformstream_enable_standard_constructor` flag (for example under older mini-oxygen - // releases) ignore the transformer, so `flush` and `cancel` would never end the span. Only a - // used transformer runs `start`. - const transform = new TransformStream({ - start() { - transformerUsed = true; - }, - flush() { - // Source stream completed normally. - endSpanOnce(); - }, - cancel() { - // Client disconnected (or downstream cancelled). The `cancel` - // is being called while the response is still considered - // active, so this is a safe place to end the span. - endSpanOnce(); - }, - }); - - if (transformerUsed) { - // Passing the original response as the init keeps `encodeBody: 'manual'`, which a - // Response does not expose as a property. Without it, workerd compresses a - // pre-compressed body a second time. - return new Response(res.body.pipeThrough(transform), res); } - } catch { - // Falls back to ending the span at handler return. - } - } - - // Non-streaming response - end span immediately and return original - endSpan(); - - // Don't dispose for protocol upgrades (101 Switching Protocols) - the connection stays alive. - // This includes WebSocket upgrades where webSocketMessage/webSocketClose handlers - // will still be called and may need the client to capture errors. - if (res.status === 101) { - waitUntil?.(client?.flush(2000)); - } else { - waitUntil?.(flushAndDispose(client)); - } - return res; - }; - - const createSpan = shouldCreateIncomingRequestSpan(client, request); - - return continueTrace( - { sentryTrace: request.headers.get('sentry-trace') || '', baggage: request.headers.get('baggage') }, - () => { - if (!createSpan) { - return respond(undefined); - } - - // Note: This span will not have a duration unless I/O happens in the handler. This is - // because of how the cloudflare workers runtime works. - // See: https://developers.cloudflare.com/workers/runtime-apis/performance/ - - // Use startSpanManual to control when span ends (needed for streaming responses) - return startSpanManual({ name, attributes }, span => { - onSpanCreated(span, request); - return respond(span); - }); + }, }, + () => handler(), ); }, wrapperOptions.context); } diff --git a/packages/cloudflare/test/request.test.ts b/packages/cloudflare/test/request.test.ts index 972180c62a8c..d05ad7420d44 100644 --- a/packages/cloudflare/test/request.test.ts +++ b/packages/cloudflare/test/request.test.ts @@ -677,6 +677,7 @@ describe('withSentry', () => { 'http.response.status_code': 200, 'http.request.body.size': 10, 'http.request.header.content-length': ['10'], + 'http.response.header.content-type': ['text/plain;charset=UTF-8'], }, op: 'http.server', origin: 'auto.http.cloudflare', @@ -1297,13 +1298,16 @@ describe('flushAndDispose', () => { }, ); - test('tears down at handler return when TransformStream ignores the transformer', async () => { - // workerd without `transformstream_enable_standard_constructor` returns an identity stream. + test('tears down at handler return when the ReadableStream constructor is not available', async () => { + const response = new Response('
shell
', { + headers: { 'content-type': 'text/x-component;charset=utf-8' }, + }); + // workerd before the 2022-11-30 compatibility date, without `streams_enable_constructors`. vi.stubGlobal( - 'TransformStream', - class extends TransformStream { + 'ReadableStream', + class { constructor() { - super(); + throw new TypeError('To use the new ReadableStream() constructor, enable streams_enable_constructors.'); } }, ); @@ -1316,16 +1320,14 @@ describe('flushAndDispose', () => { const flushSpy = vi.spyOn(SentryCore.Client.prototype, 'flush').mockResolvedValue(true); - const response = new Response('
shell
', { - headers: { 'content-type': 'text/x-component;charset=utf-8' }, - }); - const result = await wrapRequestHandler( { options, request: new Request('https://example.com'), context }, () => response, ); expect(result).toBe(response); + expect(result.bodyUsed).toBe(false); + expect(result.body?.locked).toBe(false); expect(waitUntil).toHaveBeenCalledTimes(1); flushSpy.mockRestore(); diff --git a/packages/cloudflare/test/requestModuleGraph.test.ts b/packages/cloudflare/test/requestModuleGraph.test.ts index 64827ebc8b5c..8b021862a819 100644 --- a/packages/cloudflare/test/requestModuleGraph.test.ts +++ b/packages/cloudflare/test/requestModuleGraph.test.ts @@ -69,7 +69,12 @@ describe('module graph of `wrapRequestHandler`', () => { it('contains no package that depends on Node.js APIs', () => { // `@sentry/server-utils` subscribes to `node:diagnostics_channel` and `@sentry/node` needs Node.js // throughout. Both are only allowed in `sdk.ts`, `index.ts` and `vite/` (see `.oxlintrc.json`). - expect(externals.filter(specifier => /^@sentry\/(node|server-utils)(\/|$)/.test(specifier))).toEqual([]); + // The `request-handler` entry of `@sentry/server-utils` has no Node.js imports. + const nodePackages = externals.filter( + specifier => + /^@sentry\/(node|server-utils)(\/|$)/.test(specifier) && specifier !== '@sentry/server-utils/request-handler', + ); + expect(nodePackages).toEqual([]); }); it('does not reach `async.ts`, the only shipped module importing `node:async_hooks`', () => { diff --git a/packages/deno/src/wrap-deno-request-handler.ts b/packages/deno/src/wrap-deno-request-handler.ts index 4d24405d58e4..e050c5523a2d 100644 --- a/packages/deno/src/wrap-deno-request-handler.ts +++ b/packages/deno/src/wrap-deno-request-handler.ts @@ -1,7 +1,7 @@ import type { Client } from '@sentry/core'; import { debug, getClient, withIsolationScope } from '@sentry/core'; import type { ServerRequestOptions } from '@sentry/core/server'; -import { wrapServerRequestHandler } from '@sentry/server-utils'; +import { wrapServerRequestHandler } from '@sentry/server-utils/request-handler'; export type RequestHandlerWrapperOptions = { request: Request; diff --git a/packages/server-utils/package.json b/packages/server-utils/package.json index 0b0307509a36..8a19ea2e4746 100644 --- a/packages/server-utils/package.json +++ b/packages/server-utils/package.json @@ -27,6 +27,11 @@ "import": "./build/esm/index.no-diagnostic-channels.js", "require": "./build/cjs/index.no-diagnostic-channels.js" }, + "./request-handler": { + "types": "./build/types/index.request-handler.d.ts", + "import": "./build/esm/index.request-handler.js", + "require": "./build/cjs/index.request-handler.js" + }, "./orchestrion/config": { "types": "./build/types/orchestrion/config/index.d.ts", "import": "./build/esm/orchestrion/config/index.js", diff --git a/packages/server-utils/rollup.npm.config.mjs b/packages/server-utils/rollup.npm.config.mjs index ab0a51d3ef21..74f172ec5215 100644 --- a/packages/server-utils/rollup.npm.config.mjs +++ b/packages/server-utils/rollup.npm.config.mjs @@ -100,6 +100,7 @@ export default [ entrypoints: [ 'src/index.ts', 'src/index.no-diagnostic-channels.ts', + 'src/index.request-handler.ts', 'src/orchestrion/config/index.ts', 'src/orchestrion/bundler/vite.ts', 'src/orchestrion/bundler/rollup.ts', diff --git a/packages/server-utils/src/exports.ts b/packages/server-utils/src/exports.ts index 799ed7c69d18..15381f95dc9b 100644 --- a/packages/server-utils/src/exports.ts +++ b/packages/server-utils/src/exports.ts @@ -1,7 +1,5 @@ // Shared exports not using diagnostics channels export { setHttpServerSpanRouteAttribute } from './utils/setHttpServerSpanRouteAttribute'; -export { wrapServerRequestHandler } from './utils/wrapServerRequestHandler'; -export type { ServerRequestHandlerConfig } from './utils/wrapServerRequestHandler'; export { setAsyncLocalStorageAsyncContextStrategy } from './async-context'; export { openTelemetryIntegration, getOtlpTracesEndpoint } from './opentelemetry'; export * from './ai'; diff --git a/packages/server-utils/src/index.request-handler.ts b/packages/server-utils/src/index.request-handler.ts new file mode 100644 index 000000000000..73e0902cc088 --- /dev/null +++ b/packages/server-utils/src/index.request-handler.ts @@ -0,0 +1,4 @@ +// The web `Request` handler wrapper, on its own entry with no Node.js imports, so Cloudflare can use +// it on workers without the `nodejs_compat` compatibility flag. +export { wrapServerRequestHandler } from './utils/wrapServerRequestHandler'; +export type { ServerRequestHandlerConfig } from './utils/wrapServerRequestHandler'; diff --git a/packages/server-utils/src/utils/monitorStream.ts b/packages/server-utils/src/utils/monitorStream.ts index 6b4651f990d1..20a69a15e79a 100644 --- a/packages/server-utils/src/utils/monitorStream.ts +++ b/packages/server-utils/src/utils/monitorStream.ts @@ -1,39 +1,68 @@ -import { isError } from '@sentry/core'; +import { debug, isError } from '@sentry/core'; +import { DEBUG_BUILD } from '../debug-build'; -/** Monitor the source without reading ahead or treating cancellation as a source failure. */ +/** + * Monitor the source without reading ahead or treating cancellation as a source failure. + * + * `onDone` runs once, while the returned stream is still open: before it closes, before it errors, + * or when the consumer cancels it. A runtime can end the request when the response body closes + * (Cloudflare Workers then reject `waitUntil`), so work that `onDone` starts must begin first. + */ export function monitorStream( stream: ReadableStream, onDone: () => void, onError: (error: unknown) => void, ): ReadableStream { - const reader = stream.getReader(); - // A cancelled reader closes normally; only a source failure should be captured as an error. - // A source that stops on `request.signal` errors with an `AbortError` when the client - // disconnects, which is a cancellation too. - void reader.closed.then(onDone, error => { - if (!(isError(error) && error.name === 'AbortError')) { - onError(error); + let reader: ReadableStreamDefaultReader; + let finished = false; + const finish = (error?: { error: unknown }): void => { + if (finished) { + return; } - onDone(); - }); + finished = true; + // A callback that throws must not fail the response body. + try { + // A source that stops on `request.signal` errors with an `AbortError` when the client + // disconnects, which is a cancellation too. + if (error && !(isError(error.error) && error.error.name === 'AbortError')) { + onError(error.error); + } + onDone(); + } catch (e) { + DEBUG_BUILD && debug.error('Error when a monitored stream finished:', e); + } + }; return new ReadableStream( { + // Locks the source only once the stream exists, so a constructor that throws leaves it usable. + start() { + reader = stream.getReader(); + // A source can fail while no one reads from it. A cancelled reader closes normally; only a + // source failure should be captured as an error. + void reader.closed.then( + () => finish(), + error => finish({ error }), + ); + }, async pull(controller) { try { const { done, value } = await reader.read(); if (done) { + finish(); controller.close(); reader.releaseLock(); } else { controller.enqueue(value); } } catch (error) { + finish({ error }); controller.error(error); reader.releaseLock(); } }, async cancel(reason) { + finish(); try { await reader.cancel(reason); } finally { diff --git a/packages/server-utils/src/utils/wrapServerRequestHandler.ts b/packages/server-utils/src/utils/wrapServerRequestHandler.ts index 447764ddf5e5..3f833ba9047e 100644 --- a/packages/server-utils/src/utils/wrapServerRequestHandler.ts +++ b/packages/server-utils/src/utils/wrapServerRequestHandler.ts @@ -8,11 +8,12 @@ import { USER_AGENT_ORIGINAL, } from '@sentry/conventions/attributes'; import { HTTP_SERVER } from '@sentry/conventions/op'; -import type { Client, Mechanism, Scope, Span, SpanAttributes } from '@sentry/core'; +import type { Client, Mechanism, Scope, SpanAttributes } from '@sentry/core'; import { captureBodyFromWinterCGRequest, captureException, continueTrace, + getCurrentScope, getHttpSpanDetailsFromUrlObject, hasSpansEnabled, hasSpanStreamingEnabled, @@ -25,6 +26,8 @@ import { startSpanManual, winterCGHeadersToDict, winterCGRequestToRequestData, + withIsolationScope, + withScope, } from '@sentry/core'; import type { ServerRequestOptions } from '@sentry/core/server'; import { @@ -45,14 +48,21 @@ export interface ServerRequestHandlerConfig { options: ServerRequestOptions | undefined; /** Origin of the span, e.g. `auto.http.deno`. */ origin: string; - /** Mechanism of errors that the handler or the response stream throws. */ - mechanism: Mechanism; + /** Mechanism of errors that the handler or the response stream throws. Omit to not capture them. */ + mechanism?: Mechanism; /** The address and port of the socket, if the runtime knows them. */ getSocketAddress?: () => { address: string; port?: number } | undefined; /** The matched route of the server framework, e.g. `/users/:id`. Sets `http.route` and the span name. */ routeName?: string; /** Span attributes that only the runtime knows, e.g. the route parameters. */ attributes?: SpanAttributes; + /** Whether the response streams. Return `undefined` to use the default classification. */ + isStreamingResponse?: (response: Response) => boolean | undefined; + /** + * Called once when the request is done: when the response is returned, when a streamed body + * finishes, or when the handler throws (then without a response). + */ + onDone?: (response: Response | undefined) => void; } /** @@ -74,6 +84,11 @@ export async function wrapServerRequestHandler { const { request, client, isolationScope, options, mechanism } = config; const method = request.method.toUpperCase(); + const captureError = (error: unknown): void => { + if (mechanism) { + captureException(error, { mechanism }); + } + }; const endSession = options?.sessions ? startRequestSession(client, { @@ -81,6 +96,14 @@ export async function wrapServerRequestHandler { + endSession?.(); + config.onDone?.(response); + }; + const isStreamingResponse = (response: Response): boolean => { + const classification = config.isStreamingResponse?.(response); + return typeof classification === 'boolean' ? classification : classifyResponseStreaming(response).isStreaming; + }; const socketAddress = config.getSocketAddress?.(); isolationScope.setSDKProcessingMetadata({ @@ -89,13 +112,15 @@ export async function wrapServerRequestHandler endSession?.()); + const inRequestScope = bindToRequestScope(isolationScope); + return onResponseDone( + response, + !!response && isStreamingResponse(response), + inRequestScope(() => finish(response)), + inRequestScope(captureError), + ); } const [rawName, attributes] = getHttpSpanDetailsFromUrlObject( @@ -206,8 +237,8 @@ export async function wrapServerRequestHandler { + !!response && isStreamingResponse(response), + inRequestScope(() => { if (onSpanEnd && response) { safeCallback( DEBUG_BUILD ? 'Error in `onSpanEnd`:' : '', @@ -231,15 +263,35 @@ export async function wrapServerRequestHandler { + span.setStatus({ code: SPAN_STATUS_ERROR, message: 'internal_error' }); + captureError(error); + }), ); }); }, ); } +/** + * Returns a function that binds a callback to the request's isolation scope and the current scope. + * + * A streamed body finishes after the handler returns. Without `AsyncLocalStorage` (Cloudflare + * Workers without `nodejs_compat`), its callbacks then run outside the request's scopes, where + * `getClient()` finds no client: the segment span would be sent as a transaction, and a captured + * error would not update the request session. + */ +function bindToRequestScope( + isolationScope: Scope, +): (fn: (...args: A) => void) => (...args: A) => void { + const scope = getCurrentScope(); + return fn => + (...args) => + withIsolationScope(isolationScope, () => withScope(scope, () => fn(...args))); +} + function shouldCreateSpan( client: Client, options: ServerRequestOptions | undefined, @@ -269,17 +321,19 @@ function shouldCreateSpan( */ function onResponseDone( response: R, - mechanism: Mechanism, + isStreaming: boolean, onDone: () => void, - span?: Span, + onStreamError: (error: unknown) => void, ): R { - if (response?.body && !response.body.locked && classifyResponseStreaming(response).isStreaming) { - const body = monitorStream(response.body, onDone, error => { - span?.setStatus({ code: SPAN_STATUS_ERROR, message: 'internal_error' }); - captureException(error, { mechanism }); - }); - // Passing the original response as the init keeps its status, status text and headers. - return new Response(body, response) as R; + if (isStreaming && response?.body && !response.body.locked) { + try { + // Passing the original response as the init keeps its status, status text and headers, and + // on Cloudflare also `encodeBody: 'manual'`, so workerd does not compress a compressed body again. + return new Response(monitorStream(response.body, onDone, onStreamError), response) as R; + } catch { + // Workers with a compatibility date before 2022-11-30 and without the + // `streams_enable_constructors` flag cannot construct a `ReadableStream`, so end at once. + } } onDone(); return response; diff --git a/packages/server-utils/test/requestHandlerEntry.test.ts b/packages/server-utils/test/requestHandlerEntry.test.ts new file mode 100644 index 000000000000..9df030fa3420 --- /dev/null +++ b/packages/server-utils/test/requestHandlerEntry.test.ts @@ -0,0 +1,53 @@ +import { builtinModules } from 'node:module'; +import { existsSync, readFileSync } from 'node:fs'; +import { dirname, join, resolve } from 'node:path'; +import { describe, expect, it } from 'vitest'; + +/** + * Cloudflare imports the `request-handler` entry on workers without the `nodejs_compat` compatibility + * flag, where any Node.js import fails at startup. Assert on the module graph itself rather than on + * any bundler's output. + */ +const ENTRY_POINT = resolve(__dirname, '../src/index.request-handler.ts'); +const IMPORT_SPECIFIER_REGEX = /\bfrom\s*['"]([^'"]+)['"]|\bimport\s*['"]([^'"]+)['"]/g; + +function collectExternals(entryPoint: string): string[] { + const modules = new Set(); + const externals = new Set(); + const queue = [entryPoint]; + + while (queue.length) { + const importer = queue.pop() as string; + if (modules.has(importer)) { + continue; + } + modules.add(importer); + + for (const match of readFileSync(importer, 'utf8').matchAll(IMPORT_SPECIFIER_REGEX)) { + const specifier = match[1] ?? (match[2] as string); + if (specifier.startsWith('.')) { + const withoutExtension = join(dirname(importer), specifier); + const resolved = [`${withoutExtension}.ts`, join(withoutExtension, 'index.ts')].find(existsSync); + if (!resolved) { + throw new Error(`Could not resolve '${specifier}' imported from '${importer}'`); + } + queue.push(resolved); + } else { + externals.add(specifier); + } + } + } + + return [...externals].sort(); +} + +describe('module graph of the `request-handler` entry', () => { + it('contains no Node.js built-in module and no package that needs Node.js', () => { + const externals = collectExternals(ENTRY_POINT); + + expect(externals.filter(specifier => specifier.startsWith('node:') || builtinModules.includes(specifier))).toEqual( + [], + ); + expect(externals.filter(specifier => /^@sentry\/(node|server-utils)(\/|$)/.test(specifier))).toEqual([]); + }); +}); diff --git a/packages/server-utils/test/utils/monitorStream.test.ts b/packages/server-utils/test/utils/monitorStream.test.ts index fee33b3cec75..d0921d34833d 100644 --- a/packages/server-utils/test/utils/monitorStream.test.ts +++ b/packages/server-utils/test/utils/monitorStream.test.ts @@ -117,4 +117,62 @@ describe('monitorStream', () => { await vi.waitFor(() => expect(onDone).toHaveBeenCalledTimes(1)); expect(onError).not.toHaveBeenCalled(); }); + + it('calls onDone before the stream closes', async () => { + const calls: string[] = []; + const source = new ReadableStream({ + start(controller) { + controller.enqueue(encode('a')); + controller.close(); + }, + }); + const reader = monitorStream( + source, + () => calls.push('onDone'), + () => calls.push('onError'), + ).getReader(); + void reader.closed.then(() => calls.push('closed')); + + await reader.read(); + await reader.read(); + await new Promise(resolve => setTimeout(resolve, 0)); + + expect(calls).toEqual(['onDone', 'closed']); + }); + + it('calls onDone in the same task as the cancel', async () => { + const onDone = vi.fn(); + const source = new ReadableStream({ + start(controller) { + controller.enqueue(encode('a')); + }, + }); + const reader = monitorStream(source, onDone, vi.fn()).getReader(); + await reader.read(); + + const cancelled = reader.cancel('client disconnected'); + expect(onDone).toHaveBeenCalledTimes(1); + + await cancelled; + await new Promise(resolve => setTimeout(resolve, 0)); + expect(onDone).toHaveBeenCalledTimes(1); + }); + + it('still delivers the body when onDone throws', async () => { + const source = new ReadableStream({ + start(controller) { + controller.enqueue(encode('ok')); + controller.close(); + }, + }); + const body = monitorStream( + source, + () => { + throw new Error('waitUntil failed'); + }, + vi.fn(), + ); + + expect(await new Response(body).text()).toBe('ok'); + }); }); diff --git a/packages/server-utils/test/utils/wrapServerRequestHandler.test.ts b/packages/server-utils/test/utils/wrapServerRequestHandler.test.ts index ff8cf8703fd1..f0bbad17ecea 100644 --- a/packages/server-utils/test/utils/wrapServerRequestHandler.test.ts +++ b/packages/server-utils/test/utils/wrapServerRequestHandler.test.ts @@ -220,4 +220,48 @@ describe('wrapServerRequestHandler', () => { const setCookie = transactions[0]?.contexts?.trace?.data?.['http.response.header.set-cookie'] as string[]; expect(setCookie.map(cookie => cookie.split('=')[0])).toEqual(['first', 'second']); }); + + it('calls onDone once with the response, or without one when the handler throws', async () => { + const done: (number | undefined)[] = []; + const { serve } = setup({}, { onDone: response => done.push(response?.status) }); + + const response = await serve(new Request('https://example.com/ok'), () => new Response('ok', { status: 201 })); + await response.text(); + await expect( + serve(new Request('https://example.com/throws'), () => { + throw new Error('handler failed'); + }), + ).rejects.toThrow('handler failed'); + await serve(new Request('https://example.com/cors', { method: 'OPTIONS' }), () => new Response(null)); + + expect(done).toEqual([201, undefined, 200]); + }); + + it('does not capture errors without a mechanism', async () => { + const { events, serve } = setup({}, { mechanism: undefined }); + + await expect( + serve(new Request('https://example.com/throws'), () => { + throw new Error('handler failed'); + }), + ).rejects.toThrow('handler failed'); + + expect(events).toHaveLength(0); + }); + + it('calls onDone at once when isStreamingResponse returns false', async () => { + const onDone = vi.fn(); + const { serve } = setup({}, { onDone, isStreamingResponse: () => false }); + const source = new ReadableStream({ + start() { + // never closes + }, + }); + const original = new Response(source, { headers: { 'content-type': 'text/event-stream' } }); + + const response = await serve(new Request('https://example.com/events'), () => original); + + expect(response).toBe(original); + expect(onDone).toHaveBeenCalledTimes(1); + }); }); From 45dbc4fce6fc8767fd49bd3ef91f2dfca747b6be Mon Sep 17 00:00:00 2001 From: isaacs Date: Fri, 23 Oct 2026 10:42:50 -0700 Subject: [PATCH 2/4] fix(cloudflare): Attach request data to the current scope without AsyncLocalStorage Without `AsyncLocalStorage` (for example on Shopify Oxygen, or on Workers without `nodejs_compat`), the SDK uses the stack fallback, and `withInvocationIsolationScope` does not fork. The scope it passes to its callback is a clone that is not the current isolation scope. The shared request handler wrote the request data and body to that clone, so an event captured in the handler, which reads the current scope, had no request body. The `hydrogen-react-router-7` E2E test caught this. The handler now gets the current isolation scope. With `AsyncLocalStorage`, that is the same scope as before. Without it, the request data goes where the body went before the move to the shared handler, and events now also get the request URL and method. Co-Authored-By: Claude Opus 5.5 (1M context) --- .size-limit.js | 1 - CHANGELOG.md | 2 + .../src/wrapRequestHandlerWithInit.ts | 7 ++- .../test/request-stack-strategy.test.ts | 45 +++++++++++++++++++ 4 files changed, 52 insertions(+), 3 deletions(-) create mode 100644 packages/cloudflare/test/request-stack-strategy.test.ts diff --git a/.size-limit.js b/.size-limit.js index 075c3fb89a6c..b841667d9d89 100644 --- a/.size-limit.js +++ b/.size-limit.js @@ -467,7 +467,6 @@ module.exports = [ ignore: [...builtinModules, ...nodePrefixedBuiltinModules], gzip: false, brotli: false, - limit: '521 KiB', disablePlugins: ['@size-limit/webpack'], webpack: false, modifyEsbuildConfig: function (config) { diff --git a/CHANGELOG.md b/CHANGELOG.md index b49e9282b484..8595e92811e0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,8 @@ - Request spans now set `client.address` from the `CF-Connecting-IP` header (or another forwarding header), as on the other runtimes. This follows `dataCollection.userInfo`, which is `true` by default. To leave it out, set `dataCollection: { userInfo: false }`. - Request spans now have the response header attributes, and events have the `response` context with the status code. - When the source of a streamed response fails, the SDK now captures the error and marks the span as failed. With `captureErrors: false`, it does not capture it. + - Request spans now list every `Set-Cookie` header of a response, as on Deno and Bun. + - Without `AsyncLocalStorage` (for example on Shopify Oxygen), events now also get the request URL and method. - "You miss 100 percent of the chances you don't take. — Wayne Gretzky" — Michael Scott diff --git a/packages/cloudflare/src/wrapRequestHandlerWithInit.ts b/packages/cloudflare/src/wrapRequestHandlerWithInit.ts index b10797a7023a..1f3ddde1dd6d 100644 --- a/packages/cloudflare/src/wrapRequestHandlerWithInit.ts +++ b/packages/cloudflare/src/wrapRequestHandlerWithInit.ts @@ -1,7 +1,7 @@ import type { CfProperties, IncomingRequestCfProperties } from '@cloudflare/workers-types'; import { NETWORK_PROTOCOL_NAME, NETWORK_PROTOCOL_VERSION } from '@sentry/conventions/attributes'; import type { SpanAttributes } from '@sentry/core'; -import { safeCallback } from '@sentry/core'; +import { getIsolationScope, safeCallback } from '@sentry/core'; import { wrapServerRequestHandler } from '@sentry/server-utils/request-handler'; import { DEBUG_BUILD } from './debug-build'; import { getHttpServerRequestOptions } from './integrations/httpServer'; @@ -99,7 +99,10 @@ export function wrapRequestHandlerWithInit( { request, client, - isolationScope, + // Without `AsyncLocalStorage`, `withInvocationIsolationScope` does not fork, and the scope it + // passes in is not the current one. The request data has to go on the current scope, where + // events captured in the handler read it. + isolationScope: getIsolationScope(), options: getHttpServerRequestOptions(client), origin: 'auto.http.cloudflare', mechanism: captureErrors ? { handled: false, type: getRequestErrorMechanismType(context) } : undefined, diff --git a/packages/cloudflare/test/request-stack-strategy.test.ts b/packages/cloudflare/test/request-stack-strategy.test.ts new file mode 100644 index 000000000000..3e3cda68c479 --- /dev/null +++ b/packages/cloudflare/test/request-stack-strategy.test.ts @@ -0,0 +1,45 @@ +// This file sets no async context strategy, so the SDK uses the stack fallback, as on runtimes +// without `AsyncLocalStorage` (for example Shopify Oxygen, or Workers without `nodejs_compat`). + +import type { Event } from '@sentry/core'; +import * as SentryCore from '@sentry/core'; +import { expect, test, vi } from 'vitest'; +import { httpServerIntegration, wrapRequestHandler } from '../src/request'; + +test('attaches the request data and body to events captured in the handler without AsyncLocalStorage', async () => { + let event: Event | undefined; + const body = JSON.stringify({ payload: 'x'.repeat(4_000) }); + + await wrapRequestHandler( + { + options: { + dsn: 'https://public@dsn.ingest.sentry.io/1337', + cacheClient: false, + integrations: [httpServerIntegration({ maxRequestBodySize: 'small' })], + beforeSend(captured) { + event = captured; + return null; + }, + }, + request: new Request('https://example.com/action', { + method: 'POST', + headers: { 'content-type': 'application/json' }, + body, + }), + context: { waitUntil: vi.fn(), passThroughOnException: vi.fn() } as never, + }, + () => { + SentryCore.captureException(new Error('Action Error')); + return new Response('error', { status: 500 }); + }, + ); + + expect(event?.request).toEqual( + expect.objectContaining({ + method: 'POST', + url: 'https://example.com/action', + // `maxRequestBodySize: 'small'` truncates at 1,000 bytes. + data: `${body.slice(0, 997)}...`, + }), + ); +}); From 9bedeb83fc6f062245d067f9140e8a55e144c433 Mon Sep 17 00:00:00 2001 From: isaacs Date: Thu, 29 Oct 2026 10:09:33 -0700 Subject: [PATCH 3/4] fix(server-utils): correct comment in no-diagnostics-channel export barrel --- packages/server-utils/src/exports.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/server-utils/src/exports.ts b/packages/server-utils/src/exports.ts index 15381f95dc9b..1548fcbd07cb 100644 --- a/packages/server-utils/src/exports.ts +++ b/packages/server-utils/src/exports.ts @@ -11,7 +11,7 @@ export type { PostgresConnectionContext } from './integrations/postgresjs'; // Shared, runtime-agnostic Hono instrumentation re-used by the `@sentry/hono` SDK across all runtimes. // Sourced directly from their modules (not the `./integrations/hono` barrel, which also exports the // diagnostics-channel-based `honoIntegration`) so the non-Node adapters can import them from the -// Node-free `@sentry/server-utils/no-diagnostic-channels` entry. +// diagnostics_channel-free `@sentry/server-utils/no-diagnostic-channels` entry. export { applyHonoPatches, earlyPatchHono } from './integrations/hono/applyPatches'; export { createHonoRequestMiddleware } from './integrations/hono/createHonoMiddleware'; export type { CreateHonoRequestMiddlewareOptions } from './integrations/hono/createHonoMiddleware'; From 881f9f55fa4b51c9fc4aa3225fe5f9de4acbdcde Mon Sep 17 00:00:00 2001 From: isaacs Date: Thu, 29 Oct 2026 10:11:52 -0700 Subject: [PATCH 4/4] fix(cloudflare): reword comment on getHttpServerRequestOptions --- packages/cloudflare/src/integrations/httpServer.ts | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/packages/cloudflare/src/integrations/httpServer.ts b/packages/cloudflare/src/integrations/httpServer.ts index dbcc4bb839b0..8f7afaacd29f 100644 --- a/packages/cloudflare/src/integrations/httpServer.ts +++ b/packages/cloudflare/src/integrations/httpServer.ts @@ -107,8 +107,9 @@ interface HttpServerIntegration extends Integration { } /** - * The incoming request options of the client's `httpServerIntegration`. Without the integration, - * the SDK attaches no request body, so users can turn body capture off by removing it. + * The incoming request options of the client's `httpServerIntegration`. If the integration does not + * set `maxRequestBodySize`, `dataCollection.httpBodies` decides. Without the integration, the SDK + * attaches no request body. */ export function getHttpServerRequestOptions(client: Client): ServerRequestOptions { return (