From 008b07e0cdc3fcccef965e2bce8671a91a6a5950 Mon Sep 17 00:00:00 2001 From: isaacs Date: Sun, 18 Oct 2026 15:55:47 -0700 Subject: [PATCH 1/4] feat(deno): Add incoming request options to `denoServeIntegration` `Deno.serve` is the server most Deno apps use, but its integration only took `maxRequestBodySize` and the deprecated `ignoreStatusCodes`. A user could not skip body capture for one route, such as a health check, an upload, or a route that receives secrets. `denoHttpIntegration`, Node and Cloudflare all allow this. `denoServeIntegration` now accepts `ignoreRequestBody`, `ignoreIncomingRequests`, `ignoreStaticAssets`, `spans`, `onSpanCreated` and `onSpanEnd`. They use the same names and behavior as `denoHttpIntegration`, but the callbacks receive the web `Request`. The options follow core's `node:http` server code. `urlPath` includes the query string, and the static asset filter applies only to `GET`. When the SDK skips a span, it still continues the incoming trace, attaches request data and the body, and captures errors. For a streamed response, `onSpanEnd` runs when the stream finishes. It does not run when the handler throws, because there is no `Response` to pass. Two defaults change behavior. `ignoreStaticAssets` defaults to `true`, so `GET` requests for files such as `favicon.ico` or `*.js` no longer create spans. `spans` defaults to the client's tracing configuration, so the integration no longer starts spans when tracing is disabled. Request sessions for `Deno.serve` and the same options for `Bun.serve` remain separate work. Fixes #25492 Fixes JS-3955 Co-Authored-By: Claude Opus 5.5 (1M context) --- CHANGELOG.md | 6 + packages/deno/src/integrations/deno-serve.ts | 52 ++- packages/deno/src/utils/streaming.ts | 25 +- .../deno/src/wrap-deno-request-handler.ts | 82 +++-- packages/deno/test/deno-serve.test.ts | 328 +++++++++++++++++- packages/deno/test/streaming.test.ts | 36 ++ 6 files changed, 493 insertions(+), 36 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index a25b5bccb34e..b6b8060bce4e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -21,6 +21,12 @@ Work in this release was contributed by @adityareddy-dev, @jpdefo, @Jxxunnn, @philipatkinson, @Shubham-Padkonde, @mikamikasuki, and @mketiku. Thank you for your contributions! +- **feat(deno): Add incoming request options to `denoServeIntegration`** + + `denoServeIntegration` now accepts `ignoreRequestBody`, `ignoreIncomingRequests`, `ignoreStaticAssets`, `spans`, `onSpanCreated` and `onSpanEnd`, with the same meaning as in `denoHttpIntegration`. The callbacks receive the web `Request`. + + `ignoreStaticAssets` defaults to `true`, so `Deno.serve` no longer creates spans for `GET` requests to common static assets such as `favicon.ico`, `robots.txt` or `*.js` files. To keep these spans, set `denoServeIntegration({ ignoreStaticAssets: false })`. `Deno.serve` spans also follow the client's tracing configuration by default, so the integration does not start spans when tracing is disabled. + ## 11.8.0 ### Important Changes diff --git a/packages/deno/src/integrations/deno-serve.ts b/packages/deno/src/integrations/deno-serve.ts index 156fb9fd8559..16d061cbd1f7 100644 --- a/packages/deno/src/integrations/deno-serve.ts +++ b/packages/deno/src/integrations/deno-serve.ts @@ -1,4 +1,4 @@ -import type { Event, IntegrationFn, MaxRequestBodySize } from '@sentry/core'; +import type { Event, IntegrationFn, MaxRequestBodySize, Span } from '@sentry/core'; import { debug, defineIntegration } from '@sentry/core'; import { DEFAULT_IGNORE_STATUS_CODES, processHttpServerTransactionEvent } from '@sentry/core/server'; import type { RequestHandlerWrapperOptions } from '../wrap-deno-request-handler'; @@ -17,6 +17,50 @@ export type DenoServeIntegrationOptions = { */ maxRequestBodySize?: MaxRequestBodySize; + /** + * Do not capture the request body for incoming requests where the given callback returns `true`. + * + * `url` is the full URL of the request, including the query string. `request` is the incoming + * {@link Request}. The callback runs before the SDK reads the body. + */ + ignoreRequestBody?: (url: string, request: Request) => boolean; + + /** + * Do not capture server spans for incoming requests whose URL path makes the given callback return `true`. + * + * `urlPath` is the path of the request, including the query string. `request` is the incoming + * {@link Request}. Errors thrown by the handler are still captured. + */ + ignoreIncomingRequests?: (urlPath: string, request: Request) => boolean; + + /** + * Whether to automatically ignore common static asset requests (favicon.ico, robots.txt, etc.) + * when creating server spans. + * + * @default `true` + */ + ignoreStaticAssets?: boolean; + + /** + * Whether to create spans for incoming requests. + * Errors are still captured and request data is still attached when this is `false`. + * Defaults to the client's tracing configuration (`hasSpansEnabled`). + */ + spans?: boolean; + + /** + * A hook that can be used to mutate the span for incoming requests. + * This is triggered after the span is created, before the request handler runs. + */ + onSpanCreated?: (span: Span, request: Request) => void; + + /** + * A hook that can be used to mutate the span one last time before it ends. + * For a streamed response, this runs when the stream finishes. + * It is not called when the request handler throws. + */ + onSpanEnd?: (span: Span, request: Request, response: Response) => void; + /** * Do not capture spans for incoming `Deno.serve` requests with the given status codes. * By default, some 3xx and 4xx status codes are dropped (see @default). @@ -99,6 +143,12 @@ const _denoServeIntegration = ((options: DenoServeIntegrationOptions = {}) => { 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, processEvent(event: Event): Event | null { // Gated on this integration's own span origin so it does not filter `node:http` // transactions, which `denoHttpIntegration` owns via its own `ignoreStatusCodes`. diff --git a/packages/deno/src/utils/streaming.ts b/packages/deno/src/utils/streaming.ts index ca6f778b749e..276d2455af3a 100644 --- a/packages/deno/src/utils/streaming.ts +++ b/packages/deno/src/utils/streaming.ts @@ -5,29 +5,32 @@ import { classifyResponseStreaming } from '@sentry/core/server'; * Tee a stream, and end the provided span when the stream ends. * Returns the other side of the tee, which can be used to send the * response to a client. + * + * `onEnd` runs right before the span ends. */ -export async function streamResponse(span: Span, res: Response): Promise { +export async function streamResponse(span: Span, res: Response, onEnd?: () => void): Promise { const classification = classifyResponseStreaming(res); + const endSpan = (): void => { + onEnd?.(); + span.end(); + }; // not streaming, just end the span and return the response if (!classification.isStreaming || !res.body) { - span.end(); + endSpan(); return res; } // Streaming response detected - monitor consumption to keep span alive try { - return new Response( - monitorStream(res.body, () => span.end()), - { - status: res.status, - statusText: res.statusText, - headers: res.headers, - }, - ); + return new Response(monitorStream(res.body, endSpan), { + status: res.status, + statusText: res.statusText, + headers: res.headers, + }); } catch { // tee() failed - handle without streaming - span.end(); + endSpan(); return res; } } diff --git a/packages/deno/src/wrap-deno-request-handler.ts b/packages/deno/src/wrap-deno-request-handler.ts index 1ab390d3790b..3b195f4ac140 100644 --- a/packages/deno/src/wrap-deno-request-handler.ts +++ b/packages/deno/src/wrap-deno-request-handler.ts @@ -6,7 +6,7 @@ import { SENTRY_SEGMENT_NAME_SOURCE, } from '@sentry/conventions/attributes'; import { HTTP_SERVER } from '@sentry/conventions/op'; -import type { Integration, MaxRequestBodySize } from '@sentry/core'; +import type { Client, Integration } from '@sentry/core'; import { captureBodyFromWinterCGRequest, captureException, @@ -14,6 +14,7 @@ import { debug, getClient, getHttpSpanDetailsFromUrlObject, + hasSpansEnabled, hasSpanStreamingEnabled, httpHeadersToSpanAttributes, HTTP_SPAN_NAME_FALLBACK, @@ -24,7 +25,8 @@ import { winterCGRequestToRequestData, withIsolationScope, } from '@sentry/core'; -import { getClientIPAddress } from '@sentry/core/server'; +import { getClientIPAddress, isStaticAssetRequest } from '@sentry/core/server'; +import type { DenoServeIntegrationOptions } from './integrations/deno-serve'; import { streamResponse } from './utils/streaming'; export type RequestHandlerWrapperOptions = { @@ -33,6 +35,31 @@ export type RequestHandlerWrapperOptions = { serveOptions?: Deno.ServeOptions; }; +function captureServeError(e: unknown): void { + captureException(e, { + mechanism: { + handled: false, + type: 'auto.http.deno', + data: { function: 'serve' }, + }, + }); +} + +function shouldCreateSpan( + client: Client, + integration: DenoServeIntegrationOptions | undefined, + request: Request, + urlPath: string, +): boolean { + if (!(integration?.spans ?? hasSpansEnabled(client.getOptions()))) { + return false; + } + if (integration?.ignoreStaticAssets && request.method === 'GET' && isStaticAssetRequest(urlPath)) { + return false; + } + return !integration?.ignoreIncomingRequests?.(urlPath, request); +} + const assignIfSet = , K extends keyof T>( obj: T, key: K, @@ -63,17 +90,13 @@ export const wrapDenoRequestHandler = ( try { return await handler(); } catch (e) { - captureException(e, { - mechanism: { - handled: false, - type: 'auto.http.deno', - data: { function: 'serve' }, - }, - }); + captureServeError(e); throw e; } } + const integration = client.getIntegrationByName('DenoServe'); + const urlObject = parseStringToURLObject(request.url); const [rawName, attributes] = getHttpSpanDetailsFromUrlObject( urlObject, @@ -122,22 +145,40 @@ export const wrapDenoRequestHandler = ( ipAddress: socketIp, }); - const configuredBodySize = client.getIntegrationByName( - 'DenoServe', - )?.maxRequestBodySize; const effectiveBodySize = - configuredBodySize ?? (dataCollection.httpBodies.includes('incomingRequest') ? 'medium' : 'none'); - if (request.method !== 'GET' && effectiveBodySize !== 'none') { + integration?.maxRequestBodySize ?? (dataCollection.httpBodies.includes('incomingRequest') ? 'medium' : 'none'); + if ( + request.method !== 'GET' && + effectiveBodySize !== 'none' && + !integration?.ignoreRequestBody?.(request.url, request) + ) { await captureBodyFromWinterCGRequest(request, isolationScope, effectiveBodySize); } + const urlPath = urlObject ? `${urlObject.pathname}${urlObject.search}` : '/'; + const createSpan = shouldCreateSpan(client, integration, request, urlPath); + return continueTrace( { sentryTrace: request.headers.get('sentry-trace') || '', baggage: request.headers.get('baggage'), }, - () => { + async () => { + if (!createSpan) { + try { + const res = await handler(); + isolationScope.setContext('response', { + status_code: res.status, + }); + return res; + } catch (e) { + captureServeError(e); + throw e; + } + } + return startSpanManual({ name, attributes }, async span => { + integration?.onSpanCreated?.(span, request); let res: Response; try { @@ -151,17 +192,12 @@ export const wrapDenoRequestHandler = ( ); } catch (e) { span.end(); - captureException(e, { - mechanism: { - handled: false, - type: 'auto.http.deno', - data: { function: 'serve' }, - }, - }); + captureServeError(e); throw e; } - return streamResponse(span, res); + const onSpanEnd = integration?.onSpanEnd; + return streamResponse(span, res, onSpanEnd && (() => onSpanEnd(span, request, res))); }); }, ); diff --git a/packages/deno/test/deno-serve.test.ts b/packages/deno/test/deno-serve.test.ts index f85e596e631a..33e423f0f15d 100644 --- a/packages/deno/test/deno-serve.test.ts +++ b/packages/deno/test/deno-serve.test.ts @@ -1,6 +1,6 @@ // -import type { ErrorEvent, TransactionEvent } from '@sentry/core'; +import type { ErrorEvent, Span, TransactionEvent } from '@sentry/core'; import { getMainCarrier } from '@sentry/core'; import { assertEquals, assertExists, assertMatch, assertNotEquals } from 'https://deno.land/std@0.212.0/assert/mod.ts'; import type { DenoClient } from '../build/esm/index.js'; @@ -919,3 +919,329 @@ Deno.test('Deno.serve should work when manually capturing exceptions within hand // (unless explicitly set otherwise) assertEquals(errorEvent?.exception?.values?.[0]?.mechanism?.handled, true); }); + +async function serveForTest( + handler: (request: Request) => Response | Promise, +): Promise<{ url: string; close: () => Promise }> { + const abortController = new AbortController(); + let onListen: ((_: unknown) => void) | undefined = undefined; + const p = new Promise(resolve => (onListen = resolve)); + const server = Deno.serve( + { + port: 0, + signal: abortController.signal, + onListen, + onError: error => new Response((error as Error).message, { status: 500 }), + }, + handler, + ); + await p; + return { + url: `http://localhost:${server.addr.port}`, + close: async () => { + abortController.abort(); + await server.finished; + }, + }; +} + +Deno.test('Deno.serve should not capture request bodies that ignoreRequestBody matches', async () => { + resetGlobals(); + const transactionEvents: TransactionEvent[] = []; + const ignoreRequestBodyCalls: [string, string][] = []; + + init({ + dsn: 'https://username@domain/123', + tracesSampleRate: 1, + traceLifecycle: 'static', + integrations: integrations => [ + ...integrations.filter(integration => integration.name !== 'DenoServe'), + denoServeIntegration({ + ignoreRequestBody: (url, request) => { + ignoreRequestBodyCalls.push([url, request.method]); + return url.includes('/upload'); + }, + }), + ], + beforeSendTransaction: (event: TransactionEvent) => { + transactionEvents.push(event); + return null; + }, + }) as DenoClient; + + const handlerBodies: string[] = []; + const server = await serveForTest(async request => { + handlerBodies.push(await request.text()); + return new Response('OK'); + }); + + await (await fetch(`${server.url}/upload?id=1`, { method: 'POST', body: 'secret' })).text(); + await (await fetch(`${server.url}/other`, { method: 'POST', body: 'public' })).text(); + await server.close(); + + assertEquals(handlerBodies, ['secret', 'public']); + assertEquals(ignoreRequestBodyCalls, [ + [`${server.url}/upload?id=1`, 'POST'], + [`${server.url}/other`, 'POST'], + ]); + assertEquals(transactionEvents.length, 2); + const upload = transactionEvents.find(event => event.request?.url?.includes('/upload')); + const other = transactionEvents.find(event => event.request?.url?.includes('/other')); + assertEquals(upload?.request?.data, undefined); + assertEquals(other?.request?.data, 'public'); +}); + +Deno.test('Deno.serve should not create spans for requests that ignoreIncomingRequests matches', async () => { + resetGlobals(); + const transactionEvents: TransactionEvent[] = []; + const errorEvents: ErrorEvent[] = []; + const urlPaths: string[] = []; + + init({ + dsn: 'https://username@domain/123', + tracesSampleRate: 1, + traceLifecycle: 'static', + integrations: integrations => [ + ...integrations.filter(integration => integration.name !== 'DenoServe'), + denoServeIntegration({ + ignoreIncomingRequests: (urlPath, request) => { + urlPaths.push(urlPath); + return request.method === 'GET' && urlPath.startsWith('/health'); + }, + }), + ], + beforeSend: (event: ErrorEvent) => { + errorEvents.push(event); + return null; + }, + beforeSendTransaction: (event: TransactionEvent) => { + transactionEvents.push(event); + return null; + }, + }) as DenoClient; + + const server = await serveForTest(request => { + if (request.url.includes('fail')) { + throw new Error('health check failed'); + } + return new Response('OK'); + }); + + await (await fetch(`${server.url}/health?ok=1`)).text(); + await (await fetch(`${server.url}/health/fail`)).text(); + await (await fetch(`${server.url}/users`)).text(); + await server.close(); + await delay(100); + + assertEquals(urlPaths, ['/health?ok=1', '/health/fail', '/users']); + assertEquals( + transactionEvents.map(event => event.transaction), + ['GET /users'], + ); + assertEquals(errorEvents.length, 1); + assertEquals(errorEvents[0]?.exception?.values?.[0]?.value, 'health check failed'); + assertEquals(errorEvents[0]?.exception?.values?.[0]?.mechanism?.type, 'auto.http.deno'); + assertEquals(errorEvents[0]?.request?.url?.includes('/health/fail'), true); +}); + +Deno.test('Deno.serve should not create spans for static assets by default', async () => { + resetGlobals(); + const transactionEvents: TransactionEvent[] = []; + + init({ + dsn: 'https://username@domain/123', + tracesSampleRate: 1, + traceLifecycle: 'static', + beforeSendTransaction: (event: TransactionEvent) => { + transactionEvents.push(event); + return null; + }, + }) as DenoClient; + + const server = await serveForTest(() => new Response('OK')); + await (await fetch(`${server.url}/favicon.ico`)).text(); + await (await fetch(`${server.url}/robots.txt`)).text(); + await (await fetch(`${server.url}/assets/app.js?v=2`)).text(); + await (await fetch(`${server.url}/assets/app.js`, { method: 'POST' })).text(); + await (await fetch(`${server.url}/page`)).text(); + await server.close(); + + assertEquals(transactionEvents.map(event => event.transaction).sort(), ['GET /page', 'POST /assets/app.js']); +}); + +Deno.test('Deno.serve should create spans for static assets when ignoreStaticAssets is false', async () => { + resetGlobals(); + const transactionEvents: TransactionEvent[] = []; + + init({ + dsn: 'https://username@domain/123', + tracesSampleRate: 1, + traceLifecycle: 'static', + integrations: integrations => [ + ...integrations.filter(integration => integration.name !== 'DenoServe'), + denoServeIntegration({ ignoreStaticAssets: false }), + ], + beforeSendTransaction: (event: TransactionEvent) => { + transactionEvents.push(event); + return null; + }, + }) as DenoClient; + + const server = await serveForTest(() => new Response('OK')); + await (await fetch(`${server.url}/favicon.ico`)).text(); + await server.close(); + + assertEquals( + transactionEvents.map(event => event.transaction), + ['GET /favicon.ico'], + ); +}); + +Deno.test('Deno.serve should not create spans when spans is false, but still capture errors', async () => { + resetGlobals(); + const transactionEvents: TransactionEvent[] = []; + const errorEvents: ErrorEvent[] = []; + + init({ + dsn: 'https://username@domain/123', + tracesSampleRate: 1, + traceLifecycle: 'static', + integrations: integrations => [ + ...integrations.filter(integration => integration.name !== 'DenoServe'), + denoServeIntegration({ spans: false }), + ], + beforeSend: (event: ErrorEvent) => { + errorEvents.push(event); + return null; + }, + beforeSendTransaction: (event: TransactionEvent) => { + transactionEvents.push(event); + return null; + }, + }) as DenoClient; + + const server = await serveForTest(async request => { + if (request.url.includes('error')) { + throw new Error(`failed with ${await request.text()}`); + } + return new Response('OK'); + }); + + await (await fetch(`${server.url}/ok`)).text(); + await (await fetch(`${server.url}/error`, { method: 'POST', body: 'payload' })).text(); + await server.close(); + await delay(100); + + assertEquals(transactionEvents.length, 0); + assertEquals(errorEvents.length, 1); + assertEquals(errorEvents[0]?.exception?.values?.[0]?.value, 'failed with payload'); + assertEquals(errorEvents[0]?.exception?.values?.[0]?.mechanism?.handled, false); + assertEquals(errorEvents[0]?.request?.url?.includes('/error'), true); + assertEquals(errorEvents[0]?.request?.data, 'payload'); +}); + +Deno.test('Deno.serve should not create spans when tracing is disabled', async () => { + resetGlobals(); + let spanCreatedCalls = 0; + + init({ + dsn: 'https://username@domain/123', + integrations: integrations => [ + ...integrations.filter(integration => integration.name !== 'DenoServe'), + denoServeIntegration({ + onSpanCreated: () => { + spanCreatedCalls++; + }, + }), + ], + }) as DenoClient; + + const server = await serveForTest(() => new Response('OK')); + await (await fetch(`${server.url}/page`)).text(); + await server.close(); + + assertEquals(spanCreatedCalls, 0); +}); + +Deno.test('Deno.serve should call onSpanCreated and onSpanEnd', async () => { + resetGlobals(); + const transactionEvents: TransactionEvent[] = []; + const hookCalls: string[] = []; + + init({ + dsn: 'https://username@domain/123', + tracesSampleRate: 1, + traceLifecycle: 'static', + integrations: integrations => [ + ...integrations.filter(integration => integration.name !== 'DenoServe'), + denoServeIntegration({ + onSpanCreated: (span: Span, request: Request) => { + hookCalls.push(`created ${new URL(request.url).pathname}`); + span.setAttribute('test.created', true); + }, + onSpanEnd: (span: Span, request: Request, response: Response) => { + hookCalls.push(`end ${new URL(request.url).pathname} ${response.status}`); + span.setAttribute('test.response_header', response.headers.get('x-test') ?? undefined); + }, + }), + ], + beforeSendTransaction: (event: TransactionEvent) => { + transactionEvents.push(event); + return null; + }, + }) as DenoClient; + + const server = await serveForTest(() => { + hookCalls.push('handler'); + return new Response('OK', { status: 201, headers: { 'x-test': 'yes' } }); + }); + await (await fetch(`${server.url}/hooks`)).text(); + await server.close(); + + assertEquals(hookCalls, ['created /hooks', 'handler', 'end /hooks 201']); + assertEquals(transactionEvents.length, 1); + assertEquals(transactionEvents[0]?.contexts?.trace?.data?.['test.created'], true); + assertEquals(transactionEvents[0]?.contexts?.trace?.data?.['test.response_header'], 'yes'); +}); + +Deno.test('Deno.serve should call onSpanEnd when a streamed response finishes', async () => { + resetGlobals(); + const transactionEvents: TransactionEvent[] = []; + const hookCalls: string[] = []; + + init({ + dsn: 'https://username@domain/123', + tracesSampleRate: 1, + traceLifecycle: 'static', + integrations: integrations => [ + ...integrations.filter(integration => integration.name !== 'DenoServe'), + denoServeIntegration({ + onSpanEnd: (span: Span) => { + hookCalls.push('end'); + span.setAttribute('test.ended', true); + }, + }), + ], + beforeSendTransaction: (event: TransactionEvent) => { + transactionEvents.push(event); + return null; + }, + }) as DenoClient; + + const server = await serveForTest(() => { + const stream = new ReadableStream({ + async pull(controller) { + hookCalls.push('chunk'); + controller.enqueue(new TextEncoder().encode('chunk\n')); + controller.close(); + }, + }); + return new Response(stream, { headers: { 'content-type': 'text/event-stream' } }); + }); + assertEquals(await (await fetch(`${server.url}/stream`)).text(), 'chunk\n'); + await server.close(); + + assertEquals(hookCalls, ['chunk', 'end']); + assertEquals(transactionEvents.length, 1); + assertEquals(transactionEvents[0]?.contexts?.trace?.data?.['test.ended'], true); +}); diff --git a/packages/deno/test/streaming.test.ts b/packages/deno/test/streaming.test.ts index cfa040ac01cf..1b13317d553f 100644 --- a/packages/deno/test/streaming.test.ts +++ b/packages/deno/test/streaming.test.ts @@ -82,3 +82,39 @@ Deno.test('reader.closed.then(f, f) suppresses rejection when releaseLock is cal globalThis.removeEventListener('unhandledrejection', handler); } }); + +Deno.test('calls onEnd before ending the span for a non-streamed response', async () => { + const calls: string[] = []; + const span = { end: () => calls.push('end') } as unknown as Parameters[0]; + + await streamResponse(span, new Response('{}', { headers: { 'content-type': 'application/json' } }), () => + calls.push('onEnd'), + ); + + assertEquals(calls, ['onEnd', 'end']); +}); + +Deno.test('calls onEnd before ending the span when a streamed response finishes', async () => { + const calls: string[] = []; + const span = { end: () => calls.push('end') } as unknown as Parameters[0]; + + let sourceController: ReadableStreamDefaultController | undefined; + const source = new ReadableStream({ + start(controller) { + sourceController = controller; + controller.enqueue(new TextEncoder().encode('chunk')); + }, + }); + const response = await streamResponse( + span, + new Response(source, { headers: { 'content-type': 'text/event-stream' } }), + () => calls.push('onEnd'), + ); + + await new Promise(resolve => setTimeout(resolve, 0)); + assertEquals(calls, []); + sourceController!.close(); + assertEquals(await response.text(), 'chunk'); + await new Promise(resolve => setTimeout(resolve, 0)); + assertEquals(calls, ['onEnd', 'end']); +}); From b78bdc58c07aab8e78193e8bd89b01c3f20e7ba9 Mon Sep 17 00:00:00 2001 From: isaacs Date: Sun, 18 Oct 2026 19:11:41 -0700 Subject: [PATCH 2/4] fix: bugbot comments --- .../deno/src/wrap-deno-request-handler.ts | 21 ++++- packages/deno/test/deno-serve.test.ts | 93 ++++++++++++++++++- 2 files changed, 107 insertions(+), 7 deletions(-) diff --git a/packages/deno/src/wrap-deno-request-handler.ts b/packages/deno/src/wrap-deno-request-handler.ts index 3b195f4ac140..7e8108c736df 100644 --- a/packages/deno/src/wrap-deno-request-handler.ts +++ b/packages/deno/src/wrap-deno-request-handler.ts @@ -45,6 +45,15 @@ function captureServeError(e: unknown): void { }); } +// A user hook that throws must not fail the request or leave the span open. +function callHook(name: string, hook: () => void): void { + try { + hook(); + } catch (e) { + debug.warn(`Error in the Deno.serve ${name} hook.`, e); + } +} + function shouldCreateSpan( client: Client, integration: DenoServeIntegrationOptions | undefined, @@ -178,7 +187,10 @@ export const wrapDenoRequestHandler = ( } return startSpanManual({ name, attributes }, async span => { - integration?.onSpanCreated?.(span, request); + const { onSpanCreated, onSpanEnd } = integration ?? {}; + if (onSpanCreated) { + callHook('onSpanCreated', () => onSpanCreated(span, request)); + } let res: Response; try { @@ -196,8 +208,11 @@ export const wrapDenoRequestHandler = ( throw e; } - const onSpanEnd = integration?.onSpanEnd; - return streamResponse(span, res, onSpanEnd && (() => onSpanEnd(span, request, res))); + return streamResponse( + span, + res, + onSpanEnd && (() => callHook('onSpanEnd', () => onSpanEnd(span, request, res))), + ); }); }, ); diff --git a/packages/deno/test/deno-serve.test.ts b/packages/deno/test/deno-serve.test.ts index 33e423f0f15d..9ad57075013c 100644 --- a/packages/deno/test/deno-serve.test.ts +++ b/packages/deno/test/deno-serve.test.ts @@ -997,7 +997,7 @@ Deno.test('Deno.serve should not create spans for requests that ignoreIncomingRe const errorEvents: ErrorEvent[] = []; const urlPaths: string[] = []; - init({ + const client = init({ dsn: 'https://username@domain/123', tracesSampleRate: 1, traceLifecycle: 'static', @@ -1031,7 +1031,7 @@ Deno.test('Deno.serve should not create spans for requests that ignoreIncomingRe await (await fetch(`${server.url}/health/fail`)).text(); await (await fetch(`${server.url}/users`)).text(); await server.close(); - await delay(100); + await client.flush(); assertEquals(urlPaths, ['/health?ok=1', '/health/fail', '/users']); assertEquals( @@ -1102,7 +1102,7 @@ Deno.test('Deno.serve should not create spans when spans is false, but still cap const transactionEvents: TransactionEvent[] = []; const errorEvents: ErrorEvent[] = []; - init({ + const client = init({ dsn: 'https://username@domain/123', tracesSampleRate: 1, traceLifecycle: 'static', @@ -1130,7 +1130,7 @@ Deno.test('Deno.serve should not create spans when spans is false, but still cap await (await fetch(`${server.url}/ok`)).text(); await (await fetch(`${server.url}/error`, { method: 'POST', body: 'payload' })).text(); await server.close(); - await delay(100); + await client.flush(); assertEquals(transactionEvents.length, 0); assertEquals(errorEvents.length, 1); @@ -1245,3 +1245,88 @@ Deno.test('Deno.serve should call onSpanEnd when a streamed response finishes', assertEquals(transactionEvents.length, 1); assertEquals(transactionEvents[0]?.contexts?.trace?.data?.['test.ended'], true); }); + +Deno.test('Deno.serve should still respond and end the span when onSpanCreated throws', async () => { + resetGlobals(); + const transactionEvents: TransactionEvent[] = []; + + init({ + dsn: 'https://username@domain/123', + tracesSampleRate: 1, + traceLifecycle: 'static', + integrations: integrations => [ + ...integrations.filter(integration => integration.name !== 'DenoServe'), + denoServeIntegration({ + onSpanCreated: () => { + throw new Error('onSpanCreated failed'); + }, + }), + ], + beforeSendTransaction: (event: TransactionEvent) => { + transactionEvents.push(event); + return null; + }, + }) as DenoClient; + + const server = await serveForTest(() => new Response('OK')); + const response = await fetch(`${server.url}/created`); + assertEquals(response.status, 200); + assertEquals(await response.text(), 'OK'); + await server.close(); + + assertEquals( + transactionEvents.map(event => event.transaction), + ['GET /created'], + ); +}); + +for (const [label, makeResponse] of [ + ['a response', () => new Response('OK')], + [ + 'a streamed response', + () => + new Response( + new ReadableStream({ + start(controller) { + controller.enqueue(new TextEncoder().encode('OK')); + controller.close(); + }, + }), + { headers: { 'content-type': 'text/event-stream' } }, + ), + ], +] as const) { + Deno.test(`Deno.serve should still respond and end the span when onSpanEnd throws for ${label}`, async () => { + resetGlobals(); + const transactionEvents: TransactionEvent[] = []; + + init({ + dsn: 'https://username@domain/123', + tracesSampleRate: 1, + traceLifecycle: 'static', + integrations: integrations => [ + ...integrations.filter(integration => integration.name !== 'DenoServe'), + denoServeIntegration({ + onSpanEnd: () => { + throw new Error('onSpanEnd failed'); + }, + }), + ], + beforeSendTransaction: (event: TransactionEvent) => { + transactionEvents.push(event); + return null; + }, + }) as DenoClient; + + const server = await serveForTest(makeResponse); + const response = await fetch(`${server.url}/ended`); + assertEquals(response.status, 200); + assertEquals(await response.text(), 'OK'); + await server.close(); + + assertEquals( + transactionEvents.map(event => event.transaction), + ['GET /ended'], + ); + }); +} From cbb3ec42cac02730c1712b584e31b966d2675045 Mon Sep 17 00:00:00 2001 From: isaacs Date: Mon, 19 Oct 2026 11:47:45 -0700 Subject: [PATCH 3/4] fix: bugbot --- packages/deno/test/streaming.test.ts | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/packages/deno/test/streaming.test.ts b/packages/deno/test/streaming.test.ts index 1b13317d553f..05951459e786 100644 --- a/packages/deno/test/streaming.test.ts +++ b/packages/deno/test/streaming.test.ts @@ -96,7 +96,14 @@ Deno.test('calls onEnd before ending the span for a non-streamed response', asyn Deno.test('calls onEnd before ending the span when a streamed response finishes', async () => { const calls: string[] = []; - const span = { end: () => calls.push('end') } as unknown as Parameters[0]; + let resolveEnded: () => void; + const ended = new Promise(resolve => (resolveEnded = resolve)); + const span = { + end: () => { + calls.push('end'); + resolveEnded(); + }, + } as unknown as Parameters[0]; let sourceController: ReadableStreamDefaultController | undefined; const source = new ReadableStream({ @@ -115,6 +122,6 @@ Deno.test('calls onEnd before ending the span when a streamed response finishes' assertEquals(calls, []); sourceController!.close(); assertEquals(await response.text(), 'chunk'); - await new Promise(resolve => setTimeout(resolve, 0)); + await ended; assertEquals(calls, ['onEnd', 'end']); }); From 9b230da8e76c708eebbb6e1a04aeada93ecec6a0 Mon Sep 17 00:00:00 2001 From: isaacs Date: Mon, 19 Oct 2026 12:04:40 -0700 Subject: [PATCH 4/4] fix: bugbot --- .../deno/src/wrap-deno-request-handler.ts | 23 ++++-- packages/deno/test/deno-serve.test.ts | 80 +++++++++++++++++++ 2 files changed, 95 insertions(+), 8 deletions(-) diff --git a/packages/deno/src/wrap-deno-request-handler.ts b/packages/deno/src/wrap-deno-request-handler.ts index 7e8108c736df..465adeb1ed44 100644 --- a/packages/deno/src/wrap-deno-request-handler.ts +++ b/packages/deno/src/wrap-deno-request-handler.ts @@ -45,12 +45,13 @@ function captureServeError(e: unknown): void { }); } -// A user hook that throws must not fail the request or leave the span open. -function callHook(name: string, hook: () => void): void { +// A user callback that throws must not fail the request or leave the span open. +function callUserCallback(name: string, callback: () => T, fallback: T): T { try { - hook(); + return callback(); } catch (e) { - debug.warn(`Error in the Deno.serve ${name} hook.`, e); + debug.error(`Error in the Deno.serve ${name} callback.`, e); + return fallback; } } @@ -66,7 +67,11 @@ function shouldCreateSpan( if (integration?.ignoreStaticAssets && request.method === 'GET' && isStaticAssetRequest(urlPath)) { return false; } - return !integration?.ignoreIncomingRequests?.(urlPath, request); + const ignoreIncomingRequests = integration?.ignoreIncomingRequests; + return !( + ignoreIncomingRequests && + callUserCallback('ignoreIncomingRequests', () => ignoreIncomingRequests(urlPath, request), false) + ); } const assignIfSet = , K extends keyof T>( @@ -156,10 +161,12 @@ export const wrapDenoRequestHandler = ( const effectiveBodySize = integration?.maxRequestBodySize ?? (dataCollection.httpBodies.includes('incomingRequest') ? 'medium' : 'none'); + const ignoreRequestBody = integration?.ignoreRequestBody; if ( request.method !== 'GET' && effectiveBodySize !== 'none' && - !integration?.ignoreRequestBody?.(request.url, request) + // If the filter throws, skip the body so it cannot leak data the filter was meant to hide. + !(ignoreRequestBody && callUserCallback('ignoreRequestBody', () => ignoreRequestBody(request.url, request), true)) ) { await captureBodyFromWinterCGRequest(request, isolationScope, effectiveBodySize); } @@ -189,7 +196,7 @@ export const wrapDenoRequestHandler = ( return startSpanManual({ name, attributes }, async span => { const { onSpanCreated, onSpanEnd } = integration ?? {}; if (onSpanCreated) { - callHook('onSpanCreated', () => onSpanCreated(span, request)); + callUserCallback('onSpanCreated', () => onSpanCreated(span, request), undefined); } let res: Response; @@ -211,7 +218,7 @@ export const wrapDenoRequestHandler = ( return streamResponse( span, res, - onSpanEnd && (() => callHook('onSpanEnd', () => onSpanEnd(span, request, res))), + onSpanEnd && (() => callUserCallback('onSpanEnd', () => onSpanEnd(span, request, res), undefined)), ); }); }, diff --git a/packages/deno/test/deno-serve.test.ts b/packages/deno/test/deno-serve.test.ts index 9ad57075013c..f30318f0fbcd 100644 --- a/packages/deno/test/deno-serve.test.ts +++ b/packages/deno/test/deno-serve.test.ts @@ -1330,3 +1330,83 @@ for (const [label, makeResponse] of [ ); }); } + +Deno.test('Deno.serve should not capture the body and still respond when ignoreRequestBody throws', async () => { + resetGlobals(); + const transactionEvents: TransactionEvent[] = []; + const errorEvents: ErrorEvent[] = []; + + const client = init({ + dsn: 'https://username@domain/123', + tracesSampleRate: 1, + traceLifecycle: 'static', + integrations: integrations => [ + ...integrations.filter(integration => integration.name !== 'DenoServe'), + denoServeIntegration({ + ignoreRequestBody: () => { + throw new Error('ignoreRequestBody failed'); + }, + }), + ], + beforeSend: (event: ErrorEvent) => { + errorEvents.push(event); + return null; + }, + beforeSendTransaction: (event: TransactionEvent) => { + transactionEvents.push(event); + return null; + }, + }) as DenoClient; + + const server = await serveForTest(async request => new Response(await request.text())); + const response = await fetch(`${server.url}/upload`, { method: 'POST', body: 'secret' }); + assertEquals(response.status, 200); + assertEquals(await response.text(), 'secret'); + await server.close(); + await client.flush(); + + assertEquals(errorEvents.length, 0); + assertEquals(transactionEvents.length, 1); + assertEquals(transactionEvents[0]?.request?.data, undefined); +}); + +Deno.test('Deno.serve should create the span and still respond when ignoreIncomingRequests throws', async () => { + resetGlobals(); + const transactionEvents: TransactionEvent[] = []; + const errorEvents: ErrorEvent[] = []; + + const client = init({ + dsn: 'https://username@domain/123', + tracesSampleRate: 1, + traceLifecycle: 'static', + integrations: integrations => [ + ...integrations.filter(integration => integration.name !== 'DenoServe'), + denoServeIntegration({ + ignoreIncomingRequests: () => { + throw new Error('ignoreIncomingRequests failed'); + }, + }), + ], + beforeSend: (event: ErrorEvent) => { + errorEvents.push(event); + return null; + }, + beforeSendTransaction: (event: TransactionEvent) => { + transactionEvents.push(event); + return null; + }, + }) as DenoClient; + + const server = await serveForTest(() => new Response('OK')); + const response = await fetch(`${server.url}/page`); + assertEquals(response.status, 200); + assertEquals(await response.text(), 'OK'); + await server.close(); + await client.flush(); + + assertEquals(errorEvents.length, 0); + assertEquals( + transactionEvents.map(event => event.transaction), + ['GET /page'], + ); +});