From d0950161b7aa7ed1152a78db03bc7e6e4717c4c3 Mon Sep 17 00:00:00 2001 From: Francesco Novy Date: Thu, 8 Aug 2024 16:42:57 +0200 Subject: [PATCH 01/16] test: Streamline test timeout --- dev-packages/node-integration-tests/jest.setup.js | 3 +-- .../node-integration-tests/suites/tracing/connect/test.ts | 2 -- .../node-integration-tests/suites/tracing/hapi/test.ts | 2 -- .../node-integration-tests/suites/tracing/mongodb/test.ts | 2 -- .../node-integration-tests/suites/tracing/mongoose/test.ts | 2 -- .../suites/tracing/nestjs-errors-no-express/test.ts | 2 -- .../suites/tracing/nestjs-errors/test.ts | 2 -- .../suites/tracing/nestjs-no-express/test.ts | 2 -- .../node-integration-tests/suites/tracing/nestjs/test.ts | 2 -- packages/node/test/integrations/localvariables.test.ts | 2 -- 10 files changed, 1 insertion(+), 20 deletions(-) diff --git a/dev-packages/node-integration-tests/jest.setup.js b/dev-packages/node-integration-tests/jest.setup.js index b0c26e5b05f2..59f749e83617 100644 --- a/dev-packages/node-integration-tests/jest.setup.js +++ b/dev-packages/node-integration-tests/jest.setup.js @@ -1,7 +1,6 @@ const { cleanupChildProcesses } = require('./utils/runner'); -// Increases test timeout from 5s to 45s -jest.setTimeout(45000); +jest.setTimeout(15000); afterEach(() => { cleanupChildProcesses(); diff --git a/dev-packages/node-integration-tests/suites/tracing/connect/test.ts b/dev-packages/node-integration-tests/suites/tracing/connect/test.ts index ad49a4e4532d..dd14c2277f7b 100644 --- a/dev-packages/node-integration-tests/suites/tracing/connect/test.ts +++ b/dev-packages/node-integration-tests/suites/tracing/connect/test.ts @@ -1,7 +1,5 @@ import { cleanupChildProcesses, createRunner } from '../../../utils/runner'; -jest.setTimeout(20000); - describe('connect auto-instrumentation', () => { afterAll(async () => { cleanupChildProcesses(); diff --git a/dev-packages/node-integration-tests/suites/tracing/hapi/test.ts b/dev-packages/node-integration-tests/suites/tracing/hapi/test.ts index 903b3b0b6fa8..8bb3bfdb0796 100644 --- a/dev-packages/node-integration-tests/suites/tracing/hapi/test.ts +++ b/dev-packages/node-integration-tests/suites/tracing/hapi/test.ts @@ -1,7 +1,5 @@ import { cleanupChildProcesses, createRunner } from '../../../utils/runner'; -jest.setTimeout(20000); - describe('hapi auto-instrumentation', () => { afterAll(async () => { cleanupChildProcesses(); diff --git a/dev-packages/node-integration-tests/suites/tracing/mongodb/test.ts b/dev-packages/node-integration-tests/suites/tracing/mongodb/test.ts index 2f79385521d3..59c50d32ebdc 100644 --- a/dev-packages/node-integration-tests/suites/tracing/mongodb/test.ts +++ b/dev-packages/node-integration-tests/suites/tracing/mongodb/test.ts @@ -2,8 +2,6 @@ import { MongoMemoryServer } from 'mongodb-memory-server-global'; import { cleanupChildProcesses, createRunner } from '../../../utils/runner'; -jest.setTimeout(20000); - describe('MongoDB experimental Test', () => { let mongoServer: MongoMemoryServer; diff --git a/dev-packages/node-integration-tests/suites/tracing/mongoose/test.ts b/dev-packages/node-integration-tests/suites/tracing/mongoose/test.ts index 050a3ffc9e12..30eeb62ffe31 100644 --- a/dev-packages/node-integration-tests/suites/tracing/mongoose/test.ts +++ b/dev-packages/node-integration-tests/suites/tracing/mongoose/test.ts @@ -2,8 +2,6 @@ import { MongoMemoryServer } from 'mongodb-memory-server-global'; import { cleanupChildProcesses, createRunner } from '../../../utils/runner'; -jest.setTimeout(20000); - describe('Mongoose experimental Test', () => { let mongoServer: MongoMemoryServer; diff --git a/dev-packages/node-integration-tests/suites/tracing/nestjs-errors-no-express/test.ts b/dev-packages/node-integration-tests/suites/tracing/nestjs-errors-no-express/test.ts index f38550469446..84b1aeef40c4 100644 --- a/dev-packages/node-integration-tests/suites/tracing/nestjs-errors-no-express/test.ts +++ b/dev-packages/node-integration-tests/suites/tracing/nestjs-errors-no-express/test.ts @@ -1,8 +1,6 @@ import { conditionalTest } from '../../../utils'; import { cleanupChildProcesses, createRunner } from '../../../utils/runner'; -jest.setTimeout(20000); - const { TS_VERSION } = process.env; const isOldTS = TS_VERSION && TS_VERSION.startsWith('3.'); diff --git a/dev-packages/node-integration-tests/suites/tracing/nestjs-errors/test.ts b/dev-packages/node-integration-tests/suites/tracing/nestjs-errors/test.ts index 264cbe1482cc..1b366307eac6 100644 --- a/dev-packages/node-integration-tests/suites/tracing/nestjs-errors/test.ts +++ b/dev-packages/node-integration-tests/suites/tracing/nestjs-errors/test.ts @@ -1,8 +1,6 @@ import { conditionalTest } from '../../../utils'; import { cleanupChildProcesses, createRunner } from '../../../utils/runner'; -jest.setTimeout(20000); - const { TS_VERSION } = process.env; const isOldTS = TS_VERSION && TS_VERSION.startsWith('3.'); diff --git a/dev-packages/node-integration-tests/suites/tracing/nestjs-no-express/test.ts b/dev-packages/node-integration-tests/suites/tracing/nestjs-no-express/test.ts index 70eb9e9aaa26..59532ef989da 100644 --- a/dev-packages/node-integration-tests/suites/tracing/nestjs-no-express/test.ts +++ b/dev-packages/node-integration-tests/suites/tracing/nestjs-no-express/test.ts @@ -1,8 +1,6 @@ import { conditionalTest } from '../../../utils'; import { cleanupChildProcesses, createRunner } from '../../../utils/runner'; -jest.setTimeout(20000); - const { TS_VERSION } = process.env; const isOldTS = TS_VERSION && TS_VERSION.startsWith('3.'); diff --git a/dev-packages/node-integration-tests/suites/tracing/nestjs/test.ts b/dev-packages/node-integration-tests/suites/tracing/nestjs/test.ts index 2b42f23c857a..686c93e1cad6 100644 --- a/dev-packages/node-integration-tests/suites/tracing/nestjs/test.ts +++ b/dev-packages/node-integration-tests/suites/tracing/nestjs/test.ts @@ -1,8 +1,6 @@ import { conditionalTest } from '../../../utils'; import { cleanupChildProcesses, createRunner } from '../../../utils/runner'; -jest.setTimeout(20000); - const { TS_VERSION } = process.env; const isOldTS = TS_VERSION && TS_VERSION.startsWith('3.'); diff --git a/packages/node/test/integrations/localvariables.test.ts b/packages/node/test/integrations/localvariables.test.ts index db9385214d42..d95045fb0acc 100644 --- a/packages/node/test/integrations/localvariables.test.ts +++ b/packages/node/test/integrations/localvariables.test.ts @@ -2,8 +2,6 @@ import { createRateLimiter } from '../../src/integrations/local-variables/common import { createCallbackList } from '../../src/integrations/local-variables/local-variables-sync'; import { NODE_MAJOR } from '../../src/nodeVersion'; -jest.setTimeout(20_000); - const describeIf = (condition: boolean) => (condition ? describe : describe.skip); describeIf(NODE_MAJOR >= 18)('LocalVariables', () => { From 30ba5de664ac27bfe4a2ef8cb93d1d0a3654bf99 Mon Sep 17 00:00:00 2001 From: Francesco Novy Date: Thu, 8 Aug 2024 16:50:04 +0200 Subject: [PATCH 02/16] increase timeout --- dev-packages/node-integration-tests/jest.setup.js | 2 +- dev-packages/node-integration-tests/utils/runner.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/dev-packages/node-integration-tests/jest.setup.js b/dev-packages/node-integration-tests/jest.setup.js index 59f749e83617..deb0a7190082 100644 --- a/dev-packages/node-integration-tests/jest.setup.js +++ b/dev-packages/node-integration-tests/jest.setup.js @@ -1,6 +1,6 @@ const { cleanupChildProcesses } = require('./utils/runner'); -jest.setTimeout(15000); +jest.setTimeout(25000); afterEach(() => { cleanupChildProcesses(); diff --git a/dev-packages/node-integration-tests/utils/runner.ts b/dev-packages/node-integration-tests/utils/runner.ts index ae0451f0d576..39495162ffbf 100644 --- a/dev-packages/node-integration-tests/utils/runner.ts +++ b/dev-packages/node-integration-tests/utils/runner.ts @@ -121,7 +121,7 @@ async function runDockerCompose(options: DockerOptions): Promise { const timeout = setTimeout(() => { close(); reject(new Error('Timed out waiting for docker-compose')); - }, 60_000); + }, 15_000); function newData(data: Buffer): void { const text = data.toString('utf8'); From ff129be5653b71629ab89c61d879746104654bba Mon Sep 17 00:00:00 2001 From: Francesco Novy Date: Thu, 8 Aug 2024 16:52:07 +0200 Subject: [PATCH 03/16] update timeouts --- dev-packages/node-integration-tests/jest.setup.js | 2 +- dev-packages/node-integration-tests/utils/runner.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/dev-packages/node-integration-tests/jest.setup.js b/dev-packages/node-integration-tests/jest.setup.js index deb0a7190082..bcba26d1c696 100644 --- a/dev-packages/node-integration-tests/jest.setup.js +++ b/dev-packages/node-integration-tests/jest.setup.js @@ -1,6 +1,6 @@ const { cleanupChildProcesses } = require('./utils/runner'); -jest.setTimeout(25000); +jest.setTimeout(45000); afterEach(() => { cleanupChildProcesses(); diff --git a/dev-packages/node-integration-tests/utils/runner.ts b/dev-packages/node-integration-tests/utils/runner.ts index 39495162ffbf..0a3aaaee4423 100644 --- a/dev-packages/node-integration-tests/utils/runner.ts +++ b/dev-packages/node-integration-tests/utils/runner.ts @@ -121,7 +121,7 @@ async function runDockerCompose(options: DockerOptions): Promise { const timeout = setTimeout(() => { close(); reject(new Error('Timed out waiting for docker-compose')); - }, 15_000); + }, 30_000); function newData(data: Buffer): void { const text = data.toString('utf8'); From 2aac9ce293434039be6c10b7ec5c4e0723f63f6d Mon Sep 17 00:00:00 2001 From: Francesco Novy Date: Thu, 8 Aug 2024 16:59:45 +0200 Subject: [PATCH 04/16] smarter timeout --- dev-packages/node-integration-tests/jest.setup.js | 3 ++- dev-packages/node-integration-tests/utils/runner.ts | 4 +++- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/dev-packages/node-integration-tests/jest.setup.js b/dev-packages/node-integration-tests/jest.setup.js index bcba26d1c696..88492fc5d945 100644 --- a/dev-packages/node-integration-tests/jest.setup.js +++ b/dev-packages/node-integration-tests/jest.setup.js @@ -1,6 +1,7 @@ const { cleanupChildProcesses } = require('./utils/runner'); -jest.setTimeout(45000); +// Default timeout: 15s +jest.setTimeout(15000); afterEach(() => { cleanupChildProcesses(); diff --git a/dev-packages/node-integration-tests/utils/runner.ts b/dev-packages/node-integration-tests/utils/runner.ts index 0a3aaaee4423..b0d9303af814 100644 --- a/dev-packages/node-integration-tests/utils/runner.ts +++ b/dev-packages/node-integration-tests/utils/runner.ts @@ -121,7 +121,7 @@ async function runDockerCompose(options: DockerOptions): Promise { const timeout = setTimeout(() => { close(); reject(new Error('Timed out waiting for docker-compose')); - }, 30_000); + }, 60_000); function newData(data: Buffer): void { const text = data.toString('utf8'); @@ -230,6 +230,8 @@ export function createRunner(...paths: string[]) { return this; }, withDockerCompose: function (options: DockerOptions) { + // When running docker compose, we need a larger timeout, as this takes some time... + jest.setTimeout(75000); dockerOptions = options; return this; }, From aabf9ffe721b71cb2bd32b6cd9f880225fca2f47 Mon Sep 17 00:00:00 2001 From: Francesco Novy Date: Thu, 8 Aug 2024 17:10:30 +0200 Subject: [PATCH 05/16] manually longer timeout --- .../node-integration-tests/suites/tracing/mysql2/test.ts | 3 +++ .../node-integration-tests/suites/tracing/postgres/test.ts | 3 +++ .../node-integration-tests/suites/tracing/redis-cache/test.ts | 3 +++ .../node-integration-tests/suites/tracing/redis/test.ts | 3 +++ dev-packages/node-integration-tests/utils/runner.ts | 2 -- 5 files changed, 12 insertions(+), 2 deletions(-) diff --git a/dev-packages/node-integration-tests/suites/tracing/mysql2/test.ts b/dev-packages/node-integration-tests/suites/tracing/mysql2/test.ts index db056fd222e8..60070308c7db 100644 --- a/dev-packages/node-integration-tests/suites/tracing/mysql2/test.ts +++ b/dev-packages/node-integration-tests/suites/tracing/mysql2/test.ts @@ -1,5 +1,8 @@ import { cleanupChildProcesses, createRunner } from '../../../utils/runner'; +// When running docker compose, we need a larger timeout, as this takes some time... +jest.setTimeout(75000); + describe('mysql2 auto instrumentation', () => { afterAll(() => { cleanupChildProcesses(); diff --git a/dev-packages/node-integration-tests/suites/tracing/postgres/test.ts b/dev-packages/node-integration-tests/suites/tracing/postgres/test.ts index d83f992638d1..f2549c70eb90 100644 --- a/dev-packages/node-integration-tests/suites/tracing/postgres/test.ts +++ b/dev-packages/node-integration-tests/suites/tracing/postgres/test.ts @@ -1,5 +1,8 @@ import { createRunner } from '../../../utils/runner'; +// When running docker compose, we need a larger timeout, as this takes some time... +jest.setTimeout(75000); + describe('postgres auto instrumentation', () => { test('should auto-instrument `pg` package', done => { const EXPECTED_TRANSACTION = { diff --git a/dev-packages/node-integration-tests/suites/tracing/redis-cache/test.ts b/dev-packages/node-integration-tests/suites/tracing/redis-cache/test.ts index 0c0807c8f480..8d494986ab3b 100644 --- a/dev-packages/node-integration-tests/suites/tracing/redis-cache/test.ts +++ b/dev-packages/node-integration-tests/suites/tracing/redis-cache/test.ts @@ -1,5 +1,8 @@ import { cleanupChildProcesses, createRunner } from '../../../utils/runner'; +// When running docker compose, we need a larger timeout, as this takes some time... +jest.setTimeout(75000); + describe('redis cache auto instrumentation', () => { afterAll(() => { cleanupChildProcesses(); diff --git a/dev-packages/node-integration-tests/suites/tracing/redis/test.ts b/dev-packages/node-integration-tests/suites/tracing/redis/test.ts index f68c14499a13..5a801a2f6a5b 100644 --- a/dev-packages/node-integration-tests/suites/tracing/redis/test.ts +++ b/dev-packages/node-integration-tests/suites/tracing/redis/test.ts @@ -1,5 +1,8 @@ import { cleanupChildProcesses, createRunner } from '../../../utils/runner'; +// When running docker compose, we need a larger timeout, as this takes some time... +jest.setTimeout(75000); + describe('redis auto instrumentation', () => { afterAll(() => { cleanupChildProcesses(); diff --git a/dev-packages/node-integration-tests/utils/runner.ts b/dev-packages/node-integration-tests/utils/runner.ts index b0d9303af814..ae0451f0d576 100644 --- a/dev-packages/node-integration-tests/utils/runner.ts +++ b/dev-packages/node-integration-tests/utils/runner.ts @@ -230,8 +230,6 @@ export function createRunner(...paths: string[]) { return this; }, withDockerCompose: function (options: DockerOptions) { - // When running docker compose, we need a larger timeout, as this takes some time... - jest.setTimeout(75000); dockerOptions = options; return this; }, From a8ef24d155619ade12102862e79b48211c113f7a Mon Sep 17 00:00:00 2001 From: Francesco Novy Date: Thu, 8 Aug 2024 17:23:39 +0200 Subject: [PATCH 06/16] oops... --- packages/node/test/integrations/localvariables.test.ts | 2 ++ 1 file changed, 2 insertions(+) diff --git a/packages/node/test/integrations/localvariables.test.ts b/packages/node/test/integrations/localvariables.test.ts index d95045fb0acc..db9385214d42 100644 --- a/packages/node/test/integrations/localvariables.test.ts +++ b/packages/node/test/integrations/localvariables.test.ts @@ -2,6 +2,8 @@ import { createRateLimiter } from '../../src/integrations/local-variables/common import { createCallbackList } from '../../src/integrations/local-variables/local-variables-sync'; import { NODE_MAJOR } from '../../src/nodeVersion'; +jest.setTimeout(20_000); + const describeIf = (condition: boolean) => (condition ? describe : describe.skip); describeIf(NODE_MAJOR >= 18)('LocalVariables', () => { From 4a21ce286b331be2df746610966cb0c96f8aa8a0 Mon Sep 17 00:00:00 2001 From: Francesco Novy Date: Thu, 8 Aug 2024 17:24:52 +0200 Subject: [PATCH 07/16] timeout for local variables test --- .../suites/public-api/LocalVariables/test.ts | 2 ++ 1 file changed, 2 insertions(+) diff --git a/dev-packages/node-integration-tests/suites/public-api/LocalVariables/test.ts b/dev-packages/node-integration-tests/suites/public-api/LocalVariables/test.ts index 7076388b9d29..5ed60bf65284 100644 --- a/dev-packages/node-integration-tests/suites/public-api/LocalVariables/test.ts +++ b/dev-packages/node-integration-tests/suites/public-api/LocalVariables/test.ts @@ -3,6 +3,8 @@ import * as path from 'path'; import { conditionalTest } from '../../../utils'; import { cleanupChildProcesses, createRunner } from '../../../utils/runner'; +jest.setTimeout(45_000); + const EXPECTED_LOCAL_VARIABLES_EVENT = { exception: { values: [ From a49fdad5fe629f61c9e237e622f69171a2608d09 Mon Sep 17 00:00:00 2001 From: Francesco Novy Date: Fri, 9 Aug 2024 10:14:44 +0200 Subject: [PATCH 08/16] fix one test --- .../suites/express/multiple-init/server.ts | 8 ++++++-- .../suites/express/multiple-init/test.ts | 11 +++++++++++ 2 files changed, 17 insertions(+), 2 deletions(-) diff --git a/dev-packages/node-integration-tests/suites/express/multiple-init/server.ts b/dev-packages/node-integration-tests/suites/express/multiple-init/server.ts index 4d1625035ebf..39d56710f043 100644 --- a/dev-packages/node-integration-tests/suites/express/multiple-init/server.ts +++ b/dev-packages/node-integration-tests/suites/express/multiple-init/server.ts @@ -54,15 +54,19 @@ app.get('/test/error/:id', (req, res) => { setTimeout(() => { // We flush to ensure we are sending exceptions in a certain order - Sentry.flush(3000).then( + Sentry.flush(1000).then( () => { + // We send this so we can wait for this, to know the test is ended & server can be closed + if (id === '3') { + Sentry.captureException(new Error('Final exception was captured')); + } res.send({}); }, () => { res.send({}); }, ); - }, 1000); + }, 1); }); Sentry.setupExpressErrorHandler(app); diff --git a/dev-packages/node-integration-tests/suites/express/multiple-init/test.ts b/dev-packages/node-integration-tests/suites/express/multiple-init/test.ts index f654fc361442..b80669a7c432 100644 --- a/dev-packages/node-integration-tests/suites/express/multiple-init/test.ts +++ b/dev-packages/node-integration-tests/suites/express/multiple-init/test.ts @@ -48,6 +48,17 @@ test('allows to call init multiple times', done => { }, }, }) + .expect({ + event: { + exception: { + values: [ + { + value: 'Final exception was captured', + }, + ], + }, + }, + }) .start(done); runner From 6fa1b8d8720daf5274e9c4471d6d375b3577a992 Mon Sep 17 00:00:00 2001 From: Francesco Novy Date: Fri, 9 Aug 2024 12:50:18 +0200 Subject: [PATCH 09/16] ensure everything is properly closed --- dev-packages/node-integration-tests/README.md | 8 - .../suites/tracing/connect/scenario.js | 4 +- .../suites/tracing/hapi/scenario.js | 4 +- .../nestjs-errors-no-express/scenario.ts | 4 +- .../suites/tracing/nestjs-errors/scenario.ts | 4 +- .../tracing/nestjs-no-express/scenario.ts | 4 +- .../suites/tracing/nestjs/scenario.ts | 4 +- .../requests/fetch-breadcrumbs/test.ts | 4 +- .../tracing/requests/fetch-no-tracing/test.ts | 4 +- .../fetch-sampled-no-active-span/test.ts | 4 +- .../tracing/requests/fetch-unsampled/test.ts | 4 +- .../tracing/requests/http-breadcrumbs/test.ts | 4 +- .../tracing/requests/http-no-tracing/test.ts | 4 +- .../http-sampled-no-active-span/test.ts | 4 +- .../tracing/requests/http-sampled/test.ts | 4 +- .../tracing/requests/http-unsampled/test.ts | 4 +- .../suites/tracing/spans/test.ts | 4 +- .../tracing/tracePropagationTargets/test.ts | 4 +- .../node-integration-tests/utils/index.ts | 251 +----------------- .../node-integration-tests/utils/runner.ts | 14 +- .../node-integration-tests/utils/server.ts | 24 +- 21 files changed, 64 insertions(+), 301 deletions(-) diff --git a/dev-packages/node-integration-tests/README.md b/dev-packages/node-integration-tests/README.md index 35b3c10883b7..ab1ce5e834de 100644 --- a/dev-packages/node-integration-tests/README.md +++ b/dev-packages/node-integration-tests/README.md @@ -38,14 +38,6 @@ requests, and assertions. Test server, interceptors and assertions are all run o `utils/` contains helpers and Sentry-specific assertions that can be used in (`test.ts`). -`TestEnv` class contains methods to create and execute requests on a test server instance. `TestEnv.init()` which starts -a test server and returns a `TestEnv` instance must be called by each test. The test server is automatically shut down -after each test, if a data collection helper method such as `getEnvelopeRequest` and `getAPIResponse` is used. Tests -that do not use those helper methods will need to end the server manually. - -`TestEnv` instance has two public properties: `url` and `server`. The `url` property is the base URL for the server. The -`http.Server` instance is used to finish the server eventually. - Nock interceptors are internally used to capture envelope requests by `getEnvelopeRequest` and `getMultipleEnvelopeRequest` helpers. After capturing required requests, the interceptors are removed. Nock can manually be used inside the test cases to intercept requests but should be removed before the test ends, as not to cause diff --git a/dev-packages/node-integration-tests/suites/tracing/connect/scenario.js b/dev-packages/node-integration-tests/suites/tracing/connect/scenario.js index b4013cd20434..db95fad457b2 100644 --- a/dev-packages/node-integration-tests/suites/tracing/connect/scenario.js +++ b/dev-packages/node-integration-tests/suites/tracing/connect/scenario.js @@ -13,7 +13,7 @@ Sentry.init({ const connect = require('connect'); const http = require('http'); -const init = async () => { +const run = async () => { const app = connect(); app.use('/', function (req, res, next) { @@ -34,4 +34,4 @@ const init = async () => { sendPortToRunner(port); }; -init(); +run(); diff --git a/dev-packages/node-integration-tests/suites/tracing/hapi/scenario.js b/dev-packages/node-integration-tests/suites/tracing/hapi/scenario.js index 184dcb3b8ea1..f3171eb085e0 100644 --- a/dev-packages/node-integration-tests/suites/tracing/hapi/scenario.js +++ b/dev-packages/node-integration-tests/suites/tracing/hapi/scenario.js @@ -13,7 +13,7 @@ const Boom = require('@hapi/boom'); const port = 5999; -const init = async () => { +const run = async () => { const server = Hapi.server({ host: 'localhost', port, @@ -65,4 +65,4 @@ const init = async () => { sendPortToRunner(port); }; -init(); +run(); diff --git a/dev-packages/node-integration-tests/suites/tracing/nestjs-errors-no-express/scenario.ts b/dev-packages/node-integration-tests/suites/tracing/nestjs-errors-no-express/scenario.ts index 295eba00fd9a..51173004b2f8 100644 --- a/dev-packages/node-integration-tests/suites/tracing/nestjs-errors-no-express/scenario.ts +++ b/dev-packages/node-integration-tests/suites/tracing/nestjs-errors-no-express/scenario.ts @@ -47,7 +47,7 @@ class AppController { }) class AppModule {} -async function init(): Promise { +async function run(): Promise { const app = await NestFactory.create(AppModule); const { httpAdapter } = app.get(HttpAdapterHost); Sentry.setupNestErrorHandler(app, new BaseExceptionFilter(httpAdapter)); @@ -56,4 +56,4 @@ async function init(): Promise { } // eslint-disable-next-line @typescript-eslint/no-floating-promises -init(); +run(); diff --git a/dev-packages/node-integration-tests/suites/tracing/nestjs-errors/scenario.ts b/dev-packages/node-integration-tests/suites/tracing/nestjs-errors/scenario.ts index 09a59eb8c7c7..11a0bb831c36 100644 --- a/dev-packages/node-integration-tests/suites/tracing/nestjs-errors/scenario.ts +++ b/dev-packages/node-integration-tests/suites/tracing/nestjs-errors/scenario.ts @@ -45,7 +45,7 @@ class AppController { }) class AppModule {} -async function init(): Promise { +async function run(): Promise { const app = await NestFactory.create(AppModule); const { httpAdapter } = app.get(HttpAdapterHost); Sentry.setupNestErrorHandler(app, new BaseExceptionFilter(httpAdapter)); @@ -54,4 +54,4 @@ async function init(): Promise { } // eslint-disable-next-line @typescript-eslint/no-floating-promises -init(); +run(); diff --git a/dev-packages/node-integration-tests/suites/tracing/nestjs-no-express/scenario.ts b/dev-packages/node-integration-tests/suites/tracing/nestjs-no-express/scenario.ts index 209f517193dc..b6a6e4c0dca7 100644 --- a/dev-packages/node-integration-tests/suites/tracing/nestjs-no-express/scenario.ts +++ b/dev-packages/node-integration-tests/suites/tracing/nestjs-no-express/scenario.ts @@ -47,7 +47,7 @@ class AppController { }) class AppModule {} -async function init(): Promise { +async function run(): Promise { const app = await NestFactory.create(AppModule); const { httpAdapter } = app.get(HttpAdapterHost); Sentry.setupNestErrorHandler(app, new BaseExceptionFilter(httpAdapter)); @@ -56,4 +56,4 @@ async function init(): Promise { } // eslint-disable-next-line @typescript-eslint/no-floating-promises -init(); +run(); diff --git a/dev-packages/node-integration-tests/suites/tracing/nestjs/scenario.ts b/dev-packages/node-integration-tests/suites/tracing/nestjs/scenario.ts index 19ec6c04c3e3..b75ff4d8a9ef 100644 --- a/dev-packages/node-integration-tests/suites/tracing/nestjs/scenario.ts +++ b/dev-packages/node-integration-tests/suites/tracing/nestjs/scenario.ts @@ -45,11 +45,11 @@ class AppController { }) class AppModule {} -async function init(): Promise { +async function run(): Promise { const app = await NestFactory.create(AppModule); await app.listen(port); sendPortToRunner(port); } // eslint-disable-next-line @typescript-eslint/no-floating-promises -init(); +run(); diff --git a/dev-packages/node-integration-tests/suites/tracing/requests/fetch-breadcrumbs/test.ts b/dev-packages/node-integration-tests/suites/tracing/requests/fetch-breadcrumbs/test.ts index 2c238c9ecd15..c0d783aaa594 100644 --- a/dev-packages/node-integration-tests/suites/tracing/requests/fetch-breadcrumbs/test.ts +++ b/dev-packages/node-integration-tests/suites/tracing/requests/fetch-breadcrumbs/test.ts @@ -6,7 +6,7 @@ conditionalTest({ min: 18 })('outgoing fetch', () => { test('outgoing fetch requests create breadcrumbs', done => { createTestServer(done) .start() - .then(SERVER_URL => { + .then(([SERVER_URL, closeTestServer]) => { createRunner(__dirname, 'scenario.ts') .withEnv({ SERVER_URL }) .ensureNoErrorOutput() @@ -72,7 +72,7 @@ conditionalTest({ min: 18 })('outgoing fetch', () => { }, }, }) - .start(done); + .start(closeTestServer); }); }); }); diff --git a/dev-packages/node-integration-tests/suites/tracing/requests/fetch-no-tracing/test.ts b/dev-packages/node-integration-tests/suites/tracing/requests/fetch-no-tracing/test.ts index f9ad7f92d3f1..9c732d899cde 100644 --- a/dev-packages/node-integration-tests/suites/tracing/requests/fetch-no-tracing/test.ts +++ b/dev-packages/node-integration-tests/suites/tracing/requests/fetch-no-tracing/test.ts @@ -26,7 +26,7 @@ conditionalTest({ min: 18 })('outgoing fetch', () => { expect(headers['sentry-trace']).toBeUndefined(); }) .start() - .then(SERVER_URL => { + .then(([SERVER_URL, closeTestServer]) => { createRunner(__dirname, 'scenario.ts') .withEnv({ SERVER_URL }) .ensureNoErrorOutput() @@ -42,7 +42,7 @@ conditionalTest({ min: 18 })('outgoing fetch', () => { }, }, }) - .start(done); + .start(closeTestServer); }); }); }); diff --git a/dev-packages/node-integration-tests/suites/tracing/requests/fetch-sampled-no-active-span/test.ts b/dev-packages/node-integration-tests/suites/tracing/requests/fetch-sampled-no-active-span/test.ts index 8b9ff1486565..fde1c787829a 100644 --- a/dev-packages/node-integration-tests/suites/tracing/requests/fetch-sampled-no-active-span/test.ts +++ b/dev-packages/node-integration-tests/suites/tracing/requests/fetch-sampled-no-active-span/test.ts @@ -26,7 +26,7 @@ conditionalTest({ min: 18 })('outgoing fetch', () => { expect(headers['sentry-trace']).toBeUndefined(); }) .start() - .then(SERVER_URL => { + .then(([SERVER_URL, closeTestServer]) => { createRunner(__dirname, 'scenario.ts') .withEnv({ SERVER_URL }) .expect({ @@ -41,7 +41,7 @@ conditionalTest({ min: 18 })('outgoing fetch', () => { }, }, }) - .start(done); + .start(closeTestServer); }); }); }); diff --git a/dev-packages/node-integration-tests/suites/tracing/requests/fetch-unsampled/test.ts b/dev-packages/node-integration-tests/suites/tracing/requests/fetch-unsampled/test.ts index 016881f47399..d288e9a03fbf 100644 --- a/dev-packages/node-integration-tests/suites/tracing/requests/fetch-unsampled/test.ts +++ b/dev-packages/node-integration-tests/suites/tracing/requests/fetch-unsampled/test.ts @@ -26,7 +26,7 @@ conditionalTest({ min: 18 })('outgoing fetch', () => { expect(headers['sentry-trace']).toBeUndefined(); }) .start() - .then(SERVER_URL => { + .then(([SERVER_URL, closeTestServer]) => { createRunner(__dirname, 'scenario.ts') .withEnv({ SERVER_URL }) .expect({ @@ -41,7 +41,7 @@ conditionalTest({ min: 18 })('outgoing fetch', () => { }, }, }) - .start(done); + .start(closeTestServer); }); }); }); diff --git a/dev-packages/node-integration-tests/suites/tracing/requests/http-breadcrumbs/test.ts b/dev-packages/node-integration-tests/suites/tracing/requests/http-breadcrumbs/test.ts index aab8338d0a35..812dbbb4ae60 100644 --- a/dev-packages/node-integration-tests/suites/tracing/requests/http-breadcrumbs/test.ts +++ b/dev-packages/node-integration-tests/suites/tracing/requests/http-breadcrumbs/test.ts @@ -4,7 +4,7 @@ import { createTestServer } from '../../../../utils/server'; test('outgoing http requests create breadcrumbs', done => { createTestServer(done) .start() - .then(SERVER_URL => { + .then(([SERVER_URL, closeTestServer]) => { createRunner(__dirname, 'scenario.ts') .withEnv({ SERVER_URL }) .ensureNoErrorOutput() @@ -70,6 +70,6 @@ test('outgoing http requests create breadcrumbs', done => { }, }, }) - .start(done); + .start(closeTestServer); }); }); diff --git a/dev-packages/node-integration-tests/suites/tracing/requests/http-no-tracing/test.ts b/dev-packages/node-integration-tests/suites/tracing/requests/http-no-tracing/test.ts index 308e0c6676e2..d0b570625c2b 100644 --- a/dev-packages/node-integration-tests/suites/tracing/requests/http-no-tracing/test.ts +++ b/dev-packages/node-integration-tests/suites/tracing/requests/http-no-tracing/test.ts @@ -24,7 +24,7 @@ test('outgoing http requests are correctly instrumented with tracing disabled', expect(headers['sentry-trace']).toBeUndefined(); }) .start() - .then(SERVER_URL => { + .then(([SERVER_URL, closeTestServer]) => { createRunner(__dirname, 'scenario.ts') .withEnv({ SERVER_URL }) .ensureNoErrorOutput() @@ -40,6 +40,6 @@ test('outgoing http requests are correctly instrumented with tracing disabled', }, }, }) - .start(done); + .start(closeTestServer); }); }); diff --git a/dev-packages/node-integration-tests/suites/tracing/requests/http-sampled-no-active-span/test.ts b/dev-packages/node-integration-tests/suites/tracing/requests/http-sampled-no-active-span/test.ts index 83d8774dbd46..2baff11a5faf 100644 --- a/dev-packages/node-integration-tests/suites/tracing/requests/http-sampled-no-active-span/test.ts +++ b/dev-packages/node-integration-tests/suites/tracing/requests/http-sampled-no-active-span/test.ts @@ -24,7 +24,7 @@ test('outgoing sampled http requests without active span are correctly instrumen expect(headers['sentry-trace']).toBeUndefined(); }) .start() - .then(SERVER_URL => { + .then(([SERVER_URL, closeTestServer]) => { createRunner(__dirname, 'scenario.ts') .withEnv({ SERVER_URL }) .expect({ @@ -39,6 +39,6 @@ test('outgoing sampled http requests without active span are correctly instrumen }, }, }) - .start(done); + .start(closeTestServer); }); }); diff --git a/dev-packages/node-integration-tests/suites/tracing/requests/http-sampled/test.ts b/dev-packages/node-integration-tests/suites/tracing/requests/http-sampled/test.ts index fd939bc4458c..38dfa6524019 100644 --- a/dev-packages/node-integration-tests/suites/tracing/requests/http-sampled/test.ts +++ b/dev-packages/node-integration-tests/suites/tracing/requests/http-sampled/test.ts @@ -24,7 +24,7 @@ test('outgoing sampled http requests are correctly instrumented', done => { expect(headers['sentry-trace']).toBeUndefined(); }) .start() - .then(SERVER_URL => { + .then(([SERVER_URL, closeTestServer]) => { createRunner(__dirname, 'scenario.ts') .withEnv({ SERVER_URL }) .expect({ @@ -32,6 +32,6 @@ test('outgoing sampled http requests are correctly instrumented', done => { // we're not too concerned with the actual transaction here since this is tested elsewhere }, }) - .start(done); + .start(closeTestServer); }); }); diff --git a/dev-packages/node-integration-tests/suites/tracing/requests/http-unsampled/test.ts b/dev-packages/node-integration-tests/suites/tracing/requests/http-unsampled/test.ts index ed5d30631f31..3d2e0e421863 100644 --- a/dev-packages/node-integration-tests/suites/tracing/requests/http-unsampled/test.ts +++ b/dev-packages/node-integration-tests/suites/tracing/requests/http-unsampled/test.ts @@ -24,7 +24,7 @@ test('outgoing http requests are correctly instrumented when not sampled', done expect(headers['sentry-trace']).toBeUndefined(); }) .start() - .then(SERVER_URL => { + .then(([SERVER_URL, closeTestServer]) => { createRunner(__dirname, 'scenario.ts') .withEnv({ SERVER_URL }) .expect({ @@ -39,6 +39,6 @@ test('outgoing http requests are correctly instrumented when not sampled', done }, }, }) - .start(done); + .start(closeTestServer); }); }); diff --git a/dev-packages/node-integration-tests/suites/tracing/spans/test.ts b/dev-packages/node-integration-tests/suites/tracing/spans/test.ts index 7def81fbe952..e349622d39f8 100644 --- a/dev-packages/node-integration-tests/suites/tracing/spans/test.ts +++ b/dev-packages/node-integration-tests/suites/tracing/spans/test.ts @@ -18,7 +18,7 @@ test('should capture spans for outgoing http requests', done => { 404, ) .start() - .then(SERVER_URL => { + .then(([SERVER_URL, closeTestServer]) => { createRunner(__dirname, 'scenario.ts') .withEnv({ SERVER_URL }) .expect({ @@ -43,6 +43,6 @@ test('should capture spans for outgoing http requests', done => { ]), }, }) - .start(done); + .start(closeTestServer); }); }); diff --git a/dev-packages/node-integration-tests/suites/tracing/tracePropagationTargets/test.ts b/dev-packages/node-integration-tests/suites/tracing/tracePropagationTargets/test.ts index c43b5607ef52..e6081bedd8ea 100644 --- a/dev-packages/node-integration-tests/suites/tracing/tracePropagationTargets/test.ts +++ b/dev-packages/node-integration-tests/suites/tracing/tracePropagationTargets/test.ts @@ -24,7 +24,7 @@ test('HttpIntegration should instrument correct requests when tracePropagationTa expect(headers['sentry-trace']).toBeUndefined(); }) .start() - .then(SERVER_URL => { + .then(([SERVER_URL, closeTestServer]) => { createRunner(__dirname, 'scenario.ts') .withEnv({ SERVER_URL }) .expect({ @@ -32,6 +32,6 @@ test('HttpIntegration should instrument correct requests when tracePropagationTa // we're not too concerned with the actual transaction here since this is tested elsewhere }, }) - .start(done); + .start(closeTestServer); }); }); diff --git a/dev-packages/node-integration-tests/utils/index.ts b/dev-packages/node-integration-tests/utils/index.ts index 52223f4417a2..8c12ec72e0d2 100644 --- a/dev-packages/node-integration-tests/utils/index.ts +++ b/dev-packages/node-integration-tests/utils/index.ts @@ -1,16 +1,6 @@ -import * as http from 'http'; -import type { AddressInfo } from 'net'; -import * as path from 'path'; -/* eslint-disable @typescript-eslint/no-unsafe-member-access */ -import * as Sentry from '@sentry/node'; +import type * as http from 'http'; import type { EnvelopeItemType } from '@sentry/types'; -import { logger, parseSemver } from '@sentry/utils'; -import type { AxiosRequestConfig } from 'axios'; -import axios from 'axios'; -import type { Express } from 'express'; -import type { HttpTerminator } from 'http-terminator'; -import { createHttpTerminator } from 'http-terminator'; -import nock from 'nock'; +import { parseSemver } from '@sentry/utils'; const NODE_VERSION = parseSemver(process.versions.node).major; @@ -93,240 +83,3 @@ export const assertSentryTransaction = (actual: Record, expecte export const parseEnvelope = (body: string): Array> => { return body.split('\n').map(e => JSON.parse(e)); }; - -/** - * Sends a get request to given URL. - * Flushes the Sentry event queue. - * - * @param {string} url - * @return {*} {Promise} - */ -export async function runScenario(url: string): Promise { - await axios.get(url); - await Sentry.flush(); -} - -async function makeRequest( - method: 'get' | 'post' = 'get', - url: string, - axiosConfig?: AxiosRequestConfig, -): Promise { - try { - if (method === 'get') { - await axios.get(url, axiosConfig); - } else { - await axios.post(url, axiosConfig); - } - } catch (e) { - // We sometimes expect the request to fail, but not the test. - // So, we do nothing. - logger.warn(e); - } -} - -export class TestEnv { - private _axiosConfig: AxiosRequestConfig | undefined = undefined; - private _terminator: HttpTerminator; - - public constructor(public readonly server: http.Server, public readonly url: string) { - this.server = server; - this.url = url; - this._terminator = createHttpTerminator({ server: this.server, gracefulTerminationTimeout: 0 }); - } - - /** - * Starts a test server and returns the TestEnv instance - * - * @param {string} testDir - * @param {string} [serverPath] - * @param {string} [scenarioPath] - * @return {*} {Promise} - */ - public static async init(testDir: string, serverPath?: string, scenarioPath?: string): Promise { - const defaultServerPath = path.resolve(process.cwd(), 'utils', 'defaults', 'server'); - - const [server, url] = await new Promise<[http.Server, string]>(resolve => { - // eslint-disable-next-line @typescript-eslint/no-var-requires, @typescript-eslint/no-unsafe-member-access - const app = require(serverPath || defaultServerPath).default as Express; - - app.get('/test', (_req, res) => { - try { - require(scenarioPath || `${testDir}/scenario`); - } finally { - res.status(200).end(); - } - }); - - const server = app.listen(0, () => { - const url = `http://localhost:${(server.address() as AddressInfo).port}/test`; - resolve([server, url]); - }); - }); - - return new TestEnv(server, url); - } - - /** - * Intercepts and extracts up to a number of requests containing Sentry envelopes. - * - * @param {DataCollectorOptions} options - * @returns The intercepted envelopes. - */ - public async getMultipleEnvelopeRequest(options: DataCollectorOptions): Promise[][]> { - const envelopeTypeArray = - typeof options.envelopeType === 'string' - ? [options.envelopeType] - : options.envelopeType || (['event'] as EnvelopeItemType[]); - - const resProm = this.setupNock( - options.count || 1, - typeof options.endServer === 'undefined' ? true : options.endServer, - envelopeTypeArray, - ); - - // eslint-disable-next-line @typescript-eslint/no-floating-promises - makeRequest(options.method, options.url || this.url, this._axiosConfig); - return resProm; - } - - /** - * Intercepts and extracts a single request containing a Sentry envelope - * - * @param {DataCollectorOptions} options - * @returns The extracted envelope. - */ - public async getEnvelopeRequest(options?: DataCollectorOptions): Promise>> { - const requests = await this.getMultipleEnvelopeRequest({ ...options, count: 1 }); - - if (!requests[0]) { - throw new Error('No requests found'); - } - - return requests[0]; - } - - /** - * Sends a get request to given URL, with optional headers. Returns the response. - * Ends the server instance and flushes the Sentry event queue. - * - * @param {Record} [headers] - * @return {*} {Promise} - */ - public async getAPIResponse( - url?: string, - headers: Record = {}, - endServer: boolean = true, - ): Promise { - try { - const { data } = await axios.get(url || this.url, { - headers, - // KeepAlive false to work around a Node 20 bug with ECONNRESET: https://github.com/axios/axios/issues/5929 - httpAgent: new http.Agent({ keepAlive: false }), - }); - return data; - } finally { - await Sentry.flush(); - - if (endServer) { - this.server.close(); - } - } - } - - public async setupNock( - count: number, - endServer: boolean, - envelopeType: EnvelopeItemType[], - ): Promise[][]> { - return new Promise(resolve => { - const envelopes: Record[][] = []; - const mock = nock('https://dsn.ingest.sentry.io') - .persist() - .post('/api/1337/envelope/', body => { - const envelope = parseEnvelope(body); - - if (envelopeType.includes(envelope[1]?.type as EnvelopeItemType)) { - envelopes.push(envelope); - } else { - return false; - } - - if (count === envelopes.length) { - nock.removeInterceptor(mock); - - if (endServer) { - // Cleaning nock only before the server is closed, - // not to break tests that use simultaneous requests to the server. - // Ex: Remix scope bleed tests. - nock.cleanAll(); - - // Abort all pending requests to nock to prevent hanging / flakes. - // See: https://github.com/nock/nock/issues/1118#issuecomment-544126948 - nock.abortPendingRequests(); - - this._closeServer() - .catch(e => { - logger.warn(e); - }) - .finally(() => { - resolve(envelopes); - }); - } else { - resolve(envelopes); - } - } - - return true; - }); - - mock - .query(true) // accept any query params - used for sentry_key param - .reply(200); - }); - } - - public setAxiosConfig(axiosConfig: AxiosRequestConfig): void { - this._axiosConfig = axiosConfig; - } - - public async countEnvelopes(options: { - url?: string; - timeout?: number; - envelopeType: EnvelopeItemType | EnvelopeItemType[]; - }): Promise { - return new Promise(resolve => { - let reqCount = 0; - - const mock = nock('https://dsn.ingest.sentry.io') - .persist() - .post('/api/1337/envelope/', body => { - const envelope = parseEnvelope(body); - - if (options.envelopeType.includes(envelope[1]?.type as EnvelopeItemType)) { - reqCount++; - return true; - } - - return false; - }); - - setTimeout( - () => { - nock.removeInterceptor(mock); - - nock.cleanAll(); - - // eslint-disable-next-line @typescript-eslint/no-floating-promises - this._closeServer().then(() => { - resolve(reqCount); - }); - }, - options.timeout || 1000, - ); - }); - } - - private _closeServer(): Promise { - return this._terminator.terminate(); - } -} diff --git a/dev-packages/node-integration-tests/utils/runner.ts b/dev-packages/node-integration-tests/utils/runner.ts index ae0451f0d576..f6ae051904b0 100644 --- a/dev-packages/node-integration-tests/utils/runner.ts +++ b/dev-packages/node-integration-tests/utils/runner.ts @@ -110,7 +110,7 @@ async function runDockerCompose(options: DockerOptions): Promise { return new Promise((resolve, reject) => { const cwd = join(...options.workingDirectory); const close = (): void => { - spawnSync('docker', ['compose', 'down', '--volumes'], { cwd }); + spawnSync('docker', ['compose', 'down', '--volumes'], { cwd, stdio: 'inherit' }); }; // ensure we're starting fresh @@ -359,9 +359,7 @@ export function createRunner(...paths: string[]) { } } - const serverStartup: Promise = withSentryServer - ? createBasicSentryServer(newEnvelope) - : Promise.resolve(undefined); + const serverStartup = withSentryServer ? createBasicSentryServer(newEnvelope) : Promise.resolve([]); const dockerStartup: Promise = dockerOptions ? runDockerCompose(dockerOptions) @@ -371,7 +369,13 @@ export function createRunner(...paths: string[]) { // eslint-disable-next-line @typescript-eslint/no-floating-promises startup - .then(([dockerChild, mockServerPort]) => { + .then(([dockerChild, [mockServerPort, mockServerClose]]) => { + if (mockServerClose) { + CLEANUP_STEPS.add(() => { + mockServerClose(); + }); + } + if (dockerChild) { CLEANUP_STEPS.add(dockerChild); } diff --git a/dev-packages/node-integration-tests/utils/server.ts b/dev-packages/node-integration-tests/utils/server.ts index f68f1a9c80d4..44f75d02a16e 100644 --- a/dev-packages/node-integration-tests/utils/server.ts +++ b/dev-packages/node-integration-tests/utils/server.ts @@ -9,8 +9,11 @@ import express from 'express'; * This does no checks on the envelope, it just calls the callback if it managed to parse an envelope from the raw POST * body data. */ -export function createBasicSentryServer(onEnvelope: (env: Envelope) => void): Promise { +export function createBasicSentryServer( + onEnvelope: (env: Envelope) => void, +): Promise<[port: number, close: (callback?: () => void) => void]> { const app = express(); + app.use(express.raw({ type: () => true, inflate: true, limit: '100mb' })); app.post('/api/:id/envelope/', (req, res) => { try { @@ -27,7 +30,12 @@ export function createBasicSentryServer(onEnvelope: (env: Envelope) => void): Pr return new Promise(resolve => { const server = app.listen(0, () => { const address = server.address() as AddressInfo; - resolve(address.port); + resolve([ + address.port, + () => { + server.close(); + }, + ]); }); }); } @@ -36,7 +44,7 @@ type HeaderAssertCallback = (headers: Record void) { +export function createTestServer(done: (error?: unknown) => void) { const gets: Array<[string, HeaderAssertCallback, number]> = []; return { @@ -44,7 +52,7 @@ export function createTestServer(done: (error: unknown) => void) { gets.push([path, callback, result]); return this; }, - start: async (): Promise => { + start: async (): Promise<[url: string, close: () => void]> => { const app = express(); for (const [path, callback, result] of gets) { @@ -62,7 +70,13 @@ export function createTestServer(done: (error: unknown) => void) { return new Promise(resolve => { const server = app.listen(0, () => { const address = server.address() as AddressInfo; - resolve(`http://localhost:${address.port}`); + resolve([ + `http://localhost:${address.port}`, + () => { + server.close(); + done(); + }, + ]); }); }); }, From 69aac747616eb503236c7d210343af405c74bfd2 Mon Sep 17 00:00:00 2001 From: Francesco Novy Date: Fri, 9 Aug 2024 12:52:41 +0200 Subject: [PATCH 10/16] fix debug --- dev-packages/node-integration-tests/utils/runner.ts | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/dev-packages/node-integration-tests/utils/runner.ts b/dev-packages/node-integration-tests/utils/runner.ts index f6ae051904b0..4d57d288c097 100644 --- a/dev-packages/node-integration-tests/utils/runner.ts +++ b/dev-packages/node-integration-tests/utils/runner.ts @@ -110,7 +110,10 @@ async function runDockerCompose(options: DockerOptions): Promise { return new Promise((resolve, reject) => { const cwd = join(...options.workingDirectory); const close = (): void => { - spawnSync('docker', ['compose', 'down', '--volumes'], { cwd, stdio: 'inherit' }); + spawnSync('docker', ['compose', 'down', '--volumes'], { + cwd, + stdio: process.env.DEBUG ? 'inherit' : undefined, + }); }; // ensure we're starting fresh @@ -126,6 +129,9 @@ async function runDockerCompose(options: DockerOptions): Promise { function newData(data: Buffer): void { const text = data.toString('utf8'); + // eslint-disable-next-line no-console + if (process.env.DEBUG) console.log(text); + for (const match of options.readyMatches) { if (text.includes(match)) { child.stdout.removeAllListeners(); From 54bbfa951038b0898ed5cecdaa0bc3d90d407ac7 Mon Sep 17 00:00:00 2001 From: Francesco Novy Date: Fri, 9 Aug 2024 13:29:04 +0200 Subject: [PATCH 11/16] move test env to remix --- .../integration/test/server/utils/helpers.ts | 259 +++++++++++++++++- 1 file changed, 258 insertions(+), 1 deletion(-) diff --git a/packages/remix/test/integration/test/server/utils/helpers.ts b/packages/remix/test/integration/test/server/utils/helpers.ts index eccda209fb48..981be12f314a 100644 --- a/packages/remix/test/integration/test/server/utils/helpers.ts +++ b/packages/remix/test/integration/test/server/utils/helpers.ts @@ -1,11 +1,264 @@ import * as http from 'http'; import { AddressInfo } from 'net'; +import * as path from 'path'; import { createRequestHandler } from '@remix-run/express'; +/* eslint-disable @typescript-eslint/no-unsafe-member-access */ +import * as Sentry from '@sentry/node'; +import type { EnvelopeItemType } from '@sentry/types'; +import { logger } from '@sentry/utils'; +import type { AxiosRequestConfig } from 'axios'; +import axios from 'axios'; import express from 'express'; -import { TestEnv } from '../../../../../../../dev-packages/node-integration-tests/utils'; +import type { Express } from 'express'; +import type { HttpTerminator } from 'http-terminator'; +import { createHttpTerminator } from 'http-terminator'; +import nock from 'nock'; export * from '../../../../../../../dev-packages/node-integration-tests/utils'; +type DataCollectorOptions = { + // Optional custom URL + url?: string; + + // The expected amount of requests to the envelope endpoint. + // If the amount of sent requests is lower than `count`, this function will not resolve. + count?: number; + + // The method of the request. + method?: 'get' | 'post'; + + // Whether to stop the server after the requests have been intercepted + endServer?: boolean; + + // Type(s) of the envelopes to capture + envelopeType?: EnvelopeItemType | EnvelopeItemType[]; +}; + +async function makeRequest( + method: 'get' | 'post' = 'get', + url: string, + axiosConfig?: AxiosRequestConfig, +): Promise { + try { + if (method === 'get') { + await axios.get(url, axiosConfig); + } else { + await axios.post(url, axiosConfig); + } + } catch (e) { + // We sometimes expect the request to fail, but not the test. + // So, we do nothing. + logger.warn(e); + } +} + +class TestEnv { + private _axiosConfig: AxiosRequestConfig | undefined = undefined; + private _terminator: HttpTerminator; + + public constructor(public readonly server: http.Server, public readonly url: string) { + this.server = server; + this.url = url; + this._terminator = createHttpTerminator({ server: this.server, gracefulTerminationTimeout: 0 }); + } + + /** + * Starts a test server and returns the TestEnv instance + * + * @param {string} testDir + * @param {string} [serverPath] + * @param {string} [scenarioPath] + * @return {*} {Promise} + */ + public static async init(testDir: string, serverPath?: string, scenarioPath?: string): Promise { + const defaultServerPath = path.resolve(process.cwd(), 'utils', 'defaults', 'server'); + + const [server, url] = await new Promise<[http.Server, string]>(resolve => { + // eslint-disable-next-line @typescript-eslint/no-var-requires, @typescript-eslint/no-unsafe-member-access + const app = require(serverPath || defaultServerPath).default as Express; + + app.get('/test', (_req, res) => { + try { + require(scenarioPath || `${testDir}/scenario`); + } finally { + res.status(200).end(); + } + }); + + const server = app.listen(0, () => { + const url = `http://localhost:${(server.address() as AddressInfo).port}/test`; + resolve([server, url]); + }); + }); + + return new TestEnv(server, url); + } + + /** + * Intercepts and extracts up to a number of requests containing Sentry envelopes. + * + * @param {DataCollectorOptions} options + * @returns The intercepted envelopes. + */ + public async getMultipleEnvelopeRequest(options: DataCollectorOptions): Promise[][]> { + const envelopeTypeArray = + typeof options.envelopeType === 'string' + ? [options.envelopeType] + : options.envelopeType || (['event'] as EnvelopeItemType[]); + + const resProm = this.setupNock( + options.count || 1, + typeof options.endServer === 'undefined' ? true : options.endServer, + envelopeTypeArray, + ); + + // eslint-disable-next-line @typescript-eslint/no-floating-promises + makeRequest(options.method, options.url || this.url, this._axiosConfig); + return resProm; + } + + /** + * Intercepts and extracts a single request containing a Sentry envelope + * + * @param {DataCollectorOptions} options + * @returns The extracted envelope. + */ + public async getEnvelopeRequest(options?: DataCollectorOptions): Promise>> { + const requests = await this.getMultipleEnvelopeRequest({ ...options, count: 1 }); + + if (!requests[0]) { + throw new Error('No requests found'); + } + + return requests[0]; + } + + /** + * Sends a get request to given URL, with optional headers. Returns the response. + * Ends the server instance and flushes the Sentry event queue. + * + * @param {Record} [headers] + * @return {*} {Promise} + */ + public async getAPIResponse( + url?: string, + headers: Record = {}, + endServer: boolean = true, + ): Promise { + try { + const { data } = await axios.get(url || this.url, { + headers, + // KeepAlive false to work around a Node 20 bug with ECONNRESET: https://github.com/axios/axios/issues/5929 + httpAgent: new http.Agent({ keepAlive: false }), + }); + return data; + } finally { + await Sentry.flush(); + + if (endServer) { + this.server.close(); + } + } + } + + public async setupNock( + count: number, + endServer: boolean, + envelopeType: EnvelopeItemType[], + ): Promise[][]> { + return new Promise(resolve => { + const envelopes: Record[][] = []; + const mock = nock('https://dsn.ingest.sentry.io') + .persist() + .post('/api/1337/envelope/', body => { + const envelope = parseEnvelope(body); + + if (envelopeType.includes(envelope[1]?.type as EnvelopeItemType)) { + envelopes.push(envelope); + } else { + return false; + } + + if (count === envelopes.length) { + nock.removeInterceptor(mock); + + if (endServer) { + // Cleaning nock only before the server is closed, + // not to break tests that use simultaneous requests to the server. + // Ex: Remix scope bleed tests. + nock.cleanAll(); + + // Abort all pending requests to nock to prevent hanging / flakes. + // See: https://github.com/nock/nock/issues/1118#issuecomment-544126948 + nock.abortPendingRequests(); + + this._closeServer() + .catch(e => { + logger.warn(e); + }) + .finally(() => { + resolve(envelopes); + }); + } else { + resolve(envelopes); + } + } + + return true; + }); + + mock + .query(true) // accept any query params - used for sentry_key param + .reply(200); + }); + } + + public setAxiosConfig(axiosConfig: AxiosRequestConfig): void { + this._axiosConfig = axiosConfig; + } + + public async countEnvelopes(options: { + url?: string; + timeout?: number; + envelopeType: EnvelopeItemType | EnvelopeItemType[]; + }): Promise { + return new Promise(resolve => { + let reqCount = 0; + + const mock = nock('https://dsn.ingest.sentry.io') + .persist() + .post('/api/1337/envelope/', body => { + const envelope = parseEnvelope(body); + + if (options.envelopeType.includes(envelope[1]?.type as EnvelopeItemType)) { + reqCount++; + return true; + } + + return false; + }); + + setTimeout( + () => { + nock.removeInterceptor(mock); + + nock.cleanAll(); + + // eslint-disable-next-line @typescript-eslint/no-floating-promises + this._closeServer().then(() => { + resolve(reqCount); + }); + }, + options.timeout || 1000, + ); + }); + } + + private _closeServer(): Promise { + return this._terminator.terminate(); + } +} + export class RemixTestEnv extends TestEnv { private constructor(public readonly server: http.Server, public readonly url: string) { super(server, url); @@ -27,3 +280,7 @@ export class RemixTestEnv extends TestEnv { return new RemixTestEnv(server, `http://localhost:${serverPort}`); } } + +const parseEnvelope = (body: string): Array> => { + return body.split('\n').map(e => JSON.parse(e)); +}; From 8e18a7c58e90c715fd40ce8b29fb22b80ca05a09 Mon Sep 17 00:00:00 2001 From: Francesco Novy Date: Fri, 9 Aug 2024 13:49:08 +0200 Subject: [PATCH 12/16] small cleanup --- dev-packages/node-integration-tests/utils/runner.ts | 5 +++-- dev-packages/node-integration-tests/utils/server.ts | 2 +- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/dev-packages/node-integration-tests/utils/runner.ts b/dev-packages/node-integration-tests/utils/runner.ts index 4d57d288c097..73d76b86be20 100644 --- a/dev-packages/node-integration-tests/utils/runner.ts +++ b/dev-packages/node-integration-tests/utils/runner.ts @@ -365,7 +365,9 @@ export function createRunner(...paths: string[]) { } } - const serverStartup = withSentryServer ? createBasicSentryServer(newEnvelope) : Promise.resolve([]); + const serverStartup = withSentryServer + ? createBasicSentryServer(newEnvelope) + : Promise.resolve([undefined, undefined]); const dockerStartup: Promise = dockerOptions ? runDockerCompose(dockerOptions) @@ -373,7 +375,6 @@ export function createRunner(...paths: string[]) { const startup = Promise.all([dockerStartup, serverStartup]); - // eslint-disable-next-line @typescript-eslint/no-floating-promises startup .then(([dockerChild, [mockServerPort, mockServerClose]]) => { if (mockServerClose) { diff --git a/dev-packages/node-integration-tests/utils/server.ts b/dev-packages/node-integration-tests/utils/server.ts index 44f75d02a16e..41341a223968 100644 --- a/dev-packages/node-integration-tests/utils/server.ts +++ b/dev-packages/node-integration-tests/utils/server.ts @@ -11,7 +11,7 @@ import express from 'express'; */ export function createBasicSentryServer( onEnvelope: (env: Envelope) => void, -): Promise<[port: number, close: (callback?: () => void) => void]> { +): Promise<[port: number, close: () => void]> { const app = express(); app.use(express.raw({ type: () => true, inflate: true, limit: '100mb' })); From 4132ce48dc858b99a6b9258f609218387561dd14 Mon Sep 17 00:00:00 2001 From: Francesco Novy Date: Mon, 12 Aug 2024 10:02:15 +0200 Subject: [PATCH 13/16] better type safety for ts 3.8 ? --- dev-packages/node-integration-tests/utils/runner.ts | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/dev-packages/node-integration-tests/utils/runner.ts b/dev-packages/node-integration-tests/utils/runner.ts index 73d76b86be20..e6cf7cb60765 100644 --- a/dev-packages/node-integration-tests/utils/runner.ts +++ b/dev-packages/node-integration-tests/utils/runner.ts @@ -365,7 +365,7 @@ export function createRunner(...paths: string[]) { } } - const serverStartup = withSentryServer + const serverStartup: Promise<[port: number | undefined, close: (() => void) | undefined]> = withSentryServer ? createBasicSentryServer(newEnvelope) : Promise.resolve([undefined, undefined]); @@ -373,7 +373,8 @@ export function createRunner(...paths: string[]) { ? runDockerCompose(dockerOptions) : Promise.resolve(undefined); - const startup = Promise.all([dockerStartup, serverStartup]); + const startup: Promise<[VoidFunction | undefined, [port: number | undefined, close: (() => void) | undefined]]> = + Promise.all([dockerStartup, serverStartup]); startup .then(([dockerChild, [mockServerPort, mockServerClose]]) => { From 8ac5f0b79e70df89d0f60f65c3b3e788e1e34901 Mon Sep 17 00:00:00 2001 From: Francesco Novy Date: Mon, 12 Aug 2024 10:16:46 +0200 Subject: [PATCH 14/16] no tuple name?? --- dev-packages/node-integration-tests/utils/runner.ts | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/dev-packages/node-integration-tests/utils/runner.ts b/dev-packages/node-integration-tests/utils/runner.ts index e6cf7cb60765..1aa35891157d 100644 --- a/dev-packages/node-integration-tests/utils/runner.ts +++ b/dev-packages/node-integration-tests/utils/runner.ts @@ -365,7 +365,7 @@ export function createRunner(...paths: string[]) { } } - const serverStartup: Promise<[port: number | undefined, close: (() => void) | undefined]> = withSentryServer + const serverStartup: Promise<[number | undefined, (() => void) | undefined]> = withSentryServer ? createBasicSentryServer(newEnvelope) : Promise.resolve([undefined, undefined]); @@ -373,8 +373,10 @@ export function createRunner(...paths: string[]) { ? runDockerCompose(dockerOptions) : Promise.resolve(undefined); - const startup: Promise<[VoidFunction | undefined, [port: number | undefined, close: (() => void) | undefined]]> = - Promise.all([dockerStartup, serverStartup]); + const startup: Promise<[VoidFunction | undefined, [number | undefined, (() => void) | undefined]]> = Promise.all([ + dockerStartup, + serverStartup, + ]); startup .then(([dockerChild, [mockServerPort, mockServerClose]]) => { From 3287f7622414bbae13bc96ce5030d8f140ecf2c8 Mon Sep 17 00:00:00 2001 From: Francesco Novy Date: Mon, 12 Aug 2024 10:54:27 +0200 Subject: [PATCH 15/16] mak TS 3.8 happy --- .../node-integration-tests/utils/runner.ts | 14 ++++++++------ .../node-integration-tests/utils/server.ts | 6 ++---- 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/dev-packages/node-integration-tests/utils/runner.ts b/dev-packages/node-integration-tests/utils/runner.ts index 1aa35891157d..cb4ab58347e7 100644 --- a/dev-packages/node-integration-tests/utils/runner.ts +++ b/dev-packages/node-integration-tests/utils/runner.ts @@ -365,18 +365,20 @@ export function createRunner(...paths: string[]) { } } - const serverStartup: Promise<[number | undefined, (() => void) | undefined]> = withSentryServer + // We need to properly define & pass these types around for TS 3.8, + // which otherwise fails to infer these correctly :( + type ServerStartup = [number | undefined, (() => void) | undefined]; + type DockerStartup = VoidFunction | undefined; + + const serverStartup: Promise = withSentryServer ? createBasicSentryServer(newEnvelope) : Promise.resolve([undefined, undefined]); - const dockerStartup: Promise = dockerOptions + const dockerStartup: Promise = dockerOptions ? runDockerCompose(dockerOptions) : Promise.resolve(undefined); - const startup: Promise<[VoidFunction | undefined, [number | undefined, (() => void) | undefined]]> = Promise.all([ - dockerStartup, - serverStartup, - ]); + const startup = Promise.all([dockerStartup, serverStartup]) as Promise<[DockerStartup, ServerStartup]>; startup .then(([dockerChild, [mockServerPort, mockServerClose]]) => { diff --git a/dev-packages/node-integration-tests/utils/server.ts b/dev-packages/node-integration-tests/utils/server.ts index 41341a223968..71a7adf9798f 100644 --- a/dev-packages/node-integration-tests/utils/server.ts +++ b/dev-packages/node-integration-tests/utils/server.ts @@ -9,9 +9,7 @@ import express from 'express'; * This does no checks on the envelope, it just calls the callback if it managed to parse an envelope from the raw POST * body data. */ -export function createBasicSentryServer( - onEnvelope: (env: Envelope) => void, -): Promise<[port: number, close: () => void]> { +export function createBasicSentryServer(onEnvelope: (env: Envelope) => void): Promise<[number, () => void]> { const app = express(); app.use(express.raw({ type: () => true, inflate: true, limit: '100mb' })); @@ -52,7 +50,7 @@ export function createTestServer(done: (error?: unknown) => void) { gets.push([path, callback, result]); return this; }, - start: async (): Promise<[url: string, close: () => void]> => { + start: async (): Promise<[string, () => void]> => { const app = express(); for (const [path, callback, result] of gets) { From 2a037ddbf28bd2b05010f8334b673f8b20bcaff4 Mon Sep 17 00:00:00 2001 From: Francesco Novy Date: Mon, 12 Aug 2024 11:17:39 +0200 Subject: [PATCH 16/16] add comment --- .../suites/public-api/LocalVariables/test.ts | 2 ++ 1 file changed, 2 insertions(+) diff --git a/dev-packages/node-integration-tests/suites/public-api/LocalVariables/test.ts b/dev-packages/node-integration-tests/suites/public-api/LocalVariables/test.ts index 5ed60bf65284..2640ecf94461 100644 --- a/dev-packages/node-integration-tests/suites/public-api/LocalVariables/test.ts +++ b/dev-packages/node-integration-tests/suites/public-api/LocalVariables/test.ts @@ -3,6 +3,8 @@ import * as path from 'path'; import { conditionalTest } from '../../../utils'; import { cleanupChildProcesses, createRunner } from '../../../utils/runner'; +// This test takes some time because it connects the debugger etc. +// So we increase the timeout here jest.setTimeout(45_000); const EXPECTED_LOCAL_VARIABLES_EVENT = {