feat(node): Support Express v5

Author: timfish

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

@@ -18,10 +18,11 @@
     "clean:script": "node scripts/clean.js",
     "prisma-v5:init": "cd suites/tracing/prisma-orm-v5 && yarn && yarn setup",
     "prisma-v6:init": "cd suites/tracing/prisma-orm-v6 && yarn && yarn setup",
+    "express-v5-install": "cd suites/express-v5 && yarn --no-lockfile",
     "lint": "eslint . --format stylish",
     "fix": "eslint . --format stylish --fix",
     "type-check": "tsc",
-    "pretest": "run-s --silent prisma-v5:init prisma-v6:init",
+    "pretest": "run-s --silent prisma-v5:init prisma-v6:init express-v5-install",
     "test": "jest --config ./jest.config.js",
     "test:watch": "yarn test --watch"
   },

dev-packages/node-integration-tests/suites/express-v5/handle-error-scope-data-loss/server.ts

@@ -0,0 +1,31 @@
+import { loggingTransport } from '@sentry-internal/node-integration-tests';
+import * as Sentry from '@sentry/node';
+
+Sentry.init({
+  dsn: 'https://[email protected]/1337',
+  release: '1.0',
+  transport: loggingTransport,
+});
+
+import { startExpressServerAndSendPortToRunner } from '@sentry-internal/node-integration-tests';
+import express from 'express';
+
+const app = express();
+
+Sentry.setTag('global', 'tag');
+
+app.get('/test/withScope', () => {
+  Sentry.withScope(scope => {
+    scope.setTag('local', 'tag');
+    throw new Error('test_error');
+  });
+});
+
+app.get('/test/isolationScope', () => {
+  Sentry.getIsolationScope().setTag('isolation-scope', 'tag');
+  throw new Error('isolation_test_error');
+});
+
+Sentry.setupExpressErrorHandler(app);
+
+startExpressServerAndSendPortToRunner(app);

dev-packages/node-integration-tests/suites/express-v5/handle-error-scope-data-loss/test.ts

@@ -0,0 +1,85 @@
+import { cleanupChildProcesses, createRunner } from '../../../utils/runner';
+
+afterAll(() => {
+  cleanupChildProcesses();
+});
+
+/**
+ * Why does this test exist?
+ *
+ * We recently discovered that errors caught by global handlers will potentially loose scope data from the active scope
+ * where the error was originally thrown in. The simple example in this test (see subject.ts) demonstrates this behavior
+ * (in a Node environment but the same behavior applies to the browser; see the test there).
+ *
+ * This test nevertheless covers the behavior so that we're aware.
+ */
+test('withScope scope is NOT applied to thrown error caught by global handler', done => {
+  createRunner(__dirname, 'server.ts')
+    .expect({
+      event: {
+        exception: {
+          values: [
+            {
+              mechanism: {
+                type: 'middleware',
+                handled: false,
+              },
+              type: 'Error',
+              value: 'test_error',
+              stacktrace: {
+                frames: expect.arrayContaining([
+                  expect.objectContaining({
+                    function: expect.any(String),
+                    lineno: expect.any(Number),
+                    colno: expect.any(Number),
+                  }),
+                ]),
+              },
+            },
+          ],
+        },
+        // 'local' tag is not applied to the event
+        tags: expect.not.objectContaining({ local: expect.anything() }),
+      },
+    })
+    .start(done)
+    .makeRequest('get', '/test/withScope', { expectError: true });
+});
+
+/**
+ * This test shows that the isolation scope set tags are applied correctly to the error.
+ */
+test('isolation scope is applied to thrown error caught by global handler', done => {
+  createRunner(__dirname, 'server.ts')
+    .expect({
+      event: {
+        exception: {
+          values: [
+            {
+              mechanism: {
+                type: 'middleware',
+                handled: false,
+              },
+              type: 'Error',
+              value: 'isolation_test_error',
+              stacktrace: {
+                frames: expect.arrayContaining([
+                  expect.objectContaining({
+                    function: expect.any(String),
+                    lineno: expect.any(Number),
+                    colno: expect.any(Number),
+                  }),
+                ]),
+              },
+            },
+          ],
+        },
+        tags: {
+          global: 'tag',
+          'isolation-scope': 'tag',
+        },
+      },
+    })
+    .start(done)
+    .makeRequest('get', '/test/isolationScope', { expectError: true });
+});

dev-packages/node-integration-tests/suites/express-v5/handle-error-tracesSampleRate-0/server.ts

@@ -0,0 +1,22 @@
+import { loggingTransport } from '@sentry-internal/node-integration-tests';
+import * as Sentry from '@sentry/node';
+
+Sentry.init({
+  dsn: 'https://[email protected]/1337',
+  release: '1.0',
+  transport: loggingTransport,
+  tracesSampleRate: 1,
+});
+
+import { startExpressServerAndSendPortToRunner } from '@sentry-internal/node-integration-tests';
+import express from 'express';
+
+const app = express();
+
+app.get('/test/express/:id', req => {
+  throw new Error(`test_error with id ${req.params.id}`);
+});
+
+Sentry.setupExpressErrorHandler(app);
+
+startExpressServerAndSendPortToRunner(app);

dev-packages/node-integration-tests/suites/express-v5/handle-error-tracesSampleRate-0/test.ts

@@ -0,0 +1,38 @@
+import { cleanupChildProcesses, createRunner } from '../../../utils/runner';
+
+afterAll(() => {
+  cleanupChildProcesses();
+});
+
+test('should capture and send Express controller error with txn name if tracesSampleRate is 0', done => {
+  createRunner(__dirname, 'server.ts')
+    .ignore('transaction')
+    .expect({
+      event: {
+        exception: {
+          values: [
+            {
+              mechanism: {
+                type: 'middleware',
+                handled: false,
+              },
+              type: 'Error',
+              value: 'test_error with id 123',
+              stacktrace: {
+                frames: expect.arrayContaining([
+                  expect.objectContaining({
+                    function: expect.any(String),
+                    lineno: expect.any(Number),
+                    colno: expect.any(Number),
+                  }),
+                ]),
+              },
+            },
+          ],
+        },
+        transaction: 'GET /test/express/:id',
+      },
+    })
+    .start(done)
+    .makeRequest('get', '/test/express/123', { expectError: true });
+});

dev-packages/node-integration-tests/suites/express-v5/handle-error-tracesSampleRate-unset/server.ts

@@ -0,0 +1,21 @@
+import { loggingTransport } from '@sentry-internal/node-integration-tests';
+import * as Sentry from '@sentry/node';
+
+Sentry.init({
+  dsn: 'https://[email protected]/1337',
+  release: '1.0',
+  transport: loggingTransport,
+});
+
+import { startExpressServerAndSendPortToRunner } from '@sentry-internal/node-integration-tests';
+import express from 'express';
+
+const app = express();
+
+app.get('/test/express/:id', req => {
+  throw new Error(`test_error with id ${req.params.id}`);
+});
+
+Sentry.setupExpressErrorHandler(app);
+
+startExpressServerAndSendPortToRunner(app);

dev-packages/node-integration-tests/suites/express-v5/handle-error-tracesSampleRate-unset/test.ts

@@ -0,0 +1,37 @@
+import { cleanupChildProcesses, createRunner } from '../../../utils/runner';
+
+afterAll(() => {
+  cleanupChildProcesses();
+});
+
+test('should capture and send Express controller error if tracesSampleRate is not set.', done => {
+  createRunner(__dirname, 'server.ts')
+    .ignore('transaction')
+    .expect({
+      event: {
+        exception: {
+          values: [
+            {
+              mechanism: {
+                type: 'middleware',
+                handled: false,
+              },
+              type: 'Error',
+              value: 'test_error with id 123',
+              stacktrace: {
+                frames: expect.arrayContaining([
+                  expect.objectContaining({
+                    function: expect.any(String),
+                    lineno: expect.any(Number),
+                    colno: expect.any(Number),
+                  }),
+                ]),
+              },
+            },
+          ],
+        },
+      },
+    })
+    .start(done)
+    .makeRequest('get', '/test/express/123', { expectError: true });
+});

dev-packages/node-integration-tests/suites/express-v5/multiple-init/server.ts

@@ -0,0 +1,74 @@
+import { loggingTransport } from '@sentry-internal/node-integration-tests';
+import * as Sentry from '@sentry/node';
+
+Sentry.init({
+  // No dsn, means  client is disabled
+  // dsn: 'https://[email protected]/1337',
+  release: '1.0',
+  transport: loggingTransport,
+});
+
+// We add http integration to ensure request isolation etc. works
+const initialClient = Sentry.getClient();
+initialClient?.addIntegration(Sentry.httpIntegration());
+
+// Store this so we can update the client later
+const initialCurrentScope = Sentry.getCurrentScope();
+
+import { startExpressServerAndSendPortToRunner } from '@sentry-internal/node-integration-tests';
+import express from 'express';
+
+const app = express();
+
+Sentry.setTag('global', 'tag');
+
+app.get('/test/no-init', (_req, res) => {
+  Sentry.addBreadcrumb({ message: 'no init breadcrumb' });
+  Sentry.setTag('no-init', 'tag');
+
+  res.send({});
+});
+
+app.get('/test/init', (_req, res) => {
+  // Call init again, but with DSN
+  Sentry.init({
+    dsn: 'https://[email protected]/1337',
+    release: '1.0',
+    transport: loggingTransport,
+  });
+  // Set this on initial scope, to ensure it can be inherited
+  initialCurrentScope.setClient(Sentry.getClient()!);
+
+  Sentry.addBreadcrumb({ message: 'init breadcrumb' });
+  Sentry.setTag('init', 'tag');
+
+  res.send({});
+});
+
+app.get('/test/error/:id', (req, res) => {
+  const id = req.params.id;
+  Sentry.addBreadcrumb({ message: `error breadcrumb ${id}` });
+  Sentry.setTag('error', id);
+
+  Sentry.captureException(new Error(`This is an exception ${id}`));
+
+  setTimeout(() => {
+    // We flush to ensure we are sending exceptions in a certain order
+    Sentry.flush(1000).then(
+      () => {
+        // We send this so we can wait for this, to know the test is ended & server can be closed
+        if (id === '3') {
+          Sentry.captureException(new Error('Final exception was captured'));
+        }
+        res.send({});
+      },
+      () => {
+        res.send({});
+      },
+    );
+  }, 1);
+});
+
+Sentry.setupExpressErrorHandler(app);
+
+startExpressServerAndSendPortToRunner(app);

dev-packages/node-integration-tests/suites/express-v5/multiple-init/test.ts

@@ -0,0 +1,70 @@
+import { cleanupChildProcesses, createRunner } from '../../../utils/runner';
+
+afterAll(() => {
+  cleanupChildProcesses();
+});
+
+test('allows to call init multiple times', done => {
+  const runner = createRunner(__dirname, 'server.ts')
+    .expect({
+      event: {
+        exception: {
+          values: [
+            {
+              value: 'This is an exception 2',
+            },
+          ],
+        },
+        breadcrumbs: [
+          {
+            message: 'error breadcrumb 2',
+            timestamp: expect.any(Number),
+          },
+        ],
+        tags: {
+          global: 'tag',
+          error: '2',
+        },
+      },
+    })
+    .expect({
+      event: {
+        exception: {
+          values: [
+            {
+              value: 'This is an exception 3',
+            },
+          ],
+        },
+        breadcrumbs: [
+          {
+            message: 'error breadcrumb 3',
+            timestamp: expect.any(Number),
+          },
+        ],
+        tags: {
+          global: 'tag',
+          error: '3',
+        },
+      },
+    })
+    .expect({
+      event: {
+        exception: {
+          values: [
+            {
+              value: 'Final exception was captured',
+            },
+          ],
+        },
+      },
+    })
+    .start(done);
+
+  runner
+    .makeRequest('get', '/test/no-init')
+    .then(() => runner.makeRequest('get', '/test/error/1'))
+    .then(() => runner.makeRequest('get', '/test/init'))
+    .then(() => runner.makeRequest('get', '/test/error/2'))
+    .then(() => runner.makeRequest('get', '/test/error/3'));
+});