feat(node): Ensure request bodies are reliably captured for http requests

dev-packages/e2e-tests/test-applications/node-koa/index.js

@@ -14,10 +14,12 @@ const port1 = 3030;
 const port2 = 3040;
 
 const Koa = require('koa');
+const { bodyParser } = require('@koa/bodyparser');
 const Router = require('@koa/router');
 const http = require('http');
 
 const app1 = new Koa();
+app1.use(bodyParser());
 
 Sentry.setupKoaErrorHandler(app1);
 
@@ -109,6 +111,10 @@ router1.get('/test-assert/:condition', async ctx => {
   ctx.assert(condition, 400, 'ctx.assert failed');
 });
 
+router1.post('/test-post', async ctx => {
+  ctx.body = { status: 'ok', body: ctx.request.body };
+});
+
 app1.use(router1.routes()).use(router1.allowedMethods());
 
 app1.listen(port1);

dev-packages/e2e-tests/test-applications/node-koa/package.json

@@ -10,6 +10,7 @@
     "test:assert": "pnpm test"
   },
   "dependencies": {
+    "@koa/bodyparser": "^5.1.1",
     "@koa/router": "^12.0.1",
     "@sentry/node": "latest || *",
     "@sentry/types": "latest || *",

dev-packages/e2e-tests/test-applications/node-koa/tests/assert.test.ts

@@ -16,7 +16,6 @@ test('Returns 400 from failed assert', async ({ baseURL }) => {
 
   expect(errorEvent.request).toEqual({
     method: 'GET',
-    cookies: {},
     headers: expect.any(Object),
     url: 'http://localhost:3030/test-assert/false',
   });

dev-packages/e2e-tests/test-applications/node-koa/tests/errors.test.ts

@@ -15,7 +15,6 @@ test('Sends correct error event', async ({ baseURL }) => {
 
   expect(errorEvent.request).toEqual({
     method: 'GET',
-    cookies: {},
     headers: expect.any(Object),
     url: 'http://localhost:3030/test-exception/123',
   });

dev-packages/e2e-tests/test-applications/node-koa/tests/transactions.test.ts

@@ -46,76 +46,128 @@ test('Sends an API route transaction', async ({ baseURL }) => {
     origin: 'auto.http.otel.http',
   });
 
-  expect(transactionEvent).toEqual(
-    expect.objectContaining({
-      spans: [
-        {
-          data: {
-            'koa.name': '',
-            'koa.type': 'middleware',
-            'sentry.origin': 'auto.http.otel.koa',
-            'sentry.op': 'middleware.koa',
-          },
-          op: 'middleware.koa',
-          origin: 'auto.http.otel.koa',
-          description: '< unknown >',
-          parent_span_id: expect.any(String),
-          span_id: expect.any(String),
-          start_timestamp: expect.any(Number),
-          status: 'ok',
-          timestamp: expect.any(Number),
-          trace_id: expect.any(String),
-        },
-        {
-          data: {
-            'http.route': '/test-transaction',
-            'koa.name': '/test-transaction',
-            'koa.type': 'router',
-            'sentry.origin': 'auto.http.otel.koa',
-            'sentry.op': 'router.koa',
-          },
-          op: 'router.koa',
-          description: '/test-transaction',
-          parent_span_id: expect.any(String),
-          span_id: expect.any(String),
-          start_timestamp: expect.any(Number),
-          status: 'ok',
-          timestamp: expect.any(Number),
-          trace_id: expect.any(String),
-          origin: 'auto.http.otel.koa',
-        },
-        {
-          data: {
-            'sentry.origin': 'manual',
-          },
-          description: 'test-span',
-          parent_span_id: expect.any(String),
-          span_id: expect.any(String),
-          start_timestamp: expect.any(Number),
-          status: 'ok',
-          timestamp: expect.any(Number),
-          trace_id: expect.any(String),
-          origin: 'manual',
-        },
-        {
-          data: {
-            'sentry.origin': 'manual',
-          },
-          description: 'child-span',
-          parent_span_id: expect.any(String),
-          span_id: expect.any(String),
-          start_timestamp: expect.any(Number),
-          status: 'ok',
-          timestamp: expect.any(Number),
-          trace_id: expect.any(String),
-          origin: 'manual',
-        },
-      ],
-      transaction: 'GET /test-transaction',
-      type: 'transaction',
-      transaction_info: {
-        source: 'route',
+  expect(transactionEvent).toMatchObject({
+    transaction: 'GET /test-transaction',
+    type: 'transaction',
+    transaction_info: {
+      source: 'route',
+    },
+  });
+
+  expect(transactionEvent.spans).toEqual([
+    {
+      data: {
+        'koa.name': 'bodyParser',
+        'koa.type': 'middleware',
+        'sentry.op': 'middleware.koa',
+        'sentry.origin': 'auto.http.otel.koa',
+      },
+      description: 'bodyParser',
+      op: 'middleware.koa',
+      origin: 'auto.http.otel.koa',
+      parent_span_id: expect.any(String),
+      span_id: expect.any(String),
+      start_timestamp: expect.any(Number),
+      status: 'ok',
+      timestamp: expect.any(Number),
+      trace_id: expect.any(String),
+    },
+    {
+      data: {
+        'koa.name': '',
+        'koa.type': 'middleware',
+        'sentry.origin': 'auto.http.otel.koa',
+        'sentry.op': 'middleware.koa',
+      },
+      op: 'middleware.koa',
+      origin: 'auto.http.otel.koa',
+      description: '< unknown >',
+      parent_span_id: expect.any(String),
+      span_id: expect.any(String),
+      start_timestamp: expect.any(Number),
+      status: 'ok',
+      timestamp: expect.any(Number),
+      trace_id: expect.any(String),
+    },
+    {
+      data: {
+        'http.route': '/test-transaction',
+        'koa.name': '/test-transaction',
+        'koa.type': 'router',
+        'sentry.origin': 'auto.http.otel.koa',
+        'sentry.op': 'router.koa',
       },
+      op: 'router.koa',
+      description: '/test-transaction',
+      parent_span_id: expect.any(String),
+      span_id: expect.any(String),
+      start_timestamp: expect.any(Number),
+      status: 'ok',
+      timestamp: expect.any(Number),
+      trace_id: expect.any(String),
+      origin: 'auto.http.otel.koa',
+    },
+    {
+      data: {
+        'sentry.origin': 'manual',
+      },
+      description: 'test-span',
+      parent_span_id: expect.any(String),
+      span_id: expect.any(String),
+      start_timestamp: expect.any(Number),
+      status: 'ok',
+      timestamp: expect.any(Number),
+      trace_id: expect.any(String),
+      origin: 'manual',
+    },
+    {
+      data: {
+        'sentry.origin': 'manual',
+      },
+      description: 'child-span',
+      parent_span_id: expect.any(String),
+      span_id: expect.any(String),
+      start_timestamp: expect.any(Number),
+      status: 'ok',
+      timestamp: expect.any(Number),
+      trace_id: expect.any(String),
+      origin: 'manual',
+    },
+  ]);
+});
+
+test('Captures request metadata', async ({ baseURL }) => {
+  const transactionEventPromise = waitForTransaction('node-koa', transactionEvent => {
+    return (
+      transactionEvent?.contexts?.trace?.op === 'http.server' && transactionEvent?.transaction === 'POST /test-post'
+    );
+  });
+
+  const res = await fetch(`${baseURL}/test-post`, {
+    method: 'POST',
+    body: JSON.stringify({ foo: 'bar', other: 1 }),
+    headers: {
+      'Content-Type': 'application/json',
+    },
+  });
+  const resBody = await res.json();
+
+  expect(resBody).toEqual({ status: 'ok', body: { foo: 'bar', other: 1 } });
+
+  const transactionEvent = await transactionEventPromise;
+
+  expect(transactionEvent.request).toEqual({
+    url: expect.stringMatching(/^http:\/\/localhost:(\d+)\/test-post$/),
+    method: 'POST',
+    headers: expect.objectContaining({
+      'user-agent': expect.stringContaining(''),
+      'content-type': 'application/json',
     }),
-  );
+    data: JSON.stringify({
+      foo: 'bar',
+      other: 1,
+    }),
+  });
+
+  expect(transactionEvent.user).toEqual(undefined);
 });

dev-packages/node-integration-tests/package.json

@@ -41,6 +41,7 @@
     "amqplib": "^0.10.4",
     "apollo-server": "^3.11.1",
     "axios": "^1.6.7",
+    "body-parser": "^1.20.3",
     "connect": "^3.7.0",
     "cors": "^2.8.5",
     "cron": "^3.1.6",

dev-packages/node-integration-tests/suites/express/tracing/server.js

@@ -13,11 +13,15 @@ Sentry.init({
 // express must be required after Sentry is initialized
 const express = require('express');
 const cors = require('cors');
+const bodyParser = require('body-parser');
 const { startExpressServerAndSendPortToRunner } = require('@sentry-internal/node-integration-tests');
 
 const app = express();
 
 app.use(cors());
+app.use(bodyParser.json());
+app.use(bodyParser.text());
+app.use(bodyParser.raw());
 
 app.get('/test/express', (_req, res) => {
   res.send({ response: 'response 1' });
@@ -35,6 +39,10 @@ app.get(['/test/arr/:id', /\/test\/arr[0-9]*\/required(path)?(\/optionalPath)?\/
   res.send({ response: 'response 4' });
 });
 
+app.post('/test-post', function (req, res) {
+  res.send({ status: 'ok', body: req.body });
+});
+
 Sentry.setupExpressErrorHandler(app);
 
 startExpressServerAndSendPortToRunner(app);

dev-packages/node-integration-tests/suites/express/tracing/test.ts

@@ -137,5 +137,109 @@ describe('express tracing', () => {
         .start(done)
         .makeRequest('get', `/test/${segment}`);
     }) as any);
+
+    describe('request data', () => {
+      test('correctly captures JSON request data', done => {
+        const runner = createRunner(__dirname, 'server.js')
+          .expect({
+            transaction: {
+              transaction: 'POST /test-post',
+              request: {
+                url: expect.stringMatching(/^http:\/\/localhost:(\d+)\/test-post$/),
+                method: 'POST',
+                headers: {
+                  'user-agent': expect.stringContaining(''),
+                  'content-type': 'application/json',
+                },
+                data: JSON.stringify({
+                  foo: 'bar',
+                  other: 1,
+                }),
+              },
+            },
+          })
+          .start(done);
+
+        runner.makeRequest('post', '/test-post', {}, { foo: 'bar', other: 1 });
+      });
+
+      test('correctly captures plain text request data', done => {
+        const runner = createRunner(__dirname, 'server.js')
+          .expect({
+            transaction: {
+              transaction: 'POST /test-post',
+              request: {
+                url: expect.stringMatching(/^http:\/\/localhost:(\d+)\/test-post$/),
+                method: 'POST',
+                headers: {
+                  'user-agent': expect.stringContaining(''),
+                  'content-type': 'text/plain',
+                },
+                data: 'some plain text',
+              },
+            },
+          })
+          .start(done);
+
+        runner.makeRequest(
+          'post',
+          '/test-post',
+          {
+            'Content-Type': 'text/plain',
+          },
+          'some plain text',
+        );
+      });
+
+      test('correctly captures text buffer request data', done => {
+        const runner = createRunner(__dirname, 'server.js')
+          .expect({
+            transaction: {
+              transaction: 'POST /test-post',
+              request: {
+                url: expect.stringMatching(/^http:\/\/localhost:(\d+)\/test-post$/),
+                method: 'POST',
+                headers: {
+                  'user-agent': expect.stringContaining(''),
+                  'content-type': 'application/octet-stream',
+                },
+                data: 'some plain text in buffer',
+              },
+            },
+          })
+          .start(done);
+
+        runner.makeRequest(
+          'post',
+          '/test-post',
+          { 'Content-Type': 'application/octet-stream' },
+          Buffer.from('some plain text in buffer'),
+        );
+      });
+
+      test('correctly captures non-text buffer request data', done => {
+        const runner = createRunner(__dirname, 'server.js')
+          .expect({
+            transaction: {
+              transaction: 'POST /test-post',
+              request: {
+                url: expect.stringMatching(/^http:\/\/localhost:(\d+)\/test-post$/),
+                method: 'POST',
+                headers: {
+                  'user-agent': expect.stringContaining(''),
+                  'content-type': 'application/octet-stream',
+                },
+                // This is some non-ascii string representation
+                data: expect.any(String),
+              },
+            },
+          })
+          .start(done);
+
+        const body = new Uint8Array([1, 2, 3, 4, 5]).buffer;
+
+        runner.makeRequest('post', '/test-post', { 'Content-Type': 'application/octet-stream' }, body);
+      });
+    });
   });
 });