ref(test): Refactor Node integration tests to prevent nock leaks. (#5579)

Currently, Node integration tests initialize nock interceptors on server initialization and end them after a certain timeout. This recently created problems with asynchronous event processors, and caused random cases of envelope leaks between tests.

To resolve it, this approach returns `http.Server` and `nock.Scope` instances back to the test, to eventually be provided to a helper that either collects envelopes or makes requests. The responsibility of ending those two instances is moved to the collection helpers like `getEnvelopeRequest` or `getAPIResponse`.

Tests that do not use those helpers should close the server and nock interceptors manually. 
This way, we ensure that we do not lose the context of the server and the nock interceptors between tests.

Author: onurtemizkan

packages/node-integration-tests/README.md

@@ -33,6 +33,9 @@ A custom server configuration can be used, supplying a script that exports a val
 
 `utils/` contains helpers and Sentry-specific assertions that can be used in (`test.ts`).
 
+`runServer` utility function returns an object containing `url`, [`http.Server`](https://nodejs.org/dist/latest-v16.x/docs/api/http.html#class-httpserver) and [`nock.Scope`](https://github.com/nock/nock#usage) instances for the created server. The `url` property is the base URL for the server. The `http.Server` instance is used to finish the server eventually, and the `nock.Scope` instance is used to bind / unbind interceptors for the test case.
+
+The responsibility of ending the server and interceptor is delegated to the test case. Data collection helpers such as `getEnvelopeRequest` and `getAPIResponse` expect those instances to be available and finish them before their resolution. Test that do not use those helpers will need to end the server and interceptors manually.
 ## Running Tests Locally
 
 Tests can be run locally with:

packages/node-integration-tests/suites/express/handle-error/test.ts

@@ -1,8 +1,8 @@
 import { assertSentryEvent, getEnvelopeRequest, runServer } from '../../../utils/index';
 
 test('should capture and send Express controller error.', async () => {
-  const url = await runServer(__dirname, `${__dirname}/server.ts`);
-  const event = await getEnvelopeRequest(`${url}/express`);
+  const { url, server, scope } = await runServer(__dirname, `${__dirname}/server.ts`);
+  const event = await getEnvelopeRequest({ url: `${url}/express`, server, scope });
 
   expect((event[2] as any).exception.values[0].stacktrace.frames.length).toBeGreaterThan(0);
 

packages/node-integration-tests/suites/express/sentry-trace/baggage-header-assign/test.ts

@@ -4,11 +4,14 @@ import { getAPIResponse, runServer } from '../../../../utils/index';
 import { TestAPIResponse } from '../server';
 
 test('Should not overwrite baggage if the incoming request already has Sentry baggage data.', async () => {
-  const url = await runServer(__dirname, `${path.resolve(__dirname, '..')}/server.ts`);
+  const { url, server, scope } = await runServer(__dirname, `${path.resolve(__dirname, '..')}/server.ts`);
 
-  const response = (await getAPIResponse(new URL(`${url}/express`), {
-    baggage: 'sentry-release=2.0.0,sentry-environment=myEnv',
-  })) as TestAPIResponse;
+  const response = (await getAPIResponse(
+    { url: `${url}/express`, server, scope },
+    {
+      baggage: 'sentry-release=2.0.0,sentry-environment=myEnv',
+    },
+  )) as TestAPIResponse;
 
   expect(response).toBeDefined();
   expect(response).toMatchObject({
@@ -20,12 +23,15 @@ test('Should not overwrite baggage if the incoming request already has Sentry ba
 });
 
 test('Should propagate sentry trace baggage data from an incoming to an outgoing request.', async () => {
-  const url = await runServer(__dirname, `${path.resolve(__dirname, '..')}/server.ts`);
+  const { url, server, scope } = await runServer(__dirname, `${path.resolve(__dirname, '..')}/server.ts`);
 
-  const response = (await getAPIResponse(new URL(`${url}/express`), {
-    'sentry-trace': '',
-    baggage: 'sentry-release=2.0.0,sentry-environment=myEnv,dogs=great',
-  })) as TestAPIResponse;
+  const response = (await getAPIResponse(
+    { url: `${url}/express`, server, scope },
+    {
+      'sentry-trace': '',
+      baggage: 'sentry-release=2.0.0,sentry-environment=myEnv,dogs=great',
+    },
+  )) as TestAPIResponse;
 
   expect(response).toBeDefined();
   expect(response).toMatchObject({
@@ -37,11 +43,14 @@ test('Should propagate sentry trace baggage data from an incoming to an outgoing
 });
 
 test('Should propagate empty baggage if sentry-trace header is present in incoming request but no baggage header', async () => {
-  const url = await runServer(__dirname, `${path.resolve(__dirname, '..')}/server.ts`);
+  const { url, server, scope } = await runServer(__dirname, `${path.resolve(__dirname, '..')}/server.ts`);
 
-  const response = (await getAPIResponse(new URL(`${url}/express`), {
-    'sentry-trace': '',
-  })) as TestAPIResponse;
+  const response = (await getAPIResponse(
+    { url: `${url}/express`, server, scope },
+    {
+      'sentry-trace': '',
+    },
+  )) as TestAPIResponse;
 
   expect(response).toBeDefined();
   expect(response).toMatchObject({
@@ -53,12 +62,15 @@ test('Should propagate empty baggage if sentry-trace header is present in incomi
 });
 
 test('Should propagate empty sentry and ignore original 3rd party baggage entries if sentry-trace header is present', async () => {
-  const url = await runServer(__dirname, `${path.resolve(__dirname, '..')}/server.ts`);
+  const { url, server, scope } = await runServer(__dirname, `${path.resolve(__dirname, '..')}/server.ts`);
 
-  const response = (await getAPIResponse(new URL(`${url}/express`), {
-    'sentry-trace': '',
-    baggage: 'foo=bar',
-  })) as TestAPIResponse;
+  const response = (await getAPIResponse(
+    { url: `${url}/express`, server, scope },
+    {
+      'sentry-trace': '',
+      baggage: 'foo=bar',
+    },
+  )) as TestAPIResponse;
 
   expect(response).toBeDefined();
   expect(response).toMatchObject({
@@ -70,9 +82,9 @@ test('Should propagate empty sentry and ignore original 3rd party baggage entrie
 });
 
 test('Should populate and propagate sentry baggage if sentry-trace header does not exist', async () => {
-  const url = await runServer(__dirname, `${path.resolve(__dirname, '..')}/server.ts`);
+  const { url, server, scope } = await runServer(__dirname, `${path.resolve(__dirname, '..')}/server.ts`);
 
-  const response = (await getAPIResponse(new URL(`${url}/express`), {})) as TestAPIResponse;
+  const response = (await getAPIResponse({ url: `${url}/express`, server, scope }, {})) as TestAPIResponse;
 
   expect(response).toBeDefined();
   expect(response).toMatchObject({
@@ -87,11 +99,14 @@ test('Should populate and propagate sentry baggage if sentry-trace header does n
 });
 
 test('Should populate Sentry and ignore 3rd party content if sentry-trace header does not exist', async () => {
-  const url = await runServer(__dirname, `${path.resolve(__dirname, '..')}/server.ts`);
+  const { url, server, scope } = await runServer(__dirname, `${path.resolve(__dirname, '..')}/server.ts`);
 
-  const response = (await getAPIResponse(new URL(`${url}/express`), {
-    baggage: 'foo=bar,bar=baz',
-  })) as TestAPIResponse;
+  const response = (await getAPIResponse(
+    { url: `${url}/express`, server, scope },
+    {
+      baggage: 'foo=bar,bar=baz',
+    },
+  )) as TestAPIResponse;
 
   expect(response).toBeDefined();
   expect(response).toMatchObject({

packages/node-integration-tests/suites/express/sentry-trace/baggage-header-out-bad-tx-name/test.ts

@@ -4,9 +4,9 @@ import { getAPIResponse, runServer } from '../../../../utils/index';
 import { TestAPIResponse } from '../server';
 
 test('Does not include transaction name if transaction source is not set', async () => {
-  const url = await runServer(__dirname, `${path.resolve(__dirname, '.')}/server.ts`);
+  const { url, server, scope } = await runServer(__dirname, `${path.resolve(__dirname, '.')}/server.ts`);
 
-  const response = (await getAPIResponse(new URL(`${url}/express`))) as TestAPIResponse;
+  const response = (await getAPIResponse({ url: `${url}/express`, server, scope })) as TestAPIResponse;
   const baggageString = response.test_data.baggage;
 
   expect(response).toBeDefined();

packages/node-integration-tests/suites/express/sentry-trace/baggage-header-out/test.ts

@@ -4,9 +4,9 @@ import { getAPIResponse, runServer } from '../../../../utils/index';
 import { TestAPIResponse } from '../server';
 
 test('should attach a `baggage` header to an outgoing request.', async () => {
-  const url = await runServer(__dirname, `${path.resolve(__dirname, '.')}/server.ts`);
+  const { url, server, scope } = await runServer(__dirname, `${path.resolve(__dirname, '.')}/server.ts`);
 
-  const response = (await getAPIResponse(new URL(`${url}/express`))) as TestAPIResponse;
+  const response = (await getAPIResponse({ url: `${url}/express`, server, scope })) as TestAPIResponse;
 
   expect(response).toBeDefined();
   expect(response).toMatchObject({

packages/node-integration-tests/suites/express/sentry-trace/baggage-other-vendors-with-sentry-entries/test.ts

@@ -4,12 +4,15 @@ import { getAPIResponse, runServer } from '../../../../utils/index';
 import { TestAPIResponse } from '../server';
 
 test('should ignore sentry-values in `baggage` header of a third party vendor and overwrite them with incoming DSC', async () => {
-  const url = await runServer(__dirname, `${path.resolve(__dirname, '.')}/server.ts`);
+  const { url, server, scope } = await runServer(__dirname, `${path.resolve(__dirname, '.')}/server.ts`);
 
-  const response = (await getAPIResponse(new URL(`${url}/express`), {
-    'sentry-trace': '',
-    baggage: 'sentry-release=2.1.0,sentry-environment=myEnv',
-  })) as TestAPIResponse;
+  const response = (await getAPIResponse(
+    { url: `${url}/express`, server, scope },
+    {
+      'sentry-trace': '',
+      baggage: 'sentry-release=2.1.0,sentry-environment=myEnv',
+    },
+  )) as TestAPIResponse;
 
   expect(response).toBeDefined();
   expect(response).toMatchObject({
@@ -21,9 +24,9 @@ test('should ignore sentry-values in `baggage` header of a third party vendor an
 });
 
 test('should ignore sentry-values in `baggage` header of a third party vendor and overwrite them with new DSC', async () => {
-  const url = await runServer(__dirname, `${path.resolve(__dirname, '.')}/server.ts`);
+  const { url, server, scope } = await runServer(__dirname, `${path.resolve(__dirname, '.')}/server.ts`);
 
-  const response = (await getAPIResponse(new URL(`${url}/express`), {})) as TestAPIResponse;
+  const response = (await getAPIResponse({ url: `${url}/express`, server, scope }, {})) as TestAPIResponse;
 
   expect(response).toBeDefined();
   expect(response).toMatchObject({

packages/node-integration-tests/suites/express/sentry-trace/baggage-other-vendors/test.ts

@@ -4,12 +4,15 @@ import { getAPIResponse, runServer } from '../../../../utils/index';
 import { TestAPIResponse } from '../server';
 
 test('should merge `baggage` header of a third party vendor with the Sentry DSC baggage items', async () => {
-  const url = await runServer(__dirname, `${path.resolve(__dirname, '.')}/server.ts`);
+  const { url, server, scope } = await runServer(__dirname, `${path.resolve(__dirname, '.')}/server.ts`);
 
-  const response = (await getAPIResponse(new URL(`${url}/express`), {
-    'sentry-trace': '',
-    baggage: 'sentry-release=2.0.0,sentry-environment=myEnv',
-  })) as TestAPIResponse;
+  const response = (await getAPIResponse(
+    { url: `${url}/express`, server, scope },
+    {
+      'sentry-trace': '',
+      baggage: 'sentry-release=2.0.0,sentry-environment=myEnv',
+    },
+  )) as TestAPIResponse;
 
   expect(response).toBeDefined();
   expect(response).toMatchObject({

packages/node-integration-tests/suites/express/sentry-trace/baggage-transaction-name/test.ts

@@ -4,9 +4,9 @@ import { getAPIResponse, runServer } from '../../../../utils/index';
 import { TestAPIResponse } from '../server';
 
 test('Includes transaction in baggage if the transaction name is parameterized', async () => {
-  const url = await runServer(__dirname, `${path.resolve(__dirname, '.')}/server.ts`);
+  const { url, server, scope } = await runServer(__dirname, `${path.resolve(__dirname, '.')}/server.ts`);
 
-  const response = (await getAPIResponse(new URL(`${url}/express`))) as TestAPIResponse;
+  const response = (await getAPIResponse({ url: `${url}/express`, server, scope })) as TestAPIResponse;
 
   expect(response).toBeDefined();
   expect(response).toMatchObject({

packages/node-integration-tests/suites/express/sentry-trace/trace-header-assign/test.ts

@@ -5,11 +5,14 @@ import { getAPIResponse, runServer } from '../../../../utils/index';
 import { TestAPIResponse } from '../server';
 
 test('Should assign `sentry-trace` header which sets parent trace id of an outgoing request.', async () => {
-  const url = await runServer(__dirname, `${path.resolve(__dirname, '..')}/server.ts`);
+  const { url, server, scope } = await runServer(__dirname, `${path.resolve(__dirname, '..')}/server.ts`);
 
-  const response = (await getAPIResponse(new URL(`${url}/express`), {
-    'sentry-trace': '12312012123120121231201212312012-1121201211212012-0',
-  })) as TestAPIResponse;
+  const response = (await getAPIResponse(
+    { url: `${url}/express`, server, scope },
+    {
+      'sentry-trace': '12312012123120121231201212312012-1121201211212012-0',
+    },
+  )) as TestAPIResponse;
 
   expect(response).toBeDefined();
   expect(response).toMatchObject({

packages/node-integration-tests/suites/express/sentry-trace/trace-header-out/test.ts

@@ -5,9 +5,9 @@ import { getAPIResponse, runServer } from '../../../../utils/index';
 import { TestAPIResponse } from '../server';
 
 test('should attach a `sentry-trace` header to an outgoing request.', async () => {
-  const url = await runServer(__dirname, `${path.resolve(__dirname, '..')}/server.ts`);
+  const { url, server, scope } = await runServer(__dirname, `${path.resolve(__dirname, '..')}/server.ts`);
 
-  const response = (await getAPIResponse(new URL(`${url}/express`))) as TestAPIResponse;
+  const response = (await getAPIResponse({ url: `${url}/express`, server, scope })) as TestAPIResponse;
 
   expect(response).toBeDefined();
   expect(response).toMatchObject({