Skip to content

feat(node): Support Express v5 #15380

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 6 commits into from
Feb 20, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -50,13 +50,11 @@ test('Does not send HttpExceptions to Sentry', async ({ baseURL }) => {
});

const transactionEventPromise400 = waitForTransaction('nestjs-11', transactionEvent => {
// todo(express-5): parametrize /test-expected-400-exception/:id
return transactionEvent?.transaction === 'GET /test-expected-400-exception/123';
return transactionEvent?.transaction === 'GET /test-expected-400-exception/:id';
});

const transactionEventPromise500 = waitForTransaction('nestjs-11', transactionEvent => {
// todo(express-5): parametrize /test-expected-500-exception/:id
return transactionEvent?.transaction === 'GET /test-expected-500-exception/123';
return transactionEvent?.transaction === 'GET /test-expected-500-exception/:id';
});

const response400 = await fetch(`${baseURL}/test-expected-400-exception/123`);
Expand All @@ -81,13 +79,11 @@ test('Does not send RpcExceptions to Sentry', async ({ baseURL }) => {
errorEventOccurred = true;
}

// todo(express-5): parametrize /test-expected-rpc-exception/:id
return event?.transaction === 'GET /test-expected-rpc-exception/123';
return event?.transaction === 'GET /test-expected-rpc-exception/:id';
});

const transactionEventPromise = waitForTransaction('nestjs-11', transactionEvent => {
// todo(express-5): parametrize /test-expected-rpc-exception/:id
return transactionEvent?.transaction === 'GET /test-expected-rpc-exception/123';
return transactionEvent?.transaction === 'GET /test-expected-rpc-exception/:id';
});

const response = await fetch(`${baseURL}/test-expected-rpc-exception/123`);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ test('Sends an API route transaction', async ({ baseURL }) => {

expect(transactionEvent.contexts?.trace).toEqual({
data: {
'sentry.source': 'url', // todo(express-5): 'route'
'sentry.source': 'route',
'sentry.origin': 'auto.http.otel.http',
'sentry.op': 'http.server',
'sentry.sample_rate': 1,
Expand All @@ -37,7 +37,7 @@ test('Sends an API route transaction', async ({ baseURL }) => {
'net.peer.port': expect.any(Number),
'http.status_code': 200,
'http.status_text': 'OK',
// 'http.route': '/test-transaction', // todo(express-5): add this line again
'http.route': '/test-transaction',
},
op: 'http.server',
span_id: expect.stringMatching(/[a-f0-9]{16}/),
Expand All @@ -49,7 +49,6 @@ test('Sends an API route transaction', async ({ baseURL }) => {
expect(transactionEvent).toEqual(
expect.objectContaining({
spans: expect.arrayContaining([
/* todo(express-5): add this part again
{
data: {
'express.name': '/test-transaction',
Expand All @@ -67,7 +66,7 @@ test('Sends an API route transaction', async ({ baseURL }) => {
timestamp: expect.any(Number),
trace_id: expect.stringMatching(/[a-f0-9]{32}/),
origin: 'auto.http.otel.express',
}, */
},
{
data: {
'sentry.origin': 'manual',
Expand Down Expand Up @@ -117,7 +116,7 @@ test('Sends an API route transaction', async ({ baseURL }) => {
transaction: 'GET /test-transaction',
type: 'transaction',
transaction_info: {
source: 'url', // todo(express-5): 'route'
source: 'route',
},
}),
);
Expand Down Expand Up @@ -272,8 +271,7 @@ test('API route transaction includes nest pipe span for valid request', async ({
const transactionEventPromise = waitForTransaction('nestjs-11', transactionEvent => {
return (
transactionEvent?.contexts?.trace?.op === 'http.server' &&
// todo(express-5): parametrize test-pipe-instrumentation/:id
transactionEvent?.transaction === 'GET /test-pipe-instrumentation/123' &&
transactionEvent?.transaction === 'GET /test-pipe-instrumentation/:id' &&
transactionEvent?.request?.url?.includes('/test-pipe-instrumentation/123')
);
});
Expand Down Expand Up @@ -310,8 +308,7 @@ test('API route transaction includes nest pipe span for invalid request', async
const transactionEventPromise = waitForTransaction('nestjs-11', transactionEvent => {
return (
transactionEvent?.contexts?.trace?.op === 'http.server' &&
// todo(express-5): parametrize test-pipe-instrumentation/:id
transactionEvent?.transaction === 'GET /test-pipe-instrumentation/abc' &&
transactionEvent?.transaction === 'GET /test-pipe-instrumentation/:id' &&
transactionEvent?.request?.url?.includes('/test-pipe-instrumentation/abc')
);
});
Expand Down
2 changes: 2 additions & 0 deletions dev-packages/node-integration-tests/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -16,9 +16,11 @@
"build:types": "tsc -p tsconfig.types.json",
"clean": "rimraf -g **/node_modules && run-p clean:script",
"clean:script": "node scripts/clean.js",
"express-v5-install": "cd suites/express-v5 && yarn --no-lockfile",
"lint": "eslint . --format stylish",
"fix": "eslint . --format stylish --fix",
"type-check": "tsc",
"pretest": "yarn express-v5-install",
"test": "jest --config ./jest.config.js",
"test:watch": "yarn test --watch"
},
Expand Down
Original file line number Diff line number Diff line change
@@ -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);
Original file line number Diff line number Diff line change
@@ -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 });
});
Original file line number Diff line number Diff line change
@@ -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);
Original file line number Diff line number Diff line change
@@ -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 });
});
Original file line number Diff line number Diff line change
@@ -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);
Original file line number Diff line number Diff line change
@@ -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 });
});
Loading
Loading