fix: bugbot

Author: isaacs

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<T>(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 = <T extends Record<string, unknown>, K extends keyof T>(
@@ -156,10 +161,12 @@ export const wrapDenoRequestHandler = <Addr extends Deno.Addr = Deno.Addr>(
 
     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 = <Addr extends Deno.Addr = Deno.Addr>(
         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 = <Addr extends Deno.Addr = Deno.Addr>(
           return streamResponse(
             span,
             res,
-            onSpanEnd && (() => callHook('onSpanEnd', () => onSpanEnd(span, request, res))),
+            onSpanEnd && (() => callUserCallback('onSpanEnd', () => onSpanEnd(span, request, res), undefined)),
           );
         });
       },

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'],
+  );
+});