test(node): Update express integration tests (#16204)

This updates naming etc. for express (v5) tests, and makes sure that we
test this in esm & cjs in a basic way.
It also adds test for handling of the root route (`/`) as well as 404,
where current failures are shown (see
#16203).

.size-limit.js

@@ -79,7 +79,7 @@ module.exports = [
     path: 'packages/browser/build/npm/esm/index.js',
     import: createImport('init', 'browserTracingIntegration', 'replayIntegration', 'replayCanvasIntegration'),
     gzip: true,
-    limit: '81 KB',
+    limit: '82 KB',
   },
   {
     name: '@sentry/browser (incl. Tracing, Replay, Feedback)',

dev-packages/node-integration-tests/src/index.ts

@@ -25,7 +25,10 @@ export function loggingTransport(_options: BaseTransportOptions): Transport {
  * Setting this port to something specific is useful for local debugging but dangerous for
  * CI/CD environments where port collisions can cause flakes!
  */
-export function startExpressServerAndSendPortToRunner(app: Express, port: number | undefined = undefined): void {
+export function startExpressServerAndSendPortToRunner(
+  app: Pick<Express, 'listen'>,
+  port: number | undefined = undefined,
+): void {
   const server = app.listen(port || 0, () => {
     const address = server.address() as AddressInfo;
 

dev-packages/node-integration-tests/suites/express-v5/tracing/instrument.mjs

@@ -0,0 +1,11 @@
+import * as Sentry from '@sentry/node';
+import { loggingTransport } from '@sentry-internal/node-integration-tests';
+
+Sentry.init({
+  dsn: 'https://public@dsn.ingest.sentry.io/1337',
+  release: '1.0',
+  // disable attaching headers to /test/* endpoints
+  tracePropagationTargets: [/^(?!.*test).*$/],
+  tracesSampleRate: 1.0,
+  transport: loggingTransport,
+});

dev-packages/node-integration-tests/suites/express-v5/tracing/scenario.mjs

@@ -0,0 +1,40 @@
+import * as Sentry from '@sentry/node';
+import { startExpressServerAndSendPortToRunner } from '@sentry-internal/node-integration-tests';
+import bodyParser from 'body-parser';
+import cors from 'cors';
+import express from 'express';
+
+const app = express();
+
+app.use(cors());
+app.use(bodyParser.json());
+app.use(bodyParser.text());
+app.use(bodyParser.raw());
+
+app.get('/', (_req, res) => {
+  res.send({ response: 'response 0' });
+});
+
+app.get('/test/express', (_req, res) => {
+  res.send({ response: 'response 1' });
+});
+
+app.get(/\/test\/regex/, (_req, res) => {
+  res.send({ response: 'response 2' });
+});
+
+app.get(['/test/array1', /\/test\/array[2-9]/], (_req, res) => {
+  res.send({ response: 'response 3' });
+});
+
+app.get(['/test/arr/:id', /\/test\/arr[0-9]*\/required(path)?(\/optionalPath)?\/(lastParam)?/], (_req, res) => {
+  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-v5/tracing/test.ts

@@ -1,14 +1,14 @@
-import { afterAll, describe, expect, test } from 'vitest';
-import { cleanupChildProcesses, createRunner } from '../../../utils/runner';
+import { afterAll, describe, expect } from 'vitest';
+import { cleanupChildProcesses, createEsmAndCjsTests } from '../../../utils/runner';
 
-describe('express tracing', () => {
+describe('express v5 tracing', () => {
   afterAll(() => {
     cleanupChildProcesses();
   });
 
-  describe('CJS', () => {
+  createEsmAndCjsTests(__dirname, 'scenario.mjs', 'instrument.mjs', (createRunner, test) => {
     test('should create and send transactions for Express routes and spans for middlewares.', async () => {
-      const runner = createRunner(__dirname, 'server.js')
+      const runner = createRunner()
         .expect({
           transaction: {
             contexts: {
@@ -51,7 +51,7 @@ describe('express tracing', () => {
     });
 
     test('should set a correct transaction name for routes specified in RegEx', async () => {
-      const runner = createRunner(__dirname, 'server.js')
+      const runner = createRunner()
         .expect({
           transaction: {
             transaction: 'GET /\\/test\\/regex/',
@@ -77,10 +77,52 @@ describe('express tracing', () => {
       await runner.completed();
     });
 
+    test('handles root page correctly', async () => {
+      const runner = createRunner()
+        .expect({
+          transaction: {
+            transaction: 'GET /',
+          },
+        })
+        .start();
+      runner.makeRequest('get', '/');
+      await runner.completed();
+    });
+
+    test('handles 404 page correctly', async () => {
+      const runner = createRunner()
+        .expect({
+          transaction: {
+            // FIXME: This is wrong :(
+            transaction: 'GET /',
+            contexts: {
+              trace: {
+                span_id: expect.stringMatching(/[a-f0-9]{16}/),
+                trace_id: expect.stringMatching(/[a-f0-9]{32}/),
+                data: {
+                  'http.response.status_code': 404,
+                  url: expect.stringMatching(/\/does-not-exist$/),
+                  'http.method': 'GET',
+                  // FIXME: This is wrong :(
+                  'http.route': '/',
+                  'http.url': expect.stringMatching(/\/does-not-exist$/),
+                  'http.target': '/does-not-exist',
+                },
+                op: 'http.server',
+                status: 'not_found',
+              },
+            },
+          },
+        })
+        .start();
+      runner.makeRequest('get', '/does-not-exist', { expectError: true });
+      await runner.completed();
+    });
+
     test.each([['array1'], ['array5']])(
       'should set a correct transaction name for routes consisting of arrays of routes for %p',
       async (segment: string) => {
-        const runner = await createRunner(__dirname, 'server.js')
+        const runner = await createRunner()
           .expect({
             transaction: {
               transaction: 'GET /test/array1,/\\/test\\/array[2-9]/',
@@ -115,7 +157,7 @@ describe('express tracing', () => {
       ['arr/required/lastParam'],
       ['arr55/required/lastParam'],
     ])('should handle more complex regexes in route arrays correctly for %p', async (segment: string) => {
-      const runner = await createRunner(__dirname, 'server.js')
+      const runner = await createRunner()
         .expect({
           transaction: {
             transaction: 'GET /test/arr/:id,/\\/test\\/arr[0-9]*\\/required(path)?(\\/optionalPath)?\\/(lastParam)?/',
@@ -143,7 +185,7 @@ describe('express tracing', () => {
 
     describe('request data', () => {
       test('correctly captures JSON request data', async () => {
-        const runner = createRunner(__dirname, 'server.js')
+        const runner = createRunner()
           .expect({
             transaction: {
               transaction: 'POST /test-post',
@@ -173,7 +215,7 @@ describe('express tracing', () => {
       });
 
       test('correctly captures plain text request data', async () => {
-        const runner = createRunner(__dirname, 'server.js')
+        const runner = createRunner()
           .expect({
             transaction: {
               transaction: 'POST /test-post',
@@ -198,7 +240,7 @@ describe('express tracing', () => {
       });
 
       test('correctly captures text buffer request data', async () => {
-        const runner = createRunner(__dirname, 'server.js')
+        const runner = createRunner()
           .expect({
             transaction: {
               transaction: 'POST /test-post',
@@ -223,7 +265,7 @@ describe('express tracing', () => {
       });
 
       test('correctly captures non-text buffer request data', async () => {
-        const runner = createRunner(__dirname, 'server.js')
+        const runner = createRunner()
           .expect({
             transaction: {
               transaction: 'POST /test-post',

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

@@ -3,7 +3,7 @@ import { SEMANTIC_ATTRIBUTE_SENTRY_SOURCE } from '@sentry/node';
 import { afterAll, describe, expect, test } from 'vitest';
 import { cleanupChildProcesses, createRunner } from '../../../../utils/runner';
 
-describe('express tracing', () => {
+describe('express v5 tracing', () => {
   afterAll(() => {
     cleanupChildProcesses();
   });

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

@@ -1,7 +1,7 @@
 import { afterAll, describe, test } from 'vitest';
 import { cleanupChildProcesses, createRunner } from '../../../../utils/runner';
 
-describe('express tracing experimental', () => {
+describe('express v5 tracing', () => {
   afterAll(() => {
     cleanupChildProcesses();
   });

dev-packages/node-integration-tests/suites/express-v5/tsconfig.test.json

@@ -0,0 +1,3 @@
+{
+  "extends": "../tsconfig.json"
+}

dev-packages/node-integration-tests/suites/express/tracing/instrument.mjs

@@ -0,0 +1,21 @@
+import * as Sentry from '@sentry/node';
+import { loggingTransport } from '@sentry-internal/node-integration-tests';
+
+Sentry.init({
+  dsn: 'https://public@dsn.ingest.sentry.io/1337',
+  release: '1.0',
+  // disable attaching headers to /test/* endpoints
+  tracePropagationTargets: [/^(?!.*test).*$/],
+  tracesSampleRate: 1.0,
+  transport: loggingTransport,
+  integrations: [
+    Sentry.httpIntegration({
+      ignoreIncomingRequestBody: url => {
+        if (url.includes('/test-post-ignore-body')) {
+          return true;
+        }
+        return false;
+      },
+    }),
+  ],
+});

dev-packages/node-integration-tests/suites/express-v5/tracing/server.js → dev-packages/node-integration-tests/suites/express/tracing/scenario.mjs

@@ -1,20 +1,8 @@
-const { loggingTransport } = require('@sentry-internal/node-integration-tests');
-const Sentry = require('@sentry/node');
-
-Sentry.init({
-  dsn: 'https://public@dsn.ingest.sentry.io/1337',
-  release: '1.0',
-  // disable attaching headers to /test/* endpoints
-  tracePropagationTargets: [/^(?!.*test).*$/],
-  tracesSampleRate: 1.0,
-  transport: loggingTransport,
-});
-
-// 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');
+import * as Sentry from '@sentry/node';
+import { startExpressServerAndSendPortToRunner } from '@sentry-internal/node-integration-tests';
+import bodyParser from 'body-parser';
+import cors from 'cors';
+import express from 'express';
 
 const app = express();
 
@@ -23,6 +11,10 @@ app.use(bodyParser.json());
 app.use(bodyParser.text());
 app.use(bodyParser.raw());
 
+app.get('/', (_req, res) => {
+  res.send({ response: 'response 0' });
+});
+
 app.get('/test/express', (_req, res) => {
   res.send({ response: 'response 1' });
 });
@@ -43,6 +35,10 @@ app.post('/test-post', function (req, res) {
   res.send({ status: 'ok', body: req.body });
 });
 
+app.post('/test-post-ignore-body', function (req, res) {
+  res.send({ status: 'ok', body: req.body });
+});
+
 Sentry.setupExpressErrorHandler(app);
 
 startExpressServerAndSendPortToRunner(app);