Skip to content

Commit 38316fd

Browse files
mydeamydea
authored
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).
1 parent a04f3cc commit 38316fd

File tree

13 files changed

+204
-114
lines changed

13 files changed

+204
-114
lines changed

‎.size-limit.js‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -79,7 +79,7 @@ module.exports = [
7979
path: 'packages/browser/build/npm/esm/index.js',
8080
import: createImport('init', 'browserTracingIntegration', 'replayIntegration', 'replayCanvasIntegration'),
8181
gzip: true,
82-
limit: '81 KB',
82+
limit: '82 KB',
8383
},
8484
{
8585
name: '@sentry/browser (incl. Tracing, Replay, Feedback)',

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

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,10 @@ export function loggingTransport(_options: BaseTransportOptions): Transport {
2525
* Setting this port to something specific is useful for local debugging but dangerous for
2626
* CI/CD environments where port collisions can cause flakes!
2727
*/
28-
export function startExpressServerAndSendPortToRunner(app: Express, port: number | undefined = undefined): void {
28+
export function startExpressServerAndSendPortToRunner(
29+
app: Pick<Express, 'listen'>,
30+
port: number | undefined = undefined,
31+
): void {
2932
const server = app.listen(port || 0, () => {
3033
const address = server.address() as AddressInfo;
3134

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

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
import * as Sentry from '@sentry/node';
2+
import { loggingTransport } from '@sentry-internal/node-integration-tests';
3+
4+
Sentry.init({
5+
dsn: 'https://public@dsn.ingest.sentry.io/1337',
6+
release: '1.0',
7+
// disable attaching headers to /test/* endpoints
8+
tracePropagationTargets: [/^(?!.*test).*$/],
9+
tracesSampleRate: 1.0,
10+
transport: loggingTransport,
11+
});

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

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,40 @@
1+
import * as Sentry from '@sentry/node';
2+
import { startExpressServerAndSendPortToRunner } from '@sentry-internal/node-integration-tests';
3+
import bodyParser from 'body-parser';
4+
import cors from 'cors';
5+
import express from 'express';
6+
7+
const app = express();
8+
9+
app.use(cors());
10+
app.use(bodyParser.json());
11+
app.use(bodyParser.text());
12+
app.use(bodyParser.raw());
13+
14+
app.get('/', (_req, res) => {
15+
res.send({ response: 'response 0' });
16+
});
17+
18+
app.get('/test/express', (_req, res) => {
19+
res.send({ response: 'response 1' });
20+
});
21+
22+
app.get(/\/test\/regex/, (_req, res) => {
23+
res.send({ response: 'response 2' });
24+
});
25+
26+
app.get(['/test/array1', /\/test\/array[2-9]/], (_req, res) => {
27+
res.send({ response: 'response 3' });
28+
});
29+
30+
app.get(['/test/arr/:id', /\/test\/arr[0-9]*\/required(path)?(\/optionalPath)?\/(lastParam)?/], (_req, res) => {
31+
res.send({ response: 'response 4' });
32+
});
33+
34+
app.post('/test-post', function (req, res) {
35+
res.send({ status: 'ok', body: req.body });
36+
});
37+
38+
Sentry.setupExpressErrorHandler(app);
39+
40+
startExpressServerAndSendPortToRunner(app);

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

Lines changed: 54 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,14 @@
1-
import { afterAll, describe, expect, test } from 'vitest';
2-
import { cleanupChildProcesses, createRunner } from '../../../utils/runner';
1+
import { afterAll, describe, expect } from 'vitest';
2+
import { cleanupChildProcesses, createEsmAndCjsTests } from '../../../utils/runner';
33

4-
describe('express tracing', () => {
4+
describe('express v5 tracing', () => {
55
afterAll(() => {
66
cleanupChildProcesses();
77
});
88

9-
describe('CJS', () => {
9+
createEsmAndCjsTests(__dirname, 'scenario.mjs', 'instrument.mjs', (createRunner, test) => {
1010
test('should create and send transactions for Express routes and spans for middlewares.', async () => {
11-
const runner = createRunner(__dirname, 'server.js')
11+
const runner = createRunner()
1212
.expect({
1313
transaction: {
1414
contexts: {
@@ -51,7 +51,7 @@ describe('express tracing', () => {
5151
});
5252

5353
test('should set a correct transaction name for routes specified in RegEx', async () => {
54-
const runner = createRunner(__dirname, 'server.js')
54+
const runner = createRunner()
5555
.expect({
5656
transaction: {
5757
transaction: 'GET /\\/test\\/regex/',
@@ -77,10 +77,52 @@ describe('express tracing', () => {
7777
await runner.completed();
7878
});
7979

80+
test('handles root page correctly', async () => {
81+
const runner = createRunner()
82+
.expect({
83+
transaction: {
84+
transaction: 'GET /',
85+
},
86+
})
87+
.start();
88+
runner.makeRequest('get', '/');
89+
await runner.completed();
90+
});
91+
92+
test('handles 404 page correctly', async () => {
93+
const runner = createRunner()
94+
.expect({
95+
transaction: {
96+
// FIXME: This is wrong :(
97+
transaction: 'GET /',
98+
contexts: {
99+
trace: {
100+
span_id: expect.stringMatching(/[a-f0-9]{16}/),
101+
trace_id: expect.stringMatching(/[a-f0-9]{32}/),
102+
data: {
103+
'http.response.status_code': 404,
104+
url: expect.stringMatching(/\/does-not-exist$/),
105+
'http.method': 'GET',
106+
// FIXME: This is wrong :(
107+
'http.route': '/',
108+
'http.url': expect.stringMatching(/\/does-not-exist$/),
109+
'http.target': '/does-not-exist',
110+
},
111+
op: 'http.server',
112+
status: 'not_found',
113+
},
114+
},
115+
},
116+
})
117+
.start();
118+
runner.makeRequest('get', '/does-not-exist', { expectError: true });
119+
await runner.completed();
120+
});
121+
80122
test.each([['array1'], ['array5']])(
81123
'should set a correct transaction name for routes consisting of arrays of routes for %p',
82124
async (segment: string) => {
83-
const runner = await createRunner(__dirname, 'server.js')
125+
const runner = await createRunner()
84126
.expect({
85127
transaction: {
86128
transaction: 'GET /test/array1,/\\/test\\/array[2-9]/',
@@ -115,7 +157,7 @@ describe('express tracing', () => {
115157
['arr/required/lastParam'],
116158
['arr55/required/lastParam'],
117159
])('should handle more complex regexes in route arrays correctly for %p', async (segment: string) => {
118-
const runner = await createRunner(__dirname, 'server.js')
160+
const runner = await createRunner()
119161
.expect({
120162
transaction: {
121163
transaction: 'GET /test/arr/:id,/\\/test\\/arr[0-9]*\\/required(path)?(\\/optionalPath)?\\/(lastParam)?/',
@@ -143,7 +185,7 @@ describe('express tracing', () => {
143185

144186
describe('request data', () => {
145187
test('correctly captures JSON request data', async () => {
146-
const runner = createRunner(__dirname, 'server.js')
188+
const runner = createRunner()
147189
.expect({
148190
transaction: {
149191
transaction: 'POST /test-post',
@@ -173,7 +215,7 @@ describe('express tracing', () => {
173215
});
174216

175217
test('correctly captures plain text request data', async () => {
176-
const runner = createRunner(__dirname, 'server.js')
218+
const runner = createRunner()
177219
.expect({
178220
transaction: {
179221
transaction: 'POST /test-post',
@@ -198,7 +240,7 @@ describe('express tracing', () => {
198240
});
199241

200242
test('correctly captures text buffer request data', async () => {
201-
const runner = createRunner(__dirname, 'server.js')
243+
const runner = createRunner()
202244
.expect({
203245
transaction: {
204246
transaction: 'POST /test-post',
@@ -223,7 +265,7 @@ describe('express tracing', () => {
223265
});
224266

225267
test('correctly captures non-text buffer request data', async () => {
226-
const runner = createRunner(__dirname, 'server.js')
268+
const runner = createRunner()
227269
.expect({
228270
transaction: {
229271
transaction: 'POST /test-post',

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ import { SEMANTIC_ATTRIBUTE_SENTRY_SOURCE } from '@sentry/node';
33
import { afterAll, describe, expect, test } from 'vitest';
44
import { cleanupChildProcesses, createRunner } from '../../../../utils/runner';
55

6-
describe('express tracing', () => {
6+
describe('express v5 tracing', () => {
77
afterAll(() => {
88
cleanupChildProcesses();
99
});

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
import { afterAll, describe, test } from 'vitest';
22
import { cleanupChildProcesses, createRunner } from '../../../../utils/runner';
33

4-
describe('express tracing experimental', () => {
4+
describe('express v5 tracing', () => {
55
afterAll(() => {
66
cleanupChildProcesses();
77
});

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

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
{
2+
"extends": "../tsconfig.json"
3+
}

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

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
import * as Sentry from '@sentry/node';
2+
import { loggingTransport } from '@sentry-internal/node-integration-tests';
3+
4+
Sentry.init({
5+
dsn: 'https://public@dsn.ingest.sentry.io/1337',
6+
release: '1.0',
7+
// disable attaching headers to /test/* endpoints
8+
tracePropagationTargets: [/^(?!.*test).*$/],
9+
tracesSampleRate: 1.0,
10+
transport: loggingTransport,
11+
integrations: [
12+
Sentry.httpIntegration({
13+
ignoreIncomingRequestBody: url => {
14+
if (url.includes('/test-post-ignore-body')) {
15+
return true;
16+
}
17+
return false;
18+
},
19+
}),
20+
],
21+
});

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

Lines changed: 13 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -1,20 +1,8 @@
1-
const { loggingTransport } = require('@sentry-internal/node-integration-tests');
2-
const Sentry = require('@sentry/node');
3-
4-
Sentry.init({
5-
dsn: 'https://public@dsn.ingest.sentry.io/1337',
6-
release: '1.0',
7-
// disable attaching headers to /test/* endpoints
8-
tracePropagationTargets: [/^(?!.*test).*$/],
9-
tracesSampleRate: 1.0,
10-
transport: loggingTransport,
11-
});
12-
13-
// express must be required after Sentry is initialized
14-
const express = require('express');
15-
const cors = require('cors');
16-
const bodyParser = require('body-parser');
17-
const { startExpressServerAndSendPortToRunner } = require('@sentry-internal/node-integration-tests');
1+
import * as Sentry from '@sentry/node';
2+
import { startExpressServerAndSendPortToRunner } from '@sentry-internal/node-integration-tests';
3+
import bodyParser from 'body-parser';
4+
import cors from 'cors';
5+
import express from 'express';
186

197
const app = express();
208

@@ -23,6 +11,10 @@ app.use(bodyParser.json());
2311
app.use(bodyParser.text());
2412
app.use(bodyParser.raw());
2513

14+
app.get('/', (_req, res) => {
15+
res.send({ response: 'response 0' });
16+
});
17+
2618
app.get('/test/express', (_req, res) => {
2719
res.send({ response: 'response 1' });
2820
});
@@ -43,6 +35,10 @@ app.post('/test-post', function (req, res) {
4335
res.send({ status: 'ok', body: req.body });
4436
});
4537

38+
app.post('/test-post-ignore-body', function (req, res) {
39+
res.send({ status: 'ok', body: req.body });
40+
});
41+
4642
Sentry.setupExpressErrorHandler(app);
4743

4844
startExpressServerAndSendPortToRunner(app);

0 commit comments

Comments
 (0)