review suggestions

Author: s1gr1d

dev-packages/node-integration-tests/suites/express/with-http/maxRequestBodySize/instrument-none.mjs

@@ -6,5 +6,10 @@ Sentry.init({
   release: '1.0',
   tracesSampleRate: 1.0,
   transport: loggingTransport,
-  integrations: [Sentry.httpIntegration({ maxRequestBodySize: 'none' })],
+  integrations: [
+    Sentry.httpIntegration({
+      maxRequestBodySize: 'none',
+      ignoreIncomingRequestBody: url => url.includes('/ignore-request-body'),
+    }),
+  ],
 });

dev-packages/node-integration-tests/suites/express/with-http/maxRequestBodySize/scenario.mjs

@@ -18,6 +18,15 @@ app.post('/test-body-size', (req, res) => {
   });
 });
 
+app.post('/ignore-request-body', (req, res) => {
+  const receivedSize = JSON.stringify(req.body).length;
+  res.json({
+    success: true,
+    receivedSize,
+    message: 'Payload processed successfully',
+  });
+});
+
 Sentry.setupExpressErrorHandler(app);
 
 startExpressServerAndSendPortToRunner(app);

dev-packages/node-integration-tests/suites/express/with-http/maxRequestBodySize/test.ts

@@ -89,6 +89,39 @@ describe('express with httpIntegration and maxRequestBodySize: "none"', () => {
 
         await runner.completed();
       });
+
+      test('does not capture any request bodies with none setting and "ignoreIncomingRequestBody"', async () => {
+        const runner = createRunner()
+          .expect({
+            transaction: {
+              transaction: 'POST /test-body-size',
+              request: expect.not.objectContaining({
+                data: expect.any(String),
+              }),
+            },
+          })
+          .expect({
+            transaction: {
+              transaction: 'POST /ignore-request-body',
+              request: expect.not.objectContaining({
+                data: expect.any(String),
+              }),
+            },
+          })
+          .start();
+
+        await runner.makeRequest('post', '/test-body-size', {
+          headers: { 'Content-Type': 'application/json' },
+          data: JSON.stringify(generatePayload(500)),
+        });
+
+        await runner.makeRequest('post', '/ignore-request-body', {
+          headers: { 'Content-Type': 'application/json' },
+          data: JSON.stringify(generatePayload(500)),
+        });
+
+        await runner.completed();
+      });
     },
     { failsOnEsm: false },
   );

packages/node/src/integrations/http/SentryHttpInstrumentation.ts

@@ -263,7 +263,7 @@ export class SentryHttpInstrumentation extends InstrumentationBase<SentryHttpIns
         const ipAddress = (request as { ip?: string }).ip || request.socket?.remoteAddress;
 
         const url = request.url || '/';
-        if (!ignoreIncomingRequestBody?.(url, request) || maxRequestBodySize !== 'none') {
+        if (!ignoreIncomingRequestBody?.(url, request) && maxRequestBodySize !== 'none') {
           patchRequestToCaptureBody(request, isolationScope, maxRequestBodySize);
         }
 
@@ -372,7 +372,7 @@ function getBreadcrumbData(request: http.ClientRequest): Partial<SanitizedReques
 function patchRequestToCaptureBody(
   req: http.IncomingMessage,
   isolationScope: Scope,
-  maxRequestBodySize: 'none' | 'small' | 'medium' | 'always',
+  maxRequestBodySize: 'small' | 'medium' | 'always',
 ): void {
   let bodyByteLength = 0;
   const chunks: Buffer[] = [];
@@ -386,10 +386,6 @@ function patchRequestToCaptureBody(
    */
   const callbackMap = new WeakMap();
 
-  if (maxRequestBodySize === 'none') {
-    return;
-  }
-
   const maxBodySize =
     maxRequestBodySize === 'small' ? 1_000 : maxRequestBodySize === 'medium' ? 10_000 : MAX_BODY_BYTE_LENGTH;
 
@@ -400,10 +396,9 @@ function patchRequestToCaptureBody(
         const [event, listener, ...restArgs] = args;
 
         if (event === 'data') {
-          if (DEBUG_BUILD) {
-            logger.log(INSTRUMENTATION_NAME, 'Handling request.on("data")');
-            logger.log(INSTRUMENTATION_NAME, `Requested maximum body size: ${maxBodySize}b`);
-          }
+          DEBUG_BUILD &&
+            logger.log(INSTRUMENTATION_NAME, `Handling request.on("data") with maximum body size of ${maxBodySize}b`);
+
           const callback = new Proxy(listener, {
             apply: (target, thisArg, args: Parameters<typeof listener>) => {
               try {