Skip to content

Commit 5faef85

Browse files
authored
test(node): Replace express v5 integration tests with E2E test (#17147)
We used to duplicate all the express integration tests and run yarn separately for them to test express v5. This is a bit unfortunate and also hard to keep in sync (already we added/changed tests in express v4 but not v5). This PR replaces the express v5 stuff with a dedicated express-v5 e2e test - we actually did not have any of that so far :O I thought about using v5 by default and having an express-v4 e2e test app instead, but express v4 is still vastly more in use than v5, so that's probably easier.
1 parent 9c47395 commit 5faef85

File tree

77 files changed

+721
-2718
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

77 files changed

+721
-2718
lines changed
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
dist
Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
@sentry:registry=http://127.0.0.1:4873
2+
@sentry-internal:registry=http://127.0.0.1:4873
Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
{
2+
"name": "node-express-v5-app",
3+
"version": "1.0.0",
4+
"private": true,
5+
"scripts": {
6+
"build": "tsc",
7+
"start": "node dist/app.js",
8+
"test": "playwright test",
9+
"clean": "npx rimraf node_modules pnpm-lock.yaml",
10+
"test:build": "pnpm install && pnpm build",
11+
"test:assert": "pnpm test"
12+
},
13+
"dependencies": {
14+
"@modelcontextprotocol/sdk": "^1.10.2",
15+
"@sentry/node": "latest || *",
16+
"@trpc/server": "10.45.2",
17+
"@trpc/client": "10.45.2",
18+
"@types/express": "^4.17.21",
19+
"@types/node": "^18.19.1",
20+
"express": "^5.1.0",
21+
"typescript": "~5.0.0",
22+
"zod": "~3.24.3"
23+
},
24+
"devDependencies": {
25+
"@playwright/test": "~1.53.2",
26+
"@sentry-internal/test-utils": "link:../../../test-utils",
27+
"@sentry/core": "latest || *"
28+
},
29+
"resolutions": {
30+
"@types/qs": "6.9.17"
31+
},
32+
"volta": {
33+
"extends": "../../package.json"
34+
}
35+
}
Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
import { getPlaywrightConfig } from '@sentry-internal/test-utils';
2+
3+
const config = getPlaywrightConfig({
4+
startCommand: `pnpm start`,
5+
});
6+
7+
export default config;
Lines changed: 152 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,152 @@
1+
import * as Sentry from '@sentry/node';
2+
3+
declare global {
4+
namespace globalThis {
5+
var transactionIds: string[];
6+
}
7+
}
8+
9+
Sentry.init({
10+
environment: 'qa', // dynamic sampling bias to keep transactions
11+
dsn: process.env.E2E_TEST_DSN,
12+
includeLocalVariables: true,
13+
debug: !!process.env.DEBUG,
14+
tunnel: `http://localhost:3031/`, // proxy server
15+
tracesSampleRate: 1,
16+
enableLogs: true,
17+
});
18+
19+
import { TRPCError, initTRPC } from '@trpc/server';
20+
import * as trpcExpress from '@trpc/server/adapters/express';
21+
import express from 'express';
22+
import { z } from 'zod';
23+
import { mcpRouter } from './mcp';
24+
25+
const app = express();
26+
const port = 3030;
27+
28+
app.use(mcpRouter);
29+
30+
app.get('/crash-in-with-monitor/:id', async (req, res) => {
31+
try {
32+
await Sentry.withMonitor('express-crash', async () => {
33+
throw new Error(`This is an exception withMonitor: ${req.params.id}`);
34+
});
35+
res.sendStatus(200);
36+
} catch (error: any) {
37+
res.status(500);
38+
res.send({ message: error.message, pid: process.pid });
39+
}
40+
});
41+
42+
app.get('/test-success', function (req, res) {
43+
res.send({ version: 'v1' });
44+
});
45+
46+
app.get('/test-log', function (req, res) {
47+
Sentry.logger.debug('Accessed /test-log route');
48+
res.send({ message: 'Log sent' });
49+
});
50+
51+
app.get('/test-param/:param', function (req, res) {
52+
res.send({ paramWas: req.params.param });
53+
});
54+
55+
app.get('/test-transaction', function (_req, res) {
56+
Sentry.startSpan({ name: 'test-span' }, () => undefined);
57+
58+
res.send({ status: 'ok' });
59+
});
60+
61+
app.get('/test-error', async function (req, res) {
62+
const exceptionId = Sentry.captureException(new Error('This is an error'));
63+
64+
await Sentry.flush(2000);
65+
66+
res.send({ exceptionId });
67+
});
68+
69+
app.get('/test-exception/:id', function (req, _res) {
70+
throw new Error(`This is an exception with id ${req.params.id}`);
71+
});
72+
73+
app.get('/test-local-variables-uncaught', function (req, res) {
74+
const randomVariableToRecord = Math.random();
75+
throw new Error(`Uncaught Local Variable Error - ${JSON.stringify({ randomVariableToRecord })}`);
76+
});
77+
78+
app.get('/test-local-variables-caught', function (req, res) {
79+
const randomVariableToRecord = Math.random();
80+
81+
let exceptionId: string;
82+
try {
83+
throw new Error('Local Variable Error');
84+
} catch (e) {
85+
exceptionId = Sentry.captureException(e);
86+
}
87+
88+
res.send({ exceptionId, randomVariableToRecord });
89+
});
90+
91+
Sentry.setupExpressErrorHandler(app);
92+
93+
// @ts-ignore
94+
app.use(function onError(err, req, res, next) {
95+
// The error id is attached to `res.sentry` to be returned
96+
// and optionally displayed to the user for support.
97+
res.statusCode = 500;
98+
res.end(res.sentry + '\n');
99+
});
100+
101+
app.listen(port, () => {
102+
console.log(`Example app listening on port ${port}`);
103+
});
104+
105+
Sentry.addEventProcessor(event => {
106+
global.transactionIds = global.transactionIds || [];
107+
108+
if (event.type === 'transaction') {
109+
const eventId = event.event_id;
110+
111+
if (eventId) {
112+
global.transactionIds.push(eventId);
113+
}
114+
}
115+
116+
return event;
117+
});
118+
119+
export const t = initTRPC.context<Context>().create();
120+
121+
const procedure = t.procedure.use(Sentry.trpcMiddleware({ attachRpcInput: true }));
122+
123+
export const appRouter = t.router({
124+
getSomething: procedure.input(z.string()).query(opts => {
125+
return { id: opts.input, name: 'Bilbo' };
126+
}),
127+
createSomething: procedure.mutation(async () => {
128+
await new Promise(resolve => setTimeout(resolve, 400));
129+
return { success: true };
130+
}),
131+
crashSomething: procedure
132+
.input(z.object({ nested: z.object({ nested: z.object({ nested: z.string() }) }) }))
133+
.mutation(() => {
134+
throw new Error('I crashed in a trpc handler');
135+
}),
136+
badRequest: procedure.mutation(() => {
137+
throw new TRPCError({ code: 'BAD_REQUEST', cause: new Error('Bad Request') });
138+
}),
139+
});
140+
141+
export type AppRouter = typeof appRouter;
142+
143+
const createContext = () => ({ someStaticValue: 'asdf' });
144+
type Context = Awaited<ReturnType<typeof createContext>>;
145+
146+
app.use(
147+
'/trpc',
148+
trpcExpress.createExpressMiddleware({
149+
router: appRouter,
150+
createContext,
151+
}),
152+
);
Lines changed: 64 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,64 @@
1+
import express from 'express';
2+
import { McpServer, ResourceTemplate } from '@modelcontextprotocol/sdk/server/mcp.js';
3+
import { SSEServerTransport } from '@modelcontextprotocol/sdk/server/sse.js';
4+
import { z } from 'zod';
5+
import { wrapMcpServerWithSentry } from '@sentry/node';
6+
7+
const mcpRouter = express.Router();
8+
9+
const server = wrapMcpServerWithSentry(
10+
new McpServer({
11+
name: 'Echo',
12+
version: '1.0.0',
13+
}),
14+
);
15+
16+
server.resource('echo', new ResourceTemplate('echo://{message}', { list: undefined }), async (uri, { message }) => ({
17+
contents: [
18+
{
19+
uri: uri.href,
20+
text: `Resource echo: ${message}`,
21+
},
22+
],
23+
}));
24+
25+
server.tool('echo', { message: z.string() }, async ({ message }, rest) => {
26+
return {
27+
content: [{ type: 'text', text: `Tool echo: ${message}` }],
28+
};
29+
});
30+
31+
server.prompt('echo', { message: z.string() }, ({ message }, extra) => ({
32+
messages: [
33+
{
34+
role: 'user',
35+
content: {
36+
type: 'text',
37+
text: `Please process this message: ${message}`,
38+
},
39+
},
40+
],
41+
}));
42+
43+
const transports: Record<string, SSEServerTransport> = {};
44+
45+
mcpRouter.get('/sse', async (_, res) => {
46+
const transport = new SSEServerTransport('/messages', res);
47+
transports[transport.sessionId] = transport;
48+
res.on('close', () => {
49+
delete transports[transport.sessionId];
50+
});
51+
await server.connect(transport);
52+
});
53+
54+
mcpRouter.post('/messages', async (req, res) => {
55+
const sessionId = req.query.sessionId;
56+
const transport = transports[sessionId as string];
57+
if (transport) {
58+
await transport.handlePostMessage(req, res);
59+
} else {
60+
res.status(400).send('No transport found for sessionId');
61+
}
62+
});
63+
64+
export { mcpRouter };
Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
import { startEventProxyServer } from '@sentry-internal/test-utils';
2+
3+
startEventProxyServer({
4+
port: 3031,
5+
proxyServerName: 'node-express-v5',
6+
});
Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,53 @@
1+
import { expect, test } from '@playwright/test';
2+
import { waitForError } from '@sentry-internal/test-utils';
3+
4+
test('Sends correct error event', async ({ baseURL }) => {
5+
const errorEventPromise = waitForError('node-express-v5', event => {
6+
return !event.type && event.exception?.values?.[0]?.value === 'This is an exception with id 123';
7+
});
8+
9+
await fetch(`${baseURL}/test-exception/123`);
10+
11+
const errorEvent = await errorEventPromise;
12+
13+
expect(errorEvent.exception?.values).toHaveLength(1);
14+
expect(errorEvent.exception?.values?.[0]?.value).toBe('This is an exception with id 123');
15+
16+
expect(errorEvent.request).toEqual({
17+
method: 'GET',
18+
cookies: {},
19+
headers: expect.any(Object),
20+
url: 'http://localhost:3030/test-exception/123',
21+
});
22+
23+
expect(errorEvent.transaction).toEqual('GET /test-exception/:id');
24+
25+
expect(errorEvent.contexts?.trace).toEqual({
26+
trace_id: expect.stringMatching(/[a-f0-9]{32}/),
27+
span_id: expect.stringMatching(/[a-f0-9]{16}/),
28+
});
29+
});
30+
31+
test('Should record caught exceptions with local variable', async ({ baseURL }) => {
32+
const errorEventPromise = waitForError('node-express-v5', event => {
33+
return event.transaction === 'GET /test-local-variables-caught';
34+
});
35+
36+
await fetch(`${baseURL}/test-local-variables-caught`);
37+
38+
const errorEvent = await errorEventPromise;
39+
40+
const frames = errorEvent.exception?.values?.[0]?.stacktrace?.frames;
41+
expect(frames?.[frames.length - 1]?.vars?.randomVariableToRecord).toBeDefined();
42+
});
43+
44+
test('To not crash app from withMonitor', async ({ baseURL }) => {
45+
const doRequest = async (id: number) => {
46+
const response = await fetch(`${baseURL}/crash-in-with-monitor/${id}`)
47+
return response.json();
48+
}
49+
const [response1, response2] = await Promise.all([doRequest(1), doRequest(2)])
50+
expect(response1.message).toBe('This is an exception withMonitor: 1')
51+
expect(response2.message).toBe('This is an exception withMonitor: 2')
52+
expect(response1.pid).toBe(response2.pid) //Just to double-check, TBS
53+
});
Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
import { expect, test } from '@playwright/test';
2+
import { waitForEnvelopeItem } from '@sentry-internal/test-utils';
3+
import type { SerializedLogContainer } from '@sentry/core';
4+
5+
test('should send logs', async ({ baseURL }) => {
6+
const logEnvelopePromise = waitForEnvelopeItem('node-express-v5', envelope => {
7+
return envelope[0].type === 'log' && (envelope[1] as SerializedLogContainer).items[0]?.level === 'debug';
8+
});
9+
10+
await fetch(`${baseURL}/test-log`);
11+
12+
const logEnvelope = await logEnvelopePromise;
13+
const log = (logEnvelope[1] as SerializedLogContainer).items[0];
14+
expect(log?.level).toBe('debug');
15+
expect(log?.body).toBe('Accessed /test-log route');
16+
});

0 commit comments

Comments
 (0)