Skip to content

Commit 89377da

Browse files
authored
test(node): Add test for errors-only ESM app (#12046)
This tests a node app that uses ESM, but no `--import` flag. Somehow this works for `http` (but not other packages...) but this is fine for errors-only mode, for now. Missing: We do show the warning for missing express instrumentation there, still 😬 we may need to tweak this...
1 parent ef26423 commit 89377da

File tree

8 files changed

+191
-0
lines changed

8 files changed

+191
-0
lines changed

.github/workflows/build.yml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1004,6 +1004,7 @@ jobs:
10041004
'create-remix-app-express-vite-dev',
10051005
'debug-id-sourcemaps',
10061006
'node-express-esm-loader',
1007+
'node-express-esm-without-loader',
10071008
'nextjs-app-dir',
10081009
'nextjs-14',
10091010
'react-create-hash-router',
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: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
{
2+
"name": "node-express-esm-without-loader",
3+
"version": "1.0.0",
4+
"private": true,
5+
"scripts": {
6+
"start": "node src/app.mjs",
7+
"clean": "npx rimraf node_modules pnpm-lock.yaml",
8+
"test:build": "pnpm install",
9+
"test:assert": "playwright test"
10+
},
11+
"dependencies": {
12+
"@sentry/node": "latest || *",
13+
"@sentry/opentelemetry": "latest || *",
14+
"express": "4.19.2"
15+
},
16+
"devDependencies": {
17+
"@sentry-internal/event-proxy-server": "link:../../../event-proxy-server",
18+
"@playwright/test": "^1.27.1"
19+
},
20+
"volta": {
21+
"extends": "../../package.json",
22+
"node": "18.19.1"
23+
}
24+
}
Lines changed: 70 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,70 @@
1+
import type { PlaywrightTestConfig } from '@playwright/test';
2+
import { devices } from '@playwright/test';
3+
4+
// Fix urls not resolving to localhost on Node v17+
5+
// See: https://github.com/axios/axios/issues/3821#issuecomment-1413727575
6+
import { setDefaultResultOrder } from 'dns';
7+
setDefaultResultOrder('ipv4first');
8+
9+
const eventProxyPort = 3031;
10+
const expressPort = 3030;
11+
12+
/**
13+
* See https://playwright.dev/docs/test-configuration.
14+
*/
15+
const config: PlaywrightTestConfig = {
16+
testDir: './tests',
17+
/* Maximum time one test can run for. */
18+
timeout: 150_000,
19+
expect: {
20+
/**
21+
* Maximum time expect() should wait for the condition to be met.
22+
* For example in `await expect(locator).toHaveText();`
23+
*/
24+
timeout: 5000,
25+
},
26+
/* Run tests in files in parallel */
27+
fullyParallel: true,
28+
/* Fail the build on CI if you accidentally left test.only in the source code. */
29+
forbidOnly: !!process.env.CI,
30+
/* Retry on CI only */
31+
retries: 0,
32+
/* Reporter to use. See https://playwright.dev/docs/test-reporters */
33+
reporter: 'list',
34+
/* Shared settings for all the projects below. See https://playwright.dev/docs/api/class-testoptions. */
35+
use: {
36+
/* Maximum time each action such as `click()` can take. Defaults to 0 (no limit). */
37+
actionTimeout: 0,
38+
39+
/* Base URL to use in actions like `await page.goto('/')`. */
40+
baseURL: `http://localhost:${expressPort}`,
41+
},
42+
43+
/* Configure projects for major browsers */
44+
projects: [
45+
{
46+
name: 'chromium',
47+
use: {
48+
...devices['Desktop Chrome'],
49+
},
50+
},
51+
],
52+
53+
/* Run your local dev server before starting the tests */
54+
webServer: [
55+
{
56+
command: 'node start-event-proxy.mjs',
57+
port: eventProxyPort,
58+
stdout: 'pipe',
59+
stderr: 'pipe',
60+
},
61+
{
62+
command: 'pnpm start',
63+
port: expressPort,
64+
stdout: 'pipe',
65+
stderr: 'pipe',
66+
},
67+
],
68+
};
69+
70+
export default config;
Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,46 @@
1+
import './instrument.mjs';
2+
3+
// Below other imports
4+
import * as Sentry from '@sentry/node';
5+
import express from 'express';
6+
7+
const app = express();
8+
const port = 3030;
9+
10+
app.get('/test-success', function (req, res) {
11+
setTimeout(() => {
12+
res.status(200).end();
13+
}, 100);
14+
});
15+
16+
app.get('/test-params/:param', function (req, res) {
17+
const { param } = req.params;
18+
Sentry.setTag(`param-${param}`, 'yes');
19+
Sentry.captureException(new Error(`Error for param ${param}`));
20+
21+
setTimeout(() => {
22+
res.status(200).end();
23+
}, 100);
24+
});
25+
26+
app.get('/test-error', function (req, res) {
27+
Sentry.captureException(new Error('This is an error'));
28+
setTimeout(() => {
29+
Sentry.flush(2000).then(() => {
30+
res.status(200).end();
31+
});
32+
}, 100);
33+
});
34+
35+
Sentry.setupExpressErrorHandler(app);
36+
37+
app.use(function onError(err, req, res, next) {
38+
// The error id is attached to `res.sentry` to be returned
39+
// and optionally displayed to the user for support.
40+
res.statusCode = 500;
41+
res.end(res.sentry + '\n');
42+
});
43+
44+
app.listen(port, () => {
45+
console.log(`Example app listening on port ${port}`);
46+
});
Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
import * as Sentry from '@sentry/node';
2+
3+
Sentry.init({
4+
environment: 'qa', // dynamic sampling bias to keep transactions
5+
dsn: process.env.E2E_TEST_DSN,
6+
tunnel: `http://localhost:3031/`, // proxy server
7+
});
Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
import { startEventProxyServer } from '@sentry-internal/event-proxy-server';
2+
3+
startEventProxyServer({
4+
port: 3031,
5+
proxyServerName: 'node-express-esm-without-loader',
6+
});
Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
import { expect, test } from '@playwright/test';
2+
import { waitForError } from '@sentry-internal/event-proxy-server';
3+
4+
test('Should record exceptions captured inside handlers', async ({ request }) => {
5+
const errorEventPromise = waitForError('node-express-esm-without-loader', errorEvent => {
6+
return !!errorEvent?.exception?.values?.[0]?.value?.includes('This is an error');
7+
});
8+
9+
await request.get('/test-error');
10+
11+
await expect(errorEventPromise).resolves.toBeDefined();
12+
});
13+
14+
test('Isolates requests', async ({ request }) => {
15+
const errorEventPromise = waitForError('node-express-esm-without-loader', errorEvent => {
16+
return !!errorEvent?.exception?.values?.[0]?.value?.includes('Error for param 1');
17+
});
18+
19+
const errorEventPromise2 = waitForError('node-express-esm-without-loader', errorEvent => {
20+
return !!errorEvent?.exception?.values?.[0]?.value?.includes('Error for param 2');
21+
});
22+
23+
await request.get('/test-params/1');
24+
await request.get('/test-params/2');
25+
26+
const errorEvent1 = await errorEventPromise;
27+
const errorEvent2 = await errorEventPromise2;
28+
29+
expect(errorEvent1.tags).toEqual({ 'param-1': 'yes' });
30+
expect(errorEvent2.tags).toEqual({ 'param-2': 'yes' });
31+
32+
// Transaction is not set, since we have no expressIntegration by default
33+
expect(errorEvent1.transaction).toBeUndefined();
34+
expect(errorEvent2.transaction).toBeUndefined();
35+
});

0 commit comments

Comments
 (0)