test(node): Add utility to test esm & cjs instrumentation (#16159)

Today, we do not have a lot of esm-specific node integration tests. Our
tests make it possible to test ESM, but we only use it very rarely,
sticking to cjs tests across the suite mostly. This means that we have a
bunch of gaps around our ESM support.

This PR introduces a new test utility to make it easier to test stuff in
ESM and CJS:

```js
createEsmAndCjsTests(
    __dirname,
    'scenario.mjs',
    'instrument.mjs',
    (createRunner, test) => {
      test('it works when importing the http module', async () => {
        const runner = createRunner(); // normal test as before
       });
});
```

This has a few limitations based on how this works - we can make this
more robust in the future, but for now it should be "OK":
1. It requires a .mjs based instrument file as well as an .mjs based
scenario
2. No relative imports are supported (all content must be in these two
files)
3. It simply regex replaces the esm imports with require for the CJS
tests. not perfect, but it kind of works

For tests that are known to fail on e.g. esm or cjs, you can configure
`failsOnEsm: true`. In this case, it will fail if the test _does not
fail_ (by using `test.fails()` to ensure test failure). This way we can
ensure we find out if stuff starts to fail etc.

To make this work, I had to re-write the test runner code a bit, because
it had issues with vitest unhandled rejection handling. Due to the way
we handled test completion with a promise, `test.fails` was not working
as expected, because the test indeed succeeded when it failed, but the
overall test run still failed because an unhandled rejection bubbled up.
Now, this should work as expected...

I re-wrote a few tests already to show how it works, plus added a new
test that shows an ESM test failure when importing http module (😭 )

Author: mydea

dev-packages/node-integration-tests/.eslintrc.js

@@ -12,10 +12,11 @@ module.exports = {
       },
     },
     {
-      files: ['suites/**/*.ts'],
+      files: ['suites/**/*.ts', 'suites/**/*.mjs'],
       parserOptions: {
         project: ['tsconfig.test.json'],
         sourceType: 'module',
+        ecmaVersion: 'latest',
       },
       rules: {
         '@typescript-eslint/typedef': 'off',

dev-packages/node-integration-tests/.gitignore

@@ -0,0 +1 @@
+suites/**/tmp_*

dev-packages/node-integration-tests/suites/anr/app-path.mjs

@@ -1,10 +1,9 @@
+import * as Sentry from '@sentry/node';
 import * as assert from 'assert';
 import * as crypto from 'crypto';
 import * as path from 'path';
 import * as url from 'url';
 
-import * as Sentry from '@sentry/node';
-
 global._sentryDebugIds = { [new Error().stack]: 'aaaaaaaa-aaaa-4aaa-aaaa-aaaaaaaaaa' };
 
 const __dirname = path.dirname(url.fileURLToPath(import.meta.url));

dev-packages/node-integration-tests/suites/anr/basic-multiple.mjs

@@ -1,8 +1,7 @@
+import * as Sentry from '@sentry/node';
 import * as assert from 'assert';
 import * as crypto from 'crypto';
 
-import * as Sentry from '@sentry/node';
-
 global._sentryDebugIds = { [new Error().stack]: 'aaaaaaaa-aaaa-4aaa-aaaa-aaaaaaaaaa' };
 
 setTimeout(() => {

dev-packages/node-integration-tests/suites/anr/basic.mjs

@@ -1,8 +1,7 @@
+import * as Sentry from '@sentry/node';
 import * as assert from 'assert';
 import * as crypto from 'crypto';
 
-import * as Sentry from '@sentry/node';
-
 global._sentryDebugIds = { [new Error().stack]: 'aaaaaaaa-aaaa-4aaa-aaaa-aaaaaaaaaa' };
 
 setTimeout(() => {

dev-packages/node-integration-tests/suites/anr/indefinite.mjs

@@ -1,8 +1,7 @@
+import * as Sentry from '@sentry/node';
 import * as assert from 'assert';
 import * as crypto from 'crypto';
 
-import * as Sentry from '@sentry/node';
-
 setTimeout(() => {
   process.exit();
 }, 10000);

dev-packages/node-integration-tests/suites/anr/isolated.mjs

@@ -1,8 +1,7 @@
+import * as Sentry from '@sentry/node';
 import * as assert from 'assert';
 import * as crypto from 'crypto';
 
-import * as Sentry from '@sentry/node';
-
 setTimeout(() => {
   process.exit();
 }, 10000);

dev-packages/node-integration-tests/suites/anr/test.ts

@@ -107,7 +107,7 @@ const ANR_EVENT_WITH_DEBUG_META: Event = {
   },
 };
 
-describe('should report ANR when event loop blocked', { timeout: 60_000 }, () => {
+describe('should report ANR when event loop blocked', { timeout: 90_000 }, () => {
   afterAll(() => {
     cleanupChildProcesses();
   });

dev-packages/node-integration-tests/suites/breadcrumbs/process-thread/app.mjs

@@ -1,7 +1,7 @@
+import * as Sentry from '@sentry/node';
+import { loggingTransport } from '@sentry-internal/node-integration-tests';
 import { spawn } from 'child_process';
 import { join } from 'path';
-import { loggingTransport } from '@sentry-internal/node-integration-tests';
-import * as Sentry from '@sentry/node';
 import { Worker } from 'worker_threads';
 
 const __dirname = new URL('.', import.meta.url).pathname;
@@ -13,16 +13,18 @@ Sentry.init({
   transport: loggingTransport,
 });
 
-await new Promise(resolve => {
-  const child = spawn('sleep', ['a']);
-  child.on('error', resolve);
-  child.on('exit', resolve);
-});
+(async () => {
+  await new Promise(resolve => {
+    const child = spawn('sleep', ['a']);
+    child.on('error', resolve);
+    child.on('exit', resolve);
+  });
 
-await new Promise(resolve => {
-  const worker = new Worker(join(__dirname, 'worker.mjs'));
-  worker.on('error', resolve);
-  worker.on('exit', resolve);
-});
+  await new Promise(resolve => {
+    const worker = new Worker(join(__dirname, 'worker.mjs'));
+    worker.on('error', resolve);
+    worker.on('exit', resolve);
+  });
 
-throw new Error('This is a test error');
+  throw new Error('This is a test error');
+})();

dev-packages/node-integration-tests/suites/child-process/fork.js

@@ -10,8 +10,7 @@ Sentry.init({
   transport: loggingTransport,
 });
 
-// eslint-disable-next-line no-unused-vars
-const _child = fork(path.join(__dirname, 'child.mjs'));
+fork(path.join(__dirname, 'child.mjs'));
 
 setTimeout(() => {
   throw new Error('Exiting main process');

dev-packages/node-integration-tests/suites/child-process/fork.mjs

@@ -1,7 +1,7 @@
+import * as Sentry from '@sentry/node';
+import { loggingTransport } from '@sentry-internal/node-integration-tests';
 import { fork } from 'child_process';
 import * as path from 'path';
-import { loggingTransport } from '@sentry-internal/node-integration-tests';
-import * as Sentry from '@sentry/node';
 
 const __dirname = new URL('.', import.meta.url).pathname;
 
@@ -12,7 +12,7 @@ Sentry.init({
   transport: loggingTransport,
 });
 
-const _child = fork(path.join(__dirname, 'child.mjs'));
+fork(path.join(__dirname, 'child.mjs'));
 
 setTimeout(() => {
   throw new Error('Exiting main process');

dev-packages/node-integration-tests/suites/child-process/worker.js

@@ -10,8 +10,7 @@ Sentry.init({
   transport: loggingTransport,
 });
 
-// eslint-disable-next-line no-unused-vars
-const _worker = new Worker(path.join(__dirname, 'child.js'));
+new Worker(path.join(__dirname, 'child.js'));
 
 setTimeout(() => {
   process.exit();

dev-packages/node-integration-tests/suites/child-process/worker.mjs

@@ -1,6 +1,6 @@
-import * as path from 'path';
-import { loggingTransport } from '@sentry-internal/node-integration-tests';
 import * as Sentry from '@sentry/node';
+import { loggingTransport } from '@sentry-internal/node-integration-tests';
+import * as path from 'path';
 import { Worker } from 'worker_threads';
 
 const __dirname = new URL('.', import.meta.url).pathname;
@@ -12,7 +12,7 @@ Sentry.init({
   transport: loggingTransport,
 });
 
-const _worker = new Worker(path.join(__dirname, 'child.mjs'));
+new Worker(path.join(__dirname, 'child.mjs'));
 
 setTimeout(() => {
   process.exit();

dev-packages/node-integration-tests/suites/contextLines/filename-with-spaces/instrument.mjs

@@ -1,5 +1,5 @@
-import { loggingTransport } from '@sentry-internal/node-integration-tests';
 import * as Sentry from '@sentry/node';
+import { loggingTransport } from '@sentry-internal/node-integration-tests';
 
 Sentry.init({
   dsn: 'https://public@dsn.ingest.sentry.io/1337',

dev-packages/node-integration-tests/suites/contextLines/filename-with-spaces/test.ts

@@ -8,7 +8,7 @@ describe('ContextLines integration in ESM', () => {
     const instrumentPath = join(__dirname, 'instrument.mjs');
 
     await createRunner(__dirname, 'scenario with space.mjs')
-      .withFlags('--import', instrumentPath)
+      .withInstrument(instrumentPath)
       .expect({
         event: {
           exception: {

dev-packages/node-integration-tests/suites/esm/import-in-the-middle/app.mjs

@@ -1,5 +1,5 @@
-import { loggingTransport } from '@sentry-internal/node-integration-tests';
 import * as Sentry from '@sentry/node';
+import { loggingTransport } from '@sentry-internal/node-integration-tests';
 import * as iitm from 'import-in-the-middle';
 
 new iitm.Hook((_, name) => {
@@ -14,6 +14,8 @@ Sentry.init({
   transport: loggingTransport,
 });
 
-await import('./sub-module.mjs');
-await import('http');
-await import('os');
+(async () => {
+  await import('./sub-module.mjs');
+  await import('http');
+  await import('os');
+})();

dev-packages/node-integration-tests/suites/esm/modules-integration/app.mjs

@@ -1,5 +1,5 @@
-import { loggingTransport } from '@sentry-internal/node-integration-tests';
 import * as Sentry from '@sentry/node';
+import { loggingTransport } from '@sentry-internal/node-integration-tests';
 
 Sentry.init({
   dsn: 'https://public@dsn.ingest.sentry.io/1337',

dev-packages/node-integration-tests/suites/esm/warn-esm/server.mjs

@@ -1,15 +1,13 @@
-import { loggingTransport } from '@sentry-internal/node-integration-tests';
 import * as Sentry from '@sentry/node';
+import { loggingTransport, startExpressServerAndSendPortToRunner } from '@sentry-internal/node-integration-tests';
+import express from 'express';
 
 Sentry.init({
   dsn: 'https://public@dsn.ingest.sentry.io/1337',
   release: '1.0',
   transport: loggingTransport,
 });
 
-import { startExpressServerAndSendPortToRunner } from '@sentry-internal/node-integration-tests';
-import express from 'express';
-
 const app = express();
 
 app.get('/test/success', (req, res) => {

dev-packages/node-integration-tests/suites/tracing/amqplib/init.ts renamed to dev-packages/node-integration-tests/suites/express/with-http/instrument.mjs

@@ -0,0 +1,28 @@
+import * as Sentry from '@sentry/node';
+import { startExpressServerAndSendPortToRunner } from '@sentry-internal/node-integration-tests';
+import express from 'express';
+import http from 'http';
+
+const app = express();
+
+app.get('/test', (_req, res) => {
+  http.get(`http://localhost:${app.port}/test2`, httpRes => {
+    httpRes.on('data', () => {
+      setTimeout(() => {
+        res.send({ response: 'response 1' });
+      }, 200);
+    });
+  });
+});
+
+app.get('/test2', (_req, res) => {
+  res.send({ response: 'response 2' });
+});
+
+app.get('/test3', (_req, res) => {
+  res.send({ response: 'response 3' });
+});
+
+Sentry.setupExpressErrorHandler(app);
+
+startExpressServerAndSendPortToRunner(app);

dev-packages/node-integration-tests/suites/express/with-http/scenario.mjs

@@ -0,0 +1,41 @@
+import { afterAll, describe } from 'vitest';
+import { cleanupChildProcesses, createEsmAndCjsTests } from '../../../utils/runner';
+
+describe('express with http import', () => {
+  afterAll(() => {
+    cleanupChildProcesses();
+  });
+
+  createEsmAndCjsTests(
+    __dirname,
+    'scenario.mjs',
+    'instrument.mjs',
+    (createRunner, test) => {
+      test('it works when importing the http module', async () => {
+        const runner = createRunner()
+          .expect({
+            transaction: {
+              transaction: 'GET /test2',
+            },
+          })
+          .expect({
+            transaction: {
+              transaction: 'GET /test',
+            },
+          })
+          .expect({
+            transaction: {
+              transaction: 'GET /test3',
+            },
+          })
+          .start();
+        await runner.makeRequest('get', '/test');
+        await runner.makeRequest('get', '/test3');
+        await runner.completed();
+      });
+      // TODO: This is failing on ESM because importing http is triggering the http spans twice :(
+      // We need to fix this!
+    },
+    { failsOnEsm: true },
+  );
+});

dev-packages/node-integration-tests/suites/express/with-http/test.ts

@@ -1,9 +1,5 @@
 import { register } from 'node:module';
 
-const hookScript = Buffer.from(`
-
-  `);
-
 register(
   new URL(`data:application/javascript,
 export async function resolve(specifier, context, nextResolve) {
@@ -16,6 +12,8 @@ export async function resolve(specifier, context, nextResolve) {
   import.meta.url,
 );
 
-const Sentry = await import('@sentry/node');
+(async () => {
+  const Sentry = await import('@sentry/node');
 
-Sentry.init({});
+  Sentry.init({});
+})();

dev-packages/node-integration-tests/suites/public-api/LocalVariables/deny-inspector.mjs

@@ -1,6 +1,6 @@
 /* eslint-disable no-unused-vars */
-import { loggingTransport } from '@sentry-internal/node-integration-tests';
 import * as Sentry from '@sentry/node';
+import { loggingTransport } from '@sentry-internal/node-integration-tests';
 
 Sentry.init({
   dsn: 'https://public@dsn.ingest.sentry.io/1337',