From 41a6b6706ae4d628e22a9ae7c2baa4acc9e1e156 Mon Sep 17 00:00:00 2001 From: Tim Fish Date: Tue, 9 Jan 2024 12:58:43 +0100 Subject: [PATCH 01/17] test(node): New ee2e test runner --- .../node-integration-tests/package.json | 1 + .../node-integration-tests/suites/anr/test.ts | 356 +++++++++--------- .../LocalVariables/local-variables-caught.js | 6 +- .../LocalVariables/local-variables-caught.mjs | 6 +- .../local-variables-memory-test.js | 5 +- .../LocalVariables/local-variables.js | 6 +- .../LocalVariables/no-local-variables.js | 6 +- .../suites/public-api/LocalVariables/test.ts | 186 ++++----- .../node-integration-tests/utils/runner.ts | 153 ++++++++ packages/node/src/integrations/anr/worker.ts | 8 +- packages/utils/src/index.ts | 1 + packages/utils/src/transport.ts | 17 + 12 files changed, 441 insertions(+), 310 deletions(-) create mode 100644 dev-packages/node-integration-tests/utils/runner.ts create mode 100644 packages/utils/src/transport.ts diff --git a/dev-packages/node-integration-tests/package.json b/dev-packages/node-integration-tests/package.json index c186c7fbfeb6..3ddebb265fb7 100644 --- a/dev-packages/node-integration-tests/package.json +++ b/dev-packages/node-integration-tests/package.json @@ -15,6 +15,7 @@ "type-check": "tsc", "pretest": "run-s --silent prisma:init prisma:init:new", "test": "ts-node ./utils/run-tests.ts", + "jest": "jest --config ./jest.config.js", "test:watch": "yarn test --watch" }, "dependencies": { diff --git a/dev-packages/node-integration-tests/suites/anr/test.ts b/dev-packages/node-integration-tests/suites/anr/test.ts index 2c2e513559cf..b4d4ac414cd0 100644 --- a/dev-packages/node-integration-tests/suites/anr/test.ts +++ b/dev-packages/node-integration-tests/suites/anr/test.ts @@ -1,188 +1,176 @@ -import * as childProcess from 'child_process'; -import * as path from 'path'; -import type { Event } from '@sentry/node'; -import type { SerializedSession } from '@sentry/types'; import { conditionalTest } from '../../utils'; - -/** The output will contain logging so we need to find the line that parses as JSON */ -function parseJsonLines(input: string, expected: number): T { - const results = input - .split('\n') - .map(line => { - const trimmed = line.startsWith('[ANR Worker] ') ? line.slice(13) : line; - try { - return JSON.parse(trimmed) as T; - } catch { - return undefined; - } - }) - .filter(a => a) as T; - - expect(results.length).toEqual(expected); - - return results; -} +import { assertSentryEvent, assertSentrySession, createRunner } from '../../utils/runner'; + +const ANR_TEST_TIMEOUT = 10_000; + +const EXPECTED_ANR_EVENT = { + // Ensure we have context + contexts: { + trace: { + span_id: expect.any(String), + trace_id: expect.any(String), + }, + device: { + arch: expect.any(String), + }, + app: { + app_start_time: expect.any(String), + }, + os: { + name: expect.any(String), + }, + culture: { + timezone: expect.any(String), + }, + }, + // and an exception that is our ANR + exception: { + values: [ + { + type: 'ApplicationNotResponding', + value: 'Application Not Responding for at least 200 ms', + mechanism: { type: 'ANR' }, + stacktrace: { + frames: expect.arrayContaining([ + { + colno: expect.any(Number), + lineno: expect.any(Number), + filename: expect.any(String), + function: '?', + in_app: true, + }, + { + colno: expect.any(Number), + lineno: expect.any(Number), + filename: expect.any(String), + function: 'longWork', + in_app: true, + }, + ]), + }, + }, + ], + }, +}; conditionalTest({ min: 16 })('should report ANR when event loop blocked', () => { - test('CJS', done => { - expect.assertions(13); - - const testScriptPath = path.resolve(__dirname, 'basic.js'); - - childProcess.exec(`node ${testScriptPath}`, { encoding: 'utf8' }, (_, stdout) => { - const [event] = parseJsonLines<[Event]>(stdout, 1); - - expect(event.exception?.values?.[0].mechanism).toEqual({ type: 'ANR' }); - expect(event.exception?.values?.[0].type).toEqual('ApplicationNotResponding'); - expect(event.exception?.values?.[0].value).toEqual('Application Not Responding for at least 200 ms'); - expect(event.exception?.values?.[0].stacktrace?.frames?.length).toBeGreaterThan(4); - - expect(event.exception?.values?.[0].stacktrace?.frames?.[2].function).toEqual('?'); - expect(event.exception?.values?.[0].stacktrace?.frames?.[3].function).toEqual('longWork'); - - expect(event.contexts?.trace?.trace_id).toBeDefined(); - expect(event.contexts?.trace?.span_id).toBeDefined(); - - expect(event.contexts?.device?.arch).toBeDefined(); - expect(event.contexts?.app?.app_start_time).toBeDefined(); - expect(event.contexts?.os?.name).toBeDefined(); - expect(event.contexts?.culture?.timezone).toBeDefined(); - - done(); - }); - }); - - test('Legacy API', done => { - // TODO (v8): Remove this old API and this test - expect.assertions(9); - - const testScriptPath = path.resolve(__dirname, 'legacy.js'); - - childProcess.exec(`node ${testScriptPath}`, { encoding: 'utf8' }, (_, stdout) => { - const [event] = parseJsonLines<[Event]>(stdout, 1); - - expect(event.exception?.values?.[0].mechanism).toEqual({ type: 'ANR' }); - expect(event.exception?.values?.[0].type).toEqual('ApplicationNotResponding'); - expect(event.exception?.values?.[0].value).toEqual('Application Not Responding for at least 200 ms'); - expect(event.exception?.values?.[0].stacktrace?.frames?.length).toBeGreaterThan(4); - - expect(event.exception?.values?.[0].stacktrace?.frames?.[2].function).toEqual('?'); - expect(event.exception?.values?.[0].stacktrace?.frames?.[3].function).toEqual('longWork'); - - expect(event.contexts?.trace?.trace_id).toBeDefined(); - expect(event.contexts?.trace?.span_id).toBeDefined(); - - done(); - }); - }); - - test('ESM', done => { - expect.assertions(7); - - const testScriptPath = path.resolve(__dirname, 'basic.mjs'); - - childProcess.exec(`node ${testScriptPath}`, { encoding: 'utf8' }, (_, stdout) => { - const [event] = parseJsonLines<[Event]>(stdout, 1); - - expect(event.exception?.values?.[0].mechanism).toEqual({ type: 'ANR' }); - expect(event.exception?.values?.[0].type).toEqual('ApplicationNotResponding'); - expect(event.exception?.values?.[0].value).toEqual('Application Not Responding for at least 200 ms'); - expect(event.exception?.values?.[0].stacktrace?.frames?.length).toBeGreaterThanOrEqual(4); - expect(event.exception?.values?.[0].stacktrace?.frames?.[2].function).toEqual('?'); - expect(event.exception?.values?.[0].stacktrace?.frames?.[3].function).toEqual('longWork'); - - done(); - }); - }); - - test('With --inspect', done => { - expect.assertions(7); - - const testScriptPath = path.resolve(__dirname, 'basic.js'); - - childProcess.exec(`node --inspect ${testScriptPath}`, { encoding: 'utf8' }, (_, stdout) => { - const [event] = parseJsonLines<[Event]>(stdout, 1); - - expect(event.exception?.values?.[0].mechanism).toEqual({ type: 'ANR' }); - expect(event.exception?.values?.[0].type).toEqual('ApplicationNotResponding'); - expect(event.exception?.values?.[0].value).toEqual('Application Not Responding for at least 200 ms'); - expect(event.exception?.values?.[0].stacktrace?.frames?.length).toBeGreaterThan(4); - - expect(event.exception?.values?.[0].stacktrace?.frames?.[2].function).toEqual('?'); - expect(event.exception?.values?.[0].stacktrace?.frames?.[3].function).toEqual('longWork'); - - done(); - }); - }); - - test('should exit', done => { - const testScriptPath = path.resolve(__dirname, 'should-exit.js'); - let hasClosed = false; - - setTimeout(() => { - expect(hasClosed).toBe(true); - done(); - }, 5_000); - - childProcess.exec(`node ${testScriptPath}`, { encoding: 'utf8' }, () => { - hasClosed = true; - }); - }); - - test('should exit forced', done => { - const testScriptPath = path.resolve(__dirname, 'should-exit-forced.js'); - let hasClosed = false; - - setTimeout(() => { - expect(hasClosed).toBe(true); - done(); - }, 5_000); - - childProcess.exec(`node ${testScriptPath}`, { encoding: 'utf8' }, () => { - hasClosed = true; - }); - }); - - test('With session', done => { - expect.assertions(9); - - const testScriptPath = path.resolve(__dirname, 'basic-session.js'); - - childProcess.exec(`node ${testScriptPath}`, { encoding: 'utf8' }, (_, stdout) => { - const [session, event] = parseJsonLines<[SerializedSession, Event]>(stdout, 2); - - expect(event.exception?.values?.[0].mechanism).toEqual({ type: 'ANR' }); - expect(event.exception?.values?.[0].type).toEqual('ApplicationNotResponding'); - expect(event.exception?.values?.[0].value).toEqual('Application Not Responding for at least 200 ms'); - expect(event.exception?.values?.[0].stacktrace?.frames?.length).toBeGreaterThan(4); - - expect(event.exception?.values?.[0].stacktrace?.frames?.[2].function).toEqual('?'); - expect(event.exception?.values?.[0].stacktrace?.frames?.[3].function).toEqual('longWork'); - - expect(session.status).toEqual('abnormal'); - expect(session.abnormal_mechanism).toEqual('anr_foreground'); - - done(); - }); - }); - - test('from forked process', done => { - expect.assertions(7); - - const testScriptPath = path.resolve(__dirname, 'forker.js'); - - childProcess.exec(`node ${testScriptPath}`, { encoding: 'utf8' }, (_, stdout) => { - const [event] = parseJsonLines<[Event]>(stdout, 1); - - expect(event.exception?.values?.[0].mechanism).toEqual({ type: 'ANR' }); - expect(event.exception?.values?.[0].type).toEqual('ApplicationNotResponding'); - expect(event.exception?.values?.[0].value).toEqual('Application Not Responding for at least 200 ms'); - expect(event.exception?.values?.[0].stacktrace?.frames?.length).toBeGreaterThan(4); - - expect(event.exception?.values?.[0].stacktrace?.frames?.[2].function).toEqual('?'); - expect(event.exception?.values?.[0].stacktrace?.frames?.[3].function).toEqual('longWork'); - - done(); - }); - }); + test( + 'CJS', + done => { + createRunner(__dirname, 'basic.js') + .expect({ + event: event => { + assertSentryEvent(event, EXPECTED_ANR_EVENT); + }, + }) + .start(done); + }, + ANR_TEST_TIMEOUT, + ); + + // TODO (v8): Remove this old API and this test + test( + 'Legacy API', + done => { + createRunner(__dirname, 'legacy.js') + .expect({ + event: event => { + assertSentryEvent(event, EXPECTED_ANR_EVENT); + }, + }) + .start(done); + }, + ANR_TEST_TIMEOUT, + ); + + test( + 'ESM', + done => { + createRunner(__dirname, 'basic.mjs') + .expect({ + event: event => { + assertSentryEvent(event, EXPECTED_ANR_EVENT); + }, + }) + .start(done); + }, + ANR_TEST_TIMEOUT, + ); + + test( + 'With --inspect', + done => { + createRunner(__dirname, 'basic.mjs') + .withFlags('--inspect') + .expect({ + event: event => { + assertSentryEvent(event, EXPECTED_ANR_EVENT); + }, + }) + .start(done); + }, + ANR_TEST_TIMEOUT, + ); + + test( + 'should exit', + done => { + const runner = createRunner(__dirname, 'should-exit.js').start(); + + setTimeout(() => { + expect(runner.childHasExited()).toBe(true); + done(); + }, 5_000); + }, + ANR_TEST_TIMEOUT, + ); + + test( + 'should exit forced', + done => { + const runner = createRunner(__dirname, 'should-exit-forced.js').start(); + + setTimeout(() => { + expect(runner.childHasExited()).toBe(true); + done(); + }, 5_000); + }, + ANR_TEST_TIMEOUT, + ); + + test( + 'With session', + done => { + createRunner(__dirname, 'basic-session.js') + .expect({ + session: session => { + assertSentrySession(session, { + status: 'abnormal', + abnormal_mechanism: 'anr_foreground', + }); + }, + }) + .expect({ + event: event => { + assertSentryEvent(event, EXPECTED_ANR_EVENT); + }, + }) + .start(done); + }, + ANR_TEST_TIMEOUT, + ); + + test( + 'from forked process', + done => { + createRunner(__dirname, 'forker.js') + .expect({ + event: event => { + assertSentryEvent(event, EXPECTED_ANR_EVENT); + }, + }) + .start(done); + }, + ANR_TEST_TIMEOUT, + ); }); diff --git a/dev-packages/node-integration-tests/suites/public-api/LocalVariables/local-variables-caught.js b/dev-packages/node-integration-tests/suites/public-api/LocalVariables/local-variables-caught.js index 08a8d81383a1..beba34dbb9b3 100644 --- a/dev-packages/node-integration-tests/suites/public-api/LocalVariables/local-variables-caught.js +++ b/dev-packages/node-integration-tests/suites/public-api/LocalVariables/local-variables-caught.js @@ -1,13 +1,11 @@ /* eslint-disable no-unused-vars */ const Sentry = require('@sentry/node'); +const { loggingTransport } = require('@sentry/utils'); Sentry.init({ dsn: 'https://public@dsn.ingest.sentry.io/1337', includeLocalVariables: true, - beforeSend: event => { - // eslint-disable-next-line no-console - console.log(JSON.stringify(event)); - }, + transport: loggingTransport, }); class Some { diff --git a/dev-packages/node-integration-tests/suites/public-api/LocalVariables/local-variables-caught.mjs b/dev-packages/node-integration-tests/suites/public-api/LocalVariables/local-variables-caught.mjs index 3fbf2ae69df7..aaa12880afd0 100644 --- a/dev-packages/node-integration-tests/suites/public-api/LocalVariables/local-variables-caught.mjs +++ b/dev-packages/node-integration-tests/suites/public-api/LocalVariables/local-variables-caught.mjs @@ -1,13 +1,11 @@ /* eslint-disable no-unused-vars */ import * as Sentry from '@sentry/node'; +import { loggingTransport } from '@sentry/utils'; Sentry.init({ dsn: 'https://public@dsn.ingest.sentry.io/1337', includeLocalVariables: true, - beforeSend: event => { - // eslint-disable-next-line no-console - console.log(JSON.stringify(event)); - }, + transport: loggingTransport, }); class Some { diff --git a/dev-packages/node-integration-tests/suites/public-api/LocalVariables/local-variables-memory-test.js b/dev-packages/node-integration-tests/suites/public-api/LocalVariables/local-variables-memory-test.js index 7b227d4d08de..ec8ae6630a53 100644 --- a/dev-packages/node-integration-tests/suites/public-api/LocalVariables/local-variables-memory-test.js +++ b/dev-packages/node-integration-tests/suites/public-api/LocalVariables/local-variables-memory-test.js @@ -1,12 +1,11 @@ /* eslint-disable no-unused-vars */ const Sentry = require('@sentry/node'); +const { loggingTransport } = require('@sentry/utils'); Sentry.init({ dsn: 'https://public@dsn.ingest.sentry.io/1337', includeLocalVariables: true, - beforeSend: _ => { - return null; - }, + transport: loggingTransport, // Stop the rate limiting from kicking in integrations: [new Sentry.Integrations.LocalVariables({ maxExceptionsPerSecond: 10000000 })], }); diff --git a/dev-packages/node-integration-tests/suites/public-api/LocalVariables/local-variables.js b/dev-packages/node-integration-tests/suites/public-api/LocalVariables/local-variables.js index a579a9cf5ff0..6200d7a38a7d 100644 --- a/dev-packages/node-integration-tests/suites/public-api/LocalVariables/local-variables.js +++ b/dev-packages/node-integration-tests/suites/public-api/LocalVariables/local-variables.js @@ -1,13 +1,11 @@ /* eslint-disable no-unused-vars */ const Sentry = require('@sentry/node'); +const { loggingTransport } = require('@sentry/utils'); Sentry.init({ dsn: 'https://public@dsn.ingest.sentry.io/1337', includeLocalVariables: true, - beforeSend: event => { - // eslint-disable-next-line no-console - console.log(JSON.stringify(event)); - }, + transport: loggingTransport, }); process.on('uncaughtException', () => { diff --git a/dev-packages/node-integration-tests/suites/public-api/LocalVariables/no-local-variables.js b/dev-packages/node-integration-tests/suites/public-api/LocalVariables/no-local-variables.js index e9f189647e1a..c4cccfb4ce69 100644 --- a/dev-packages/node-integration-tests/suites/public-api/LocalVariables/no-local-variables.js +++ b/dev-packages/node-integration-tests/suites/public-api/LocalVariables/no-local-variables.js @@ -1,12 +1,10 @@ /* eslint-disable no-unused-vars */ const Sentry = require('@sentry/node'); +const { loggingTransport } = require('@sentry/utils'); Sentry.init({ dsn: 'https://public@dsn.ingest.sentry.io/1337', - beforeSend: event => { - // eslint-disable-next-line no-console - console.log(JSON.stringify(event)); - }, + transport: loggingTransport, }); process.on('uncaughtException', () => { 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 1c58fd802122..71b203e94ef7 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 @@ -1,113 +1,93 @@ import * as childProcess from 'child_process'; import * as path from 'path'; -import type { Event } from '@sentry/node'; - import { conditionalTest } from '../../../utils'; +import { assertSentryEvent, createRunner } from '../../../utils/runner'; + +const LOCAL_VARIABLES_TEST_TIMEOUT = 5_000; +const EXPECTED_LOCAL_VARIABLES_EVENT = { + exception: { + values: [ + { + stacktrace: { + frames: expect.arrayContaining([ + expect.objectContaining({ + function: 'one', + vars: { + name: 'some name', + arr: [1, '2', null], + obj: { name: 'some name', num: 5 }, + ty: '', + }, + }), + expect.objectContaining({ + function: 'Some.two', + vars: { name: 'some name' }, + }), + ]), + }, + }, + ], + }, +}; conditionalTest({ min: 18 })('LocalVariables integration', () => { - test('Should not include local variables by default', done => { - expect.assertions(2); - - const testScriptPath = path.resolve(__dirname, 'no-local-variables.js'); - - childProcess.exec(`node ${testScriptPath}`, { encoding: 'utf8' }, (_, stdout) => { - const event = JSON.parse(stdout) as Event; - - const frames = event.exception?.values?.[0].stacktrace?.frames || []; - const lastFrame = frames[frames.length - 1]; - - expect(lastFrame.vars).toBeUndefined(); - - const penultimateFrame = frames[frames.length - 2]; - - expect(penultimateFrame.vars).toBeUndefined(); - - done(); - }); - }); - - test('Should include local variables when enabled', done => { - expect.assertions(4); - - const testScriptPath = path.resolve(__dirname, 'local-variables.js'); - - childProcess.exec(`node ${testScriptPath}`, { encoding: 'utf8' }, (_, stdout) => { - const event = JSON.parse(stdout) as Event; - - const frames = event.exception?.values?.[0].stacktrace?.frames || []; - const lastFrame = frames[frames.length - 1]; - - expect(lastFrame.function).toBe('Some.two'); - expect(lastFrame.vars).toEqual({ name: 'some name' }); - - const penultimateFrame = frames[frames.length - 2]; - - expect(penultimateFrame.function).toBe('one'); - expect(penultimateFrame.vars).toEqual({ - name: 'some name', - arr: [1, '2', null], - obj: { name: 'some name', num: 5 }, - ty: '', - }); - - done(); - }); - }); - - test('Should include local variables with ESM', done => { - expect.assertions(4); - - const testScriptPath = path.resolve(__dirname, 'local-variables-caught.mjs'); - - childProcess.exec(`node ${testScriptPath}`, { encoding: 'utf8' }, (_, stdout) => { - const event = JSON.parse(stdout) as Event; - - const frames = event.exception?.values?.[0].stacktrace?.frames || []; - const lastFrame = frames[frames.length - 1]; - - expect(lastFrame.function).toBe('Some.two'); - expect(lastFrame.vars).toEqual({ name: 'some name' }); - - const penultimateFrame = frames[frames.length - 2]; - - expect(penultimateFrame.function).toBe('one'); - expect(penultimateFrame.vars).toEqual({ - name: 'some name', - arr: [1, '2', null], - obj: { name: 'some name', num: 5 }, - ty: '', - }); - - done(); - }); - }); + test( + 'Should not include local variables by default', + done => { + createRunner(__dirname, 'no-local-variables.js') + .expect({ + event: event => { + const frames = event.exception?.values?.[0].stacktrace?.frames || []; + const lastFrame = frames[frames.length - 1]; + + expect(lastFrame.vars).toBeUndefined(); + + const penultimateFrame = frames[frames.length - 2]; + + expect(penultimateFrame.vars).toBeUndefined(); + }, + }) + .start(done); + }, + LOCAL_VARIABLES_TEST_TIMEOUT, + ); + + test( + 'Should include local variables when enabled', + done => { + createRunner(__dirname, 'local-variables.js') + .expect({ + event: event => { + assertSentryEvent(event, EXPECTED_LOCAL_VARIABLES_EVENT); + }, + }) + .start(done); + }, + LOCAL_VARIABLES_TEST_TIMEOUT, + ); + + test( + 'Should include local variables with ESM', + done => { + createRunner(__dirname, 'local-variables-caught.mjs') + .expect({ + event: event => { + assertSentryEvent(event, EXPECTED_LOCAL_VARIABLES_EVENT); + }, + }) + .start(done); + }, + LOCAL_VARIABLES_TEST_TIMEOUT, + ); test('Includes local variables for caught exceptions when enabled', done => { - expect.assertions(4); - - const testScriptPath = path.resolve(__dirname, 'local-variables-caught.js'); - - childProcess.exec(`node ${testScriptPath}`, { encoding: 'utf8' }, (_, stdout) => { - const event = JSON.parse(stdout) as Event; - - const frames = event.exception?.values?.[0].stacktrace?.frames || []; - const lastFrame = frames[frames.length - 1]; - - expect(lastFrame.function).toBe('Some.two'); - expect(lastFrame.vars).toEqual({ name: 'some name' }); - - const penultimateFrame = frames[frames.length - 2]; - - expect(penultimateFrame.function).toBe('one'); - expect(penultimateFrame.vars).toEqual({ - name: 'some name', - arr: [1, '2', null], - obj: { name: 'some name', num: 5 }, - ty: '', - }); - - done(); - }); + createRunner(__dirname, 'local-variables-caught.js') + .expect({ + event: event => { + assertSentryEvent(event, EXPECTED_LOCAL_VARIABLES_EVENT); + }, + }) + .start(done); }); test('Should not leak memory', done => { diff --git a/dev-packages/node-integration-tests/utils/runner.ts b/dev-packages/node-integration-tests/utils/runner.ts new file mode 100644 index 000000000000..73556c6fb63c --- /dev/null +++ b/dev-packages/node-integration-tests/utils/runner.ts @@ -0,0 +1,153 @@ +import { spawn } from 'child_process'; +import { join } from 'path'; +import type { Envelope, Event, SerializedSession } from '@sentry/types'; + +export function assertSentryEvent(actual: Event, expected: Event): void { + expect(actual).toMatchObject({ + event_id: expect.any(String), + ...expected, + }); +} + +export function assertSentrySession(actual: SerializedSession, expected: Partial): void { + expect(actual).toMatchObject({ + sid: expect.any(String), + ...expected, + }); +} + +type Expected = + | { + event: (event: Event) => void; + } + | { + transaction: (event: Event) => void; + } + | { + session: (event: SerializedSession) => void; + }; + +/** */ +// eslint-disable-next-line @typescript-eslint/explicit-function-return-type +export function createRunner(...paths: string[]) { + const testPath = join(...paths); + + const expectedEnvelopes: Expected[] = []; + const flags: string[] = []; + let hasExited = false; + + if (testPath.endsWith('.ts')) { + flags.push('-r', 'ts-node/register'); + } + + return { + expect: function (expected: Expected) { + expectedEnvelopes.push(expected); + return this; + }, + withFlags: function (...args: string[]) { + flags.push(...args); + return this; + }, + start: function (done?: (e?: unknown) => void) { + const expectedEnvelopeCount = expectedEnvelopes.length; + let envelopeCount = 0; + + const child = spawn('node', [...flags, testPath]); + + child.on('close', () => { + hasExited = true; + }); + + // Pass error to done to end the test quickly + child.on('error', e => { + done?.(e); + }); + + function checkDone(): void { + envelopeCount++; + if (envelopeCount === expectedEnvelopeCount) { + child.kill(); + done?.(); + } + } + + function tryParseLine(line: string): void { + // Lines can have leading '[something] [{' which we need to remove + const cleanedLine = line.replace(/^.*?] \[{"/, '[{"'); + + if (!cleanedLine.startsWith('[{')) { + return; + } + + let envelope: Envelope | undefined; + try { + envelope = JSON.parse(cleanedLine) as Envelope; + } catch (_) { + // + } + + if (!envelope) { + return; + } + + for (const item of envelope[1]) { + const envelopeItemType = item[0].type; + + const expected = expectedEnvelopes.shift(); + + // Catch any error or failed assertions and pass them to done + try { + if (!expected) { + throw new Error(`No more expected envelope items but we received a '${envelopeItemType}' item`); + } + + const expectedType = Object.keys(expected)[0]; + + if (expectedType !== envelopeItemType) { + throw new Error(`Expected envelope item type '${expectedType}' but got '${envelopeItemType}'`); + } + + if ('event' in expected) { + expected.event(item[1] as Event); + checkDone(); + } + + if ('transaction' in expected) { + expected.transaction(item[1] as Event); + checkDone(); + } + + if ('session' in expected) { + expected.session(item[1] as SerializedSession); + checkDone(); + } + } catch (e) { + done?.(e); + } + } + } + + let buffer = Buffer.alloc(0); + + child.stdout.on('data', (data: Buffer) => { + buffer = Buffer.concat([buffer, data]); + + let splitIndex = -1; + while ((splitIndex = buffer.indexOf(0xa)) >= 0) { + const line = buffer.subarray(0, splitIndex).toString(); + + buffer = Buffer.from(buffer.subarray(splitIndex + 1)); + + tryParseLine(line); + } + }); + + return { + childHasExited: function (): boolean { + return hasExited; + }, + }; + }, + }; +} diff --git a/packages/node/src/integrations/anr/worker.ts b/packages/node/src/integrations/anr/worker.ts index f87ff8bb672f..0a3f64b0c7ae 100644 --- a/packages/node/src/integrations/anr/worker.ts +++ b/packages/node/src/integrations/anr/worker.ts @@ -46,9 +46,9 @@ async function sendAbnormalSession(): Promise { log('Sending abnormal session'); updateSession(session, { status: 'abnormal', abnormal_mechanism: 'anr_foreground' }); - log(JSON.stringify(session)); - const envelope = createSessionEnvelope(session, options.dsn, options.sdkMetadata); + log(JSON.stringify(envelope)); + await transport.send(envelope); try { @@ -117,9 +117,9 @@ async function sendAnrEvent(frames?: StackFrame[], traceContext?: TraceContext): tags: options.staticTags, }; - log(JSON.stringify(event)); - const envelope = createEventEnvelope(event, options.dsn, options.sdkMetadata); + log(JSON.stringify(envelope)); + await transport.send(envelope); await transport.flush(2000); diff --git a/packages/utils/src/index.ts b/packages/utils/src/index.ts index 5483f2aa7e41..030825b43210 100644 --- a/packages/utils/src/index.ts +++ b/packages/utils/src/index.ts @@ -36,3 +36,4 @@ export * from './parameterize'; export * from './anr'; export * from './lru'; export * from './buildPolyfills'; +export * from './transport'; diff --git a/packages/utils/src/transport.ts b/packages/utils/src/transport.ts new file mode 100644 index 000000000000..2f9cc3098b23 --- /dev/null +++ b/packages/utils/src/transport.ts @@ -0,0 +1,17 @@ +import type { BaseTransportOptions, Envelope, Transport, TransportMakeRequestResponse } from '@sentry/types'; + +/** + * Debug logging transport + */ +export function loggingTransport(_options: BaseTransportOptions): Transport { + return { + send(request: Envelope): Promise { + // eslint-disable-next-line no-console + console.log(JSON.stringify(request)); + return Promise.resolve({ statusCode: 200 }); + }, + flush(): PromiseLike { + return Promise.resolve(true); + }, + }; +} From abb74bf2cd8de21c81c684d4e33f94e429a90bfa Mon Sep 17 00:00:00 2001 From: Tim Fish Date: Tue, 9 Jan 2024 13:23:52 +0100 Subject: [PATCH 02/17] Fix lint --- dev-packages/node-integration-tests/tsconfig.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dev-packages/node-integration-tests/tsconfig.json b/dev-packages/node-integration-tests/tsconfig.json index 782d8f9c517f..7fb7682893f9 100644 --- a/dev-packages/node-integration-tests/tsconfig.json +++ b/dev-packages/node-integration-tests/tsconfig.json @@ -6,6 +6,6 @@ "compilerOptions": { // package-specific options "esModuleInterop": true, - "types": ["node"] + "types": ["node", "jest"] } } From f15e1ada060b1cb429b4ed6a624c87408b3e2a9c Mon Sep 17 00:00:00 2001 From: Tim Fish Date: Tue, 9 Jan 2024 13:45:23 +0100 Subject: [PATCH 03/17] Allow ignoring of specific envelope item types --- .../suites/public-api/LocalVariables/test.ts | 4 ++++ dev-packages/node-integration-tests/utils/runner.ts | 11 ++++++++++- 2 files changed, 14 insertions(+), 1 deletion(-) 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 71b203e94ef7..fac3bcb3532f 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 @@ -35,6 +35,7 @@ conditionalTest({ min: 18 })('LocalVariables integration', () => { 'Should not include local variables by default', done => { createRunner(__dirname, 'no-local-variables.js') + .ignore('session') .expect({ event: event => { const frames = event.exception?.values?.[0].stacktrace?.frames || []; @@ -56,6 +57,7 @@ conditionalTest({ min: 18 })('LocalVariables integration', () => { 'Should include local variables when enabled', done => { createRunner(__dirname, 'local-variables.js') + .ignore('session') .expect({ event: event => { assertSentryEvent(event, EXPECTED_LOCAL_VARIABLES_EVENT); @@ -70,6 +72,7 @@ conditionalTest({ min: 18 })('LocalVariables integration', () => { 'Should include local variables with ESM', done => { createRunner(__dirname, 'local-variables-caught.mjs') + .ignore('session') .expect({ event: event => { assertSentryEvent(event, EXPECTED_LOCAL_VARIABLES_EVENT); @@ -82,6 +85,7 @@ conditionalTest({ min: 18 })('LocalVariables integration', () => { test('Includes local variables for caught exceptions when enabled', done => { createRunner(__dirname, 'local-variables-caught.js') + .ignore('session') .expect({ event: event => { assertSentryEvent(event, EXPECTED_LOCAL_VARIABLES_EVENT); diff --git a/dev-packages/node-integration-tests/utils/runner.ts b/dev-packages/node-integration-tests/utils/runner.ts index 73556c6fb63c..dbfb9cbf10bf 100644 --- a/dev-packages/node-integration-tests/utils/runner.ts +++ b/dev-packages/node-integration-tests/utils/runner.ts @@ -1,6 +1,6 @@ import { spawn } from 'child_process'; import { join } from 'path'; -import type { Envelope, Event, SerializedSession } from '@sentry/types'; +import type { Envelope, EnvelopeItemType, Event, SerializedSession } from '@sentry/types'; export function assertSentryEvent(actual: Event, expected: Event): void { expect(actual).toMatchObject({ @@ -34,6 +34,7 @@ export function createRunner(...paths: string[]) { const expectedEnvelopes: Expected[] = []; const flags: string[] = []; + const ignored: EnvelopeItemType[] = []; let hasExited = false; if (testPath.endsWith('.ts')) { @@ -49,6 +50,10 @@ export function createRunner(...paths: string[]) { flags.push(...args); return this; }, + ignore: function (...types: EnvelopeItemType[]) { + ignored.push(...types); + return this; + }, start: function (done?: (e?: unknown) => void) { const expectedEnvelopeCount = expectedEnvelopes.length; let envelopeCount = 0; @@ -94,6 +99,10 @@ export function createRunner(...paths: string[]) { for (const item of envelope[1]) { const envelopeItemType = item[0].type; + if (ignored.includes(envelopeItemType)) { + continue; + } + const expected = expectedEnvelopes.shift(); // Catch any error or failed assertions and pass them to done From 4da924f7dd94634db1bd9f90356877ba8415bc4c Mon Sep 17 00:00:00 2001 From: Tim Fish Date: Tue, 9 Jan 2024 14:17:36 +0100 Subject: [PATCH 04/17] Move `loggingTransport` to `@sentry-internal/node-integration-tests` --- dev-packages/node-integration-tests/.eslintrc.js | 2 +- dev-packages/node-integration-tests/package.json | 6 ++++++ dev-packages/node-integration-tests/rollup.npm.config.mjs | 3 +++ .../node-integration-tests/src/index.ts | 0 .../public-api/LocalVariables/local-variables-caught.js | 2 +- .../public-api/LocalVariables/local-variables-caught.mjs | 2 +- .../LocalVariables/local-variables-memory-test.js | 2 +- .../suites/public-api/LocalVariables/local-variables.js | 2 +- .../suites/public-api/LocalVariables/no-local-variables.js | 2 +- dev-packages/node-integration-tests/tsconfig.json | 2 +- packages/utils/src/index.ts | 1 - 11 files changed, 16 insertions(+), 8 deletions(-) create mode 100644 dev-packages/node-integration-tests/rollup.npm.config.mjs rename packages/utils/src/transport.ts => dev-packages/node-integration-tests/src/index.ts (100%) diff --git a/dev-packages/node-integration-tests/.eslintrc.js b/dev-packages/node-integration-tests/.eslintrc.js index 6c8a493dccb3..df04aa267446 100644 --- a/dev-packages/node-integration-tests/.eslintrc.js +++ b/dev-packages/node-integration-tests/.eslintrc.js @@ -6,7 +6,7 @@ module.exports = { extends: ['../../.eslintrc.js'], overrides: [ { - files: ['utils/**/*.ts'], + files: ['utils/**/*.ts', 'src/**/*.ts'], parserOptions: { project: ['tsconfig.json'], sourceType: 'module', diff --git a/dev-packages/node-integration-tests/package.json b/dev-packages/node-integration-tests/package.json index 3ddebb265fb7..04c1448eef9c 100644 --- a/dev-packages/node-integration-tests/package.json +++ b/dev-packages/node-integration-tests/package.json @@ -6,7 +6,13 @@ "node": ">=10" }, "private": true, + "main": "build/cjs/index.js", + "module": "build/esm/index.js", "scripts": { + "build": "run-s build:transpile build:types", + "build:dev": "yarn build", + "build:transpile": "rollup -c rollup.npm.config.mjs", + "build:types": "tsc -p tsconfig.types.json", "clean": "rimraf -g **/node_modules", "prisma:init": "(cd suites/tracing/prisma-orm && ts-node ./setup.ts)", "prisma:init:new": "(cd suites/tracing-new/prisma-orm && ts-node ./setup.ts)", diff --git a/dev-packages/node-integration-tests/rollup.npm.config.mjs b/dev-packages/node-integration-tests/rollup.npm.config.mjs new file mode 100644 index 000000000000..84a06f2fb64a --- /dev/null +++ b/dev-packages/node-integration-tests/rollup.npm.config.mjs @@ -0,0 +1,3 @@ +import { makeBaseNPMConfig, makeNPMConfigVariants } from '@sentry-internal/rollup-utils'; + +export default makeNPMConfigVariants(makeBaseNPMConfig()); diff --git a/packages/utils/src/transport.ts b/dev-packages/node-integration-tests/src/index.ts similarity index 100% rename from packages/utils/src/transport.ts rename to dev-packages/node-integration-tests/src/index.ts diff --git a/dev-packages/node-integration-tests/suites/public-api/LocalVariables/local-variables-caught.js b/dev-packages/node-integration-tests/suites/public-api/LocalVariables/local-variables-caught.js index beba34dbb9b3..7c86004da43b 100644 --- a/dev-packages/node-integration-tests/suites/public-api/LocalVariables/local-variables-caught.js +++ b/dev-packages/node-integration-tests/suites/public-api/LocalVariables/local-variables-caught.js @@ -1,6 +1,6 @@ /* eslint-disable no-unused-vars */ const Sentry = require('@sentry/node'); -const { loggingTransport } = require('@sentry/utils'); +const { loggingTransport } = require('@sentry-internal/node-integration-tests'); Sentry.init({ dsn: 'https://public@dsn.ingest.sentry.io/1337', diff --git a/dev-packages/node-integration-tests/suites/public-api/LocalVariables/local-variables-caught.mjs b/dev-packages/node-integration-tests/suites/public-api/LocalVariables/local-variables-caught.mjs index aaa12880afd0..7e7fe832f977 100644 --- a/dev-packages/node-integration-tests/suites/public-api/LocalVariables/local-variables-caught.mjs +++ b/dev-packages/node-integration-tests/suites/public-api/LocalVariables/local-variables-caught.mjs @@ -1,6 +1,6 @@ /* eslint-disable no-unused-vars */ import * as Sentry from '@sentry/node'; -import { loggingTransport } from '@sentry/utils'; +import { loggingTransport } from '@sentry-internal/node-integration-tests'; Sentry.init({ dsn: 'https://public@dsn.ingest.sentry.io/1337', diff --git a/dev-packages/node-integration-tests/suites/public-api/LocalVariables/local-variables-memory-test.js b/dev-packages/node-integration-tests/suites/public-api/LocalVariables/local-variables-memory-test.js index ec8ae6630a53..7aa9feb9ae2a 100644 --- a/dev-packages/node-integration-tests/suites/public-api/LocalVariables/local-variables-memory-test.js +++ b/dev-packages/node-integration-tests/suites/public-api/LocalVariables/local-variables-memory-test.js @@ -1,6 +1,6 @@ /* eslint-disable no-unused-vars */ const Sentry = require('@sentry/node'); -const { loggingTransport } = require('@sentry/utils'); +const { loggingTransport } = require('@sentry-internal/node-integration-tests'); Sentry.init({ dsn: 'https://public@dsn.ingest.sentry.io/1337', diff --git a/dev-packages/node-integration-tests/suites/public-api/LocalVariables/local-variables.js b/dev-packages/node-integration-tests/suites/public-api/LocalVariables/local-variables.js index 6200d7a38a7d..4a16ad89b5aa 100644 --- a/dev-packages/node-integration-tests/suites/public-api/LocalVariables/local-variables.js +++ b/dev-packages/node-integration-tests/suites/public-api/LocalVariables/local-variables.js @@ -1,6 +1,6 @@ /* eslint-disable no-unused-vars */ const Sentry = require('@sentry/node'); -const { loggingTransport } = require('@sentry/utils'); +const { loggingTransport } = require('@sentry-internal/node-integration-tests'); Sentry.init({ dsn: 'https://public@dsn.ingest.sentry.io/1337', diff --git a/dev-packages/node-integration-tests/suites/public-api/LocalVariables/no-local-variables.js b/dev-packages/node-integration-tests/suites/public-api/LocalVariables/no-local-variables.js index c4cccfb4ce69..f01e33a9cafa 100644 --- a/dev-packages/node-integration-tests/suites/public-api/LocalVariables/no-local-variables.js +++ b/dev-packages/node-integration-tests/suites/public-api/LocalVariables/no-local-variables.js @@ -1,6 +1,6 @@ /* eslint-disable no-unused-vars */ const Sentry = require('@sentry/node'); -const { loggingTransport } = require('@sentry/utils'); +const { loggingTransport } = require('@sentry-internal/node-integration-tests'); Sentry.init({ dsn: 'https://public@dsn.ingest.sentry.io/1337', diff --git a/dev-packages/node-integration-tests/tsconfig.json b/dev-packages/node-integration-tests/tsconfig.json index 7fb7682893f9..92db70d5ca09 100644 --- a/dev-packages/node-integration-tests/tsconfig.json +++ b/dev-packages/node-integration-tests/tsconfig.json @@ -1,7 +1,7 @@ { "extends": "../../tsconfig.json", - "include": ["utils/**/*.ts"], + "include": ["utils/**/*.ts", "src/**/*.ts"], "compilerOptions": { // package-specific options diff --git a/packages/utils/src/index.ts b/packages/utils/src/index.ts index 030825b43210..5483f2aa7e41 100644 --- a/packages/utils/src/index.ts +++ b/packages/utils/src/index.ts @@ -36,4 +36,3 @@ export * from './parameterize'; export * from './anr'; export * from './lru'; export * from './buildPolyfills'; -export * from './transport'; From 005c65bbdabbf1d3aa3c9e715c977d157b1025f0 Mon Sep 17 00:00:00 2001 From: Tim Fish Date: Tue, 9 Jan 2024 15:03:30 +0100 Subject: [PATCH 05/17] Allow building of types --- .../node-integration-tests/tsconfig.types.json | 10 ++++++++++ 1 file changed, 10 insertions(+) create mode 100644 dev-packages/node-integration-tests/tsconfig.types.json diff --git a/dev-packages/node-integration-tests/tsconfig.types.json b/dev-packages/node-integration-tests/tsconfig.types.json new file mode 100644 index 000000000000..65455f66bd75 --- /dev/null +++ b/dev-packages/node-integration-tests/tsconfig.types.json @@ -0,0 +1,10 @@ +{ + "extends": "./tsconfig.json", + + "compilerOptions": { + "declaration": true, + "declarationMap": true, + "emitDeclarationOnly": true, + "outDir": "build/types" + } +} From 24a0b206d1c427afd1a10e4b7c566a6eba7892bb Mon Sep 17 00:00:00 2001 From: Tim Fish Date: Tue, 9 Jan 2024 15:27:48 +0100 Subject: [PATCH 06/17] Longer tests --- .../suites/public-api/LocalVariables/test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 fac3bcb3532f..407fd3e1fa09 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,7 +3,7 @@ import * as path from 'path'; import { conditionalTest } from '../../../utils'; import { assertSentryEvent, createRunner } from '../../../utils/runner'; -const LOCAL_VARIABLES_TEST_TIMEOUT = 5_000; +const LOCAL_VARIABLES_TEST_TIMEOUT = 20_000; const EXPECTED_LOCAL_VARIABLES_EVENT = { exception: { values: [ From fd992b4d06f772f68b9e4b0c080baca145bbce09 Mon Sep 17 00:00:00 2001 From: Tim Fish Date: Tue, 9 Jan 2024 15:29:17 +0100 Subject: [PATCH 07/17] fix lint --- .../suites/public-api/LocalVariables/local-variables-caught.mjs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dev-packages/node-integration-tests/suites/public-api/LocalVariables/local-variables-caught.mjs b/dev-packages/node-integration-tests/suites/public-api/LocalVariables/local-variables-caught.mjs index 7e7fe832f977..37e5966bc575 100644 --- a/dev-packages/node-integration-tests/suites/public-api/LocalVariables/local-variables-caught.mjs +++ b/dev-packages/node-integration-tests/suites/public-api/LocalVariables/local-variables-caught.mjs @@ -1,6 +1,6 @@ /* eslint-disable no-unused-vars */ -import * as Sentry from '@sentry/node'; import { loggingTransport } from '@sentry-internal/node-integration-tests'; +import * as Sentry from '@sentry/node'; Sentry.init({ dsn: 'https://public@dsn.ingest.sentry.io/1337', From 111a7c2aae5d6a452f33510bfb5257fbf38364d8 Mon Sep 17 00:00:00 2001 From: Tim Fish Date: Tue, 9 Jan 2024 16:14:01 +0100 Subject: [PATCH 08/17] Build utils before testing --- dev-packages/node-integration-tests/package.json | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/dev-packages/node-integration-tests/package.json b/dev-packages/node-integration-tests/package.json index 04c1448eef9c..9e67a45e106e 100644 --- a/dev-packages/node-integration-tests/package.json +++ b/dev-packages/node-integration-tests/package.json @@ -8,6 +8,7 @@ "private": true, "main": "build/cjs/index.js", "module": "build/esm/index.js", + "types": "build/types/src/index.d.ts", "scripts": { "build": "run-s build:transpile build:types", "build:dev": "yarn build", @@ -19,7 +20,7 @@ "lint": "eslint . --format stylish", "fix": "eslint . --format stylish --fix", "type-check": "tsc", - "pretest": "run-s --silent prisma:init prisma:init:new", + "pretest": "run-s --silent build prisma:init prisma:init:new", "test": "ts-node ./utils/run-tests.ts", "jest": "jest --config ./jest.config.js", "test:watch": "yarn test --watch" From 48587cd2569bb8d9292148694645803ad833f669 Mon Sep 17 00:00:00 2001 From: Tim Fish Date: Tue, 9 Jan 2024 16:29:52 +0100 Subject: [PATCH 09/17] Ensure e2e utils are cached on build --- .github/workflows/build.yml | 1 + dev-packages/node-integration-tests/package.json | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 413dd3d88aa3..20d26be494df 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -35,6 +35,7 @@ env: # packages/utils/cjs and packages/utils/esm: Symlinks to the folders inside of `build`, needed for tests CACHED_BUILD_PATHS: | + ${{ github.workspace }}/dev-packages/*/build ${{ github.workspace }}/packages/*/build ${{ github.workspace }}/packages/ember/*.d.ts ${{ github.workspace }}/packages/gatsby/*.d.ts diff --git a/dev-packages/node-integration-tests/package.json b/dev-packages/node-integration-tests/package.json index 9e67a45e106e..2f28ebadf913 100644 --- a/dev-packages/node-integration-tests/package.json +++ b/dev-packages/node-integration-tests/package.json @@ -20,7 +20,7 @@ "lint": "eslint . --format stylish", "fix": "eslint . --format stylish --fix", "type-check": "tsc", - "pretest": "run-s --silent build prisma:init prisma:init:new", + "pretest": "run-s --silent prisma:init prisma:init:new", "test": "ts-node ./utils/run-tests.ts", "jest": "jest --config ./jest.config.js", "test:watch": "yarn test --watch" From 120cc5b92e0eae9be5e797446a1004f15c0423a7 Mon Sep 17 00:00:00 2001 From: Tim Fish Date: Tue, 9 Jan 2024 16:42:09 +0100 Subject: [PATCH 10/17] support server in scenario and making requests to it --- .../node-integration-tests/src/index.ts | 14 ++ .../suites/express/tracing/server.ts | 4 +- .../suites/express/tracing/test.ts | 218 ++++++++++-------- .../node-integration-tests/utils/runner.ts | 49 ++++ packages/node/src/integrations/anr/worker.ts | 2 + 5 files changed, 185 insertions(+), 102 deletions(-) diff --git a/dev-packages/node-integration-tests/src/index.ts b/dev-packages/node-integration-tests/src/index.ts index 2f9cc3098b23..423b1ac3d93f 100644 --- a/dev-packages/node-integration-tests/src/index.ts +++ b/dev-packages/node-integration-tests/src/index.ts @@ -1,4 +1,6 @@ +import type { AddressInfo } from 'net'; import type { BaseTransportOptions, Envelope, Transport, TransportMakeRequestResponse } from '@sentry/types'; +import type { Express } from 'express'; /** * Debug logging transport @@ -15,3 +17,15 @@ export function loggingTransport(_options: BaseTransportOptions): Transport { }, }; } + +/** + * Starts an express server and sends the port to the runner + */ +export function startExpressServerAndSendPortToRunner(app: Express): void { + const server = app.listen(0, () => { + const address = server.address() as AddressInfo; + + // eslint-disable-next-line no-console + console.log(`{"port":${address.port}}`); + }); +} diff --git a/dev-packages/node-integration-tests/suites/express/tracing/server.ts b/dev-packages/node-integration-tests/suites/express/tracing/server.ts index 1c56a81fef98..dfd6df7526fd 100644 --- a/dev-packages/node-integration-tests/suites/express/tracing/server.ts +++ b/dev-packages/node-integration-tests/suites/express/tracing/server.ts @@ -1,3 +1,4 @@ +import { loggingTransport, startExpressServerAndSendPortToRunner } from '@sentry-internal/node-integration-tests'; import * as Sentry from '@sentry/node'; import cors from 'cors'; import express from 'express'; @@ -11,6 +12,7 @@ Sentry.init({ tracePropagationTargets: [/^(?!.*test).*$/], integrations: [new Sentry.Integrations.Http({ tracing: true }), new Sentry.Integrations.Express({ app })], tracesSampleRate: 1.0, + transport: loggingTransport, }); app.use(Sentry.Handlers.requestHandler()); @@ -36,4 +38,4 @@ app.get(['/test/arr/:id', /\/test\/arr[0-9]*\/required(path)?(\/optionalPath)?\/ app.use(Sentry.Handlers.errorHandler()); -export default app; +startExpressServerAndSendPortToRunner(app); diff --git a/dev-packages/node-integration-tests/suites/express/tracing/test.ts b/dev-packages/node-integration-tests/suites/express/tracing/test.ts index e391ca881b30..825bd2daf168 100644 --- a/dev-packages/node-integration-tests/suites/express/tracing/test.ts +++ b/dev-packages/node-integration-tests/suites/express/tracing/test.ts @@ -1,89 +1,101 @@ -import { TestEnv, assertSentryTransaction } from '../../../utils/index'; +import { assertSentryTransaction, createRunner } from '../../../utils/runner'; -test('should create and send transactions for Express routes and spans for middlewares.', async () => { - const env = await TestEnv.init(__dirname, `${__dirname}/server.ts`); - const envelope = await env.getEnvelopeRequest({ url: `${env.url}/express`, envelopeType: 'transaction' }); - - expect(envelope).toHaveLength(3); - - assertSentryTransaction(envelope[2], { - contexts: { - trace: { - data: { - url: '/test/express', - 'http.response.status_code': 200, - }, - op: 'http.server', - status: 'ok', - tags: { - 'http.status_code': '200', - }, - }, - }, - spans: [ - { - description: 'corsMiddleware', - op: 'middleware.express.use', +test('should create and send transactions for Express routes and spans for middlewares.', done => { + createRunner(__dirname, 'server.ts') + .expect({ + transaction: transaction => { + assertSentryTransaction(transaction, { + contexts: { + trace: { + span_id: expect.any(String), + trace_id: expect.any(String), + data: { + url: '/test/express', + 'http.response.status_code': 200, + }, + op: 'http.server', + status: 'ok', + tags: { + 'http.status_code': '200', + }, + }, + }, + spans: [ + expect.objectContaining({ + description: 'corsMiddleware', + op: 'middleware.express.use', + }), + ], + }); }, - ], - }); + }) + .start(done) + .makeRequest('get', '/test/express'); }); -test('should set a correct transaction name for routes specified in RegEx', async () => { - const env = await TestEnv.init(__dirname, `${__dirname}/server.ts`); - const envelope = await env.getEnvelopeRequest({ url: `${env.url}/regex`, envelopeType: 'transaction' }); - - expect(envelope).toHaveLength(3); - - assertSentryTransaction(envelope[2], { - transaction: 'GET /\\/test\\/regex/', - transaction_info: { - source: 'route', - }, - contexts: { - trace: { - data: { - url: '/test/regex', - 'http.response.status_code': 200, - }, - op: 'http.server', - status: 'ok', - tags: { - 'http.status_code': '200', - }, +test('should set a correct transaction name for routes specified in RegEx', done => { + createRunner(__dirname, 'server.ts') + .expect({ + transaction: transaction => { + assertSentryTransaction(transaction, { + transaction: 'GET /\\/test\\/regex/', + transaction_info: { + source: 'route', + }, + contexts: { + trace: { + trace_id: expect.any(String), + span_id: expect.any(String), + data: { + url: '/test/regex', + 'http.response.status_code': 200, + }, + op: 'http.server', + status: 'ok', + tags: { + 'http.status_code': '200', + }, + }, + }, + }); }, - }, - }); + }) + .start(done) + .makeRequest('get', '/test/regex'); }); test.each([['array1'], ['array5']])( 'should set a correct transaction name for routes consisting of arrays of routes', - async segment => { - const env = await TestEnv.init(__dirname, `${__dirname}/server.ts`); - const envelope = await env.getEnvelopeRequest({ url: `${env.url}/${segment}`, envelopeType: 'transaction' }); - - expect(envelope).toHaveLength(3); - - assertSentryTransaction(envelope[2], { - transaction: 'GET /test/array1,/\\/test\\/array[2-9]', - transaction_info: { - source: 'route', - }, - contexts: { - trace: { - data: { - url: `/test/${segment}`, - 'http.response.status_code': 200, - }, - op: 'http.server', - status: 'ok', - tags: { - 'http.status_code': '200', - }, + ((segment: string, done: () => void) => { + createRunner(__dirname, 'server.ts') + .expect({ + transaction: transaction => { + assertSentryTransaction(transaction, { + transaction: 'GET /test/array1,/\\/test\\/array[2-9]', + transaction_info: { + source: 'route', + }, + contexts: { + trace: { + trace_id: expect.any(String), + span_id: expect.any(String), + data: { + url: `/test/${segment}`, + 'http.response.status_code': 200, + }, + op: 'http.server', + status: 'ok', + tags: { + 'http.status_code': '200', + }, + }, + }, + }); }, - }, - }); - }, + }) + .start(done) + .makeRequest('get', `/test/${segment}`); + }) as any, ); test.each([ @@ -95,29 +107,33 @@ test.each([ ['arr55/required/lastParam'], ['arr/requiredPath/optionalPath/'], ['arr/requiredPath/optionalPath/lastParam'], -])('should handle more complex regexes in route arrays correctly', async segment => { - const env = await TestEnv.init(__dirname, `${__dirname}/server.ts`); - const envelope = await env.getEnvelopeRequest({ url: `${env.url}/${segment}`, envelopeType: 'transaction' }); - - expect(envelope).toHaveLength(3); - - assertSentryTransaction(envelope[2], { - transaction: 'GET /test/arr/:id,/\\/test\\/arr[0-9]*\\/required(path)?(\\/optionalPath)?\\/(lastParam)?', - transaction_info: { - source: 'route', - }, - contexts: { - trace: { - data: { - url: `/test/${segment}`, - 'http.response.status_code': 200, - }, - op: 'http.server', - status: 'ok', - tags: { - 'http.status_code': '200', - }, +])('should handle more complex regexes in route arrays correctly', ((segment: string, done: () => void) => { + createRunner(__dirname, 'server.ts') + .expect({ + transaction: transaction => { + assertSentryTransaction(transaction, { + transaction: 'GET /test/arr/:id,/\\/test\\/arr[0-9]*\\/required(path)?(\\/optionalPath)?\\/(lastParam)?', + transaction_info: { + source: 'route', + }, + contexts: { + trace: { + trace_id: expect.any(String), + span_id: expect.any(String), + data: { + url: `/test/${segment}`, + 'http.response.status_code': 200, + }, + op: 'http.server', + status: 'ok', + tags: { + 'http.status_code': '200', + }, + }, + }, + }); }, - }, - }); -}); + }) + .start(done) + .makeRequest('get', `/test/${segment}`); +}) as any); diff --git a/dev-packages/node-integration-tests/utils/runner.ts b/dev-packages/node-integration-tests/utils/runner.ts index dbfb9cbf10bf..2b4105c5dedf 100644 --- a/dev-packages/node-integration-tests/utils/runner.ts +++ b/dev-packages/node-integration-tests/utils/runner.ts @@ -1,6 +1,7 @@ import { spawn } from 'child_process'; import { join } from 'path'; import type { Envelope, EnvelopeItemType, Event, SerializedSession } from '@sentry/types'; +import axios from 'axios'; export function assertSentryEvent(actual: Event, expected: Event): void { expect(actual).toMatchObject({ @@ -16,6 +17,17 @@ export function assertSentrySession(actual: SerializedSession, expected: Partial }); } +export function assertSentryTransaction(actual: Event, expected: Partial): void { + expect(actual).toMatchObject({ + event_id: expect.any(String), + timestamp: expect.anything(), + start_timestamp: expect.anything(), + spans: expect.any(Array), + type: 'transaction', + ...expected, + }); +} + type Expected = | { event: (event: Event) => void; @@ -57,6 +69,7 @@ export function createRunner(...paths: string[]) { start: function (done?: (e?: unknown) => void) { const expectedEnvelopeCount = expectedEnvelopes.length; let envelopeCount = 0; + let serverPort: number | undefined; const child = spawn('node', [...flags, testPath]); @@ -69,6 +82,17 @@ export function createRunner(...paths: string[]) { done?.(e); }); + async function waitForServerPort(timeout = 10_000): Promise { + let remaining = timeout; + while (serverPort === undefined) { + await new Promise(resolve => setTimeout(resolve, 100)); + remaining -= 100; + if (remaining < 0) { + throw new Error('Timed out waiting for server port'); + } + } + } + function checkDone(): void { envelopeCount++; if (envelopeCount === expectedEnvelopeCount) { @@ -81,6 +105,12 @@ export function createRunner(...paths: string[]) { // Lines can have leading '[something] [{' which we need to remove const cleanedLine = line.replace(/^.*?] \[{"/, '[{"'); + if (cleanedLine.startsWith('{"port":')) { + const { port } = JSON.parse(cleanedLine) as { port: number }; + serverPort = port; + return; + } + if (!cleanedLine.startsWith('[{')) { return; } @@ -156,6 +186,25 @@ export function createRunner(...paths: string[]) { childHasExited: function (): boolean { return hasExited; }, + makeRequest: async function ( + method: 'get' | 'post', + path: string, + headers: Record = {}, + ): Promise { + try { + await waitForServerPort(); + + const url = `http://localhost:${serverPort}${path}`; + if (method === 'get') { + return (await axios.get(url, { headers })).data; + } else { + return (await axios.post(url, { headers })).data; + } + } catch (e) { + done?.(e); + return undefined; + } + }, }; }, }; diff --git a/packages/node/src/integrations/anr/worker.ts b/packages/node/src/integrations/anr/worker.ts index 0a3f64b0c7ae..a546fc5068ff 100644 --- a/packages/node/src/integrations/anr/worker.ts +++ b/packages/node/src/integrations/anr/worker.ts @@ -47,6 +47,7 @@ async function sendAbnormalSession(): Promise { updateSession(session, { status: 'abnormal', abnormal_mechanism: 'anr_foreground' }); const envelope = createSessionEnvelope(session, options.dsn, options.sdkMetadata); + // Log the envelope so to aid in testing log(JSON.stringify(envelope)); await transport.send(envelope); @@ -118,6 +119,7 @@ async function sendAnrEvent(frames?: StackFrame[], traceContext?: TraceContext): }; const envelope = createEventEnvelope(event, options.dsn, options.sdkMetadata); + // Log the envelope so to aid in testing log(JSON.stringify(envelope)); await transport.send(envelope); From aec3278ddd751ee16f04c18b81f4312ac91bda01 Mon Sep 17 00:00:00 2001 From: Tim Fish Date: Tue, 9 Jan 2024 17:09:29 +0100 Subject: [PATCH 11/17] remove the timeout overrides and add some more comments --- .../node-integration-tests/suites/anr/test.ts | 188 +++++++----------- .../suites/public-api/LocalVariables/test.ts | 81 ++++---- .../node-integration-tests/utils/runner.ts | 21 +- 3 files changed, 120 insertions(+), 170 deletions(-) diff --git a/dev-packages/node-integration-tests/suites/anr/test.ts b/dev-packages/node-integration-tests/suites/anr/test.ts index b4d4ac414cd0..f6ef170d6470 100644 --- a/dev-packages/node-integration-tests/suites/anr/test.ts +++ b/dev-packages/node-integration-tests/suites/anr/test.ts @@ -1,8 +1,6 @@ import { conditionalTest } from '../../utils'; import { assertSentryEvent, assertSentrySession, createRunner } from '../../utils/runner'; -const ANR_TEST_TIMEOUT = 10_000; - const EXPECTED_ANR_EVENT = { // Ensure we have context contexts: { @@ -54,123 +52,91 @@ const EXPECTED_ANR_EVENT = { }; conditionalTest({ min: 16 })('should report ANR when event loop blocked', () => { - test( - 'CJS', - done => { - createRunner(__dirname, 'basic.js') - .expect({ - event: event => { - assertSentryEvent(event, EXPECTED_ANR_EVENT); - }, - }) - .start(done); - }, - ANR_TEST_TIMEOUT, - ); + test('CJS', done => { + createRunner(__dirname, 'basic.js') + .expect({ + event: event => { + assertSentryEvent(event, EXPECTED_ANR_EVENT); + }, + }) + .start(done); + }); // TODO (v8): Remove this old API and this test - test( - 'Legacy API', - done => { - createRunner(__dirname, 'legacy.js') - .expect({ - event: event => { - assertSentryEvent(event, EXPECTED_ANR_EVENT); - }, - }) - .start(done); - }, - ANR_TEST_TIMEOUT, - ); + test('Legacy API', done => { + createRunner(__dirname, 'legacy.js') + .expect({ + event: event => { + assertSentryEvent(event, EXPECTED_ANR_EVENT); + }, + }) + .start(done); + }); - test( - 'ESM', - done => { - createRunner(__dirname, 'basic.mjs') - .expect({ - event: event => { - assertSentryEvent(event, EXPECTED_ANR_EVENT); - }, - }) - .start(done); - }, - ANR_TEST_TIMEOUT, - ); + test('ESM', done => { + createRunner(__dirname, 'basic.mjs') + .expect({ + event: event => { + assertSentryEvent(event, EXPECTED_ANR_EVENT); + }, + }) + .start(done); + }); - test( - 'With --inspect', - done => { - createRunner(__dirname, 'basic.mjs') - .withFlags('--inspect') - .expect({ - event: event => { - assertSentryEvent(event, EXPECTED_ANR_EVENT); - }, - }) - .start(done); - }, - ANR_TEST_TIMEOUT, - ); + test('With --inspect', done => { + createRunner(__dirname, 'basic.mjs') + .withFlags('--inspect') + .expect({ + event: event => { + assertSentryEvent(event, EXPECTED_ANR_EVENT); + }, + }) + .start(done); + }); - test( - 'should exit', - done => { - const runner = createRunner(__dirname, 'should-exit.js').start(); + test('should exit', done => { + const runner = createRunner(__dirname, 'should-exit.js').start(); - setTimeout(() => { - expect(runner.childHasExited()).toBe(true); - done(); - }, 5_000); - }, - ANR_TEST_TIMEOUT, - ); + setTimeout(() => { + expect(runner.childHasExited()).toBe(true); + done(); + }, 5_000); + }); - test( - 'should exit forced', - done => { - const runner = createRunner(__dirname, 'should-exit-forced.js').start(); + test('should exit forced', done => { + const runner = createRunner(__dirname, 'should-exit-forced.js').start(); - setTimeout(() => { - expect(runner.childHasExited()).toBe(true); - done(); - }, 5_000); - }, - ANR_TEST_TIMEOUT, - ); + setTimeout(() => { + expect(runner.childHasExited()).toBe(true); + done(); + }, 5_000); + }); - test( - 'With session', - done => { - createRunner(__dirname, 'basic-session.js') - .expect({ - session: session => { - assertSentrySession(session, { - status: 'abnormal', - abnormal_mechanism: 'anr_foreground', - }); - }, - }) - .expect({ - event: event => { - assertSentryEvent(event, EXPECTED_ANR_EVENT); - }, - }) - .start(done); - }, - ANR_TEST_TIMEOUT, - ); + test('With session', done => { + createRunner(__dirname, 'basic-session.js') + .expect({ + session: session => { + assertSentrySession(session, { + status: 'abnormal', + abnormal_mechanism: 'anr_foreground', + }); + }, + }) + .expect({ + event: event => { + assertSentryEvent(event, EXPECTED_ANR_EVENT); + }, + }) + .start(done); + }); - test( - 'from forked process', - done => { - createRunner(__dirname, 'forker.js') - .expect({ - event: event => { - assertSentryEvent(event, EXPECTED_ANR_EVENT); - }, - }) - .start(done); - }, - ANR_TEST_TIMEOUT, - ); + test('from forked process', done => { + createRunner(__dirname, 'forker.js') + .expect({ + event: event => { + assertSentryEvent(event, EXPECTED_ANR_EVENT); + }, + }) + .start(done); + }); }); 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 407fd3e1fa09..8b7e5fe280e4 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,7 +3,6 @@ import * as path from 'path'; import { conditionalTest } from '../../../utils'; import { assertSentryEvent, createRunner } from '../../../utils/runner'; -const LOCAL_VARIABLES_TEST_TIMEOUT = 20_000; const EXPECTED_LOCAL_VARIABLES_EVENT = { exception: { values: [ @@ -31,57 +30,45 @@ const EXPECTED_LOCAL_VARIABLES_EVENT = { }; conditionalTest({ min: 18 })('LocalVariables integration', () => { - test( - 'Should not include local variables by default', - done => { - createRunner(__dirname, 'no-local-variables.js') - .ignore('session') - .expect({ - event: event => { - const frames = event.exception?.values?.[0].stacktrace?.frames || []; - const lastFrame = frames[frames.length - 1]; + test('Should not include local variables by default', done => { + createRunner(__dirname, 'no-local-variables.js') + .ignore('session') + .expect({ + event: event => { + const frames = event.exception?.values?.[0].stacktrace?.frames || []; + const lastFrame = frames[frames.length - 1]; - expect(lastFrame.vars).toBeUndefined(); + expect(lastFrame.vars).toBeUndefined(); - const penultimateFrame = frames[frames.length - 2]; + const penultimateFrame = frames[frames.length - 2]; - expect(penultimateFrame.vars).toBeUndefined(); - }, - }) - .start(done); - }, - LOCAL_VARIABLES_TEST_TIMEOUT, - ); + expect(penultimateFrame.vars).toBeUndefined(); + }, + }) + .start(done); + }); - test( - 'Should include local variables when enabled', - done => { - createRunner(__dirname, 'local-variables.js') - .ignore('session') - .expect({ - event: event => { - assertSentryEvent(event, EXPECTED_LOCAL_VARIABLES_EVENT); - }, - }) - .start(done); - }, - LOCAL_VARIABLES_TEST_TIMEOUT, - ); + test('Should include local variables when enabled', done => { + createRunner(__dirname, 'local-variables.js') + .ignore('session') + .expect({ + event: event => { + assertSentryEvent(event, EXPECTED_LOCAL_VARIABLES_EVENT); + }, + }) + .start(done); + }); - test( - 'Should include local variables with ESM', - done => { - createRunner(__dirname, 'local-variables-caught.mjs') - .ignore('session') - .expect({ - event: event => { - assertSentryEvent(event, EXPECTED_LOCAL_VARIABLES_EVENT); - }, - }) - .start(done); - }, - LOCAL_VARIABLES_TEST_TIMEOUT, - ); + test('Should include local variables with ESM', done => { + createRunner(__dirname, 'local-variables-caught.mjs') + .ignore('session') + .expect({ + event: event => { + assertSentryEvent(event, EXPECTED_LOCAL_VARIABLES_EVENT); + }, + }) + .start(done); + }); test('Includes local variables for caught exceptions when enabled', done => { createRunner(__dirname, 'local-variables-caught.js') diff --git a/dev-packages/node-integration-tests/utils/runner.ts b/dev-packages/node-integration-tests/utils/runner.ts index 2b4105c5dedf..bf7c748f3a58 100644 --- a/dev-packages/node-integration-tests/utils/runner.ts +++ b/dev-packages/node-integration-tests/utils/runner.ts @@ -93,7 +93,8 @@ export function createRunner(...paths: string[]) { } } - function checkDone(): void { + /** Called after each expect callback to check if we're complete */ + function expectCallbackCalled(): void { envelopeCount++; if (envelopeCount === expectedEnvelopeCount) { child.kill(); @@ -105,12 +106,14 @@ export function createRunner(...paths: string[]) { // Lines can have leading '[something] [{' which we need to remove const cleanedLine = line.replace(/^.*?] \[{"/, '[{"'); + // See if we have a port message if (cleanedLine.startsWith('{"port":')) { const { port } = JSON.parse(cleanedLine) as { port: number }; serverPort = port; return; } + // Skip any lines that don't start with envelope JSON if (!cleanedLine.startsWith('[{')) { return; } @@ -119,10 +122,6 @@ export function createRunner(...paths: string[]) { try { envelope = JSON.parse(cleanedLine) as Envelope; } catch (_) { - // - } - - if (!envelope) { return; } @@ -135,7 +134,7 @@ export function createRunner(...paths: string[]) { const expected = expectedEnvelopes.shift(); - // Catch any error or failed assertions and pass them to done + // Catch any error or failed assertions and pass them to done to end the test quickly try { if (!expected) { throw new Error(`No more expected envelope items but we received a '${envelopeItemType}' item`); @@ -149,17 +148,17 @@ export function createRunner(...paths: string[]) { if ('event' in expected) { expected.event(item[1] as Event); - checkDone(); + expectCallbackCalled(); } if ('transaction' in expected) { expected.transaction(item[1] as Event); - checkDone(); + expectCallbackCalled(); } if ('session' in expected) { expected.session(item[1] as SerializedSession); - checkDone(); + expectCallbackCalled(); } } catch (e) { done?.(e); @@ -168,16 +167,14 @@ export function createRunner(...paths: string[]) { } let buffer = Buffer.alloc(0); - child.stdout.on('data', (data: Buffer) => { + // This is horribly memory inefficient but it's only for tests buffer = Buffer.concat([buffer, data]); let splitIndex = -1; while ((splitIndex = buffer.indexOf(0xa)) >= 0) { const line = buffer.subarray(0, splitIndex).toString(); - buffer = Buffer.from(buffer.subarray(splitIndex + 1)); - tryParseLine(line); } }); From 171abfb1c442d60980b3c341f6d3859dd6ead52b Mon Sep 17 00:00:00 2001 From: Tim Fish Date: Tue, 9 Jan 2024 20:48:41 +0100 Subject: [PATCH 12/17] swap test --- dev-packages/node-integration-tests/suites/anr/test.ts | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/dev-packages/node-integration-tests/suites/anr/test.ts b/dev-packages/node-integration-tests/suites/anr/test.ts index f6ef170d6470..33676c0d9766 100644 --- a/dev-packages/node-integration-tests/suites/anr/test.ts +++ b/dev-packages/node-integration-tests/suites/anr/test.ts @@ -52,8 +52,9 @@ const EXPECTED_ANR_EVENT = { }; conditionalTest({ min: 16 })('should report ANR when event loop blocked', () => { - test('CJS', done => { - createRunner(__dirname, 'basic.js') + // TODO (v8): Remove this old API and this test + test('Legacy API', done => { + createRunner(__dirname, 'legacy.js') .expect({ event: event => { assertSentryEvent(event, EXPECTED_ANR_EVENT); @@ -62,9 +63,8 @@ conditionalTest({ min: 16 })('should report ANR when event loop blocked', () => .start(done); }); - // TODO (v8): Remove this old API and this test - test('Legacy API', done => { - createRunner(__dirname, 'legacy.js') + test('CJS', done => { + createRunner(__dirname, 'basic.js') .expect({ event: event => { assertSentryEvent(event, EXPECTED_ANR_EVENT); From cfb7e0dc734bd919ac708d6e73d15706e3fe2f08 Mon Sep 17 00:00:00 2001 From: Tim Fish Date: Tue, 9 Jan 2024 21:42:52 +0100 Subject: [PATCH 13/17] Push dummy test --- dev-packages/node-integration-tests/suites/anr/test.ts | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/dev-packages/node-integration-tests/suites/anr/test.ts b/dev-packages/node-integration-tests/suites/anr/test.ts index 33676c0d9766..1e4bccff00a5 100644 --- a/dev-packages/node-integration-tests/suites/anr/test.ts +++ b/dev-packages/node-integration-tests/suites/anr/test.ts @@ -52,6 +52,11 @@ const EXPECTED_ANR_EVENT = { }; conditionalTest({ min: 16 })('should report ANR when event loop blocked', () => { + test('dummy test', done => { + // The first test in here always fails so maybe this fixed it? + done(); + }); + // TODO (v8): Remove this old API and this test test('Legacy API', done => { createRunner(__dirname, 'legacy.js') From ff1f88a4f654a30f2c65be18d1b40bad0c2b4a17 Mon Sep 17 00:00:00 2001 From: Tim Fish Date: Tue, 9 Jan 2024 22:06:39 +0100 Subject: [PATCH 14/17] dummy test --- dev-packages/node-integration-tests/suites/anr/test.ts | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/dev-packages/node-integration-tests/suites/anr/test.ts b/dev-packages/node-integration-tests/suites/anr/test.ts index 1e4bccff00a5..fb72e3ef364e 100644 --- a/dev-packages/node-integration-tests/suites/anr/test.ts +++ b/dev-packages/node-integration-tests/suites/anr/test.ts @@ -53,7 +53,15 @@ const EXPECTED_ANR_EVENT = { conditionalTest({ min: 16 })('should report ANR when event loop blocked', () => { test('dummy test', done => { - // The first test in here always fails so maybe this fixed it? + // The first test in here always fails so maybe this fixes it? + createRunner(__dirname, 'basic.js') + .expect({ + event: _event => { + // + }, + }) + .start(); + done(); }); From 97391aefc29c83a54f0fa8f84b1dc9373fbbc4ae Mon Sep 17 00:00:00 2001 From: Tim Fish Date: Wed, 10 Jan 2024 11:22:31 +0100 Subject: [PATCH 15/17] Reduce Anr hangs --- .../suites/anr/basic-session.js | 4 ++-- .../node-integration-tests/suites/anr/basic.js | 4 ++-- .../node-integration-tests/suites/anr/basic.mjs | 4 ++-- .../node-integration-tests/suites/anr/forked.js | 4 ++-- .../node-integration-tests/suites/anr/legacy.js | 4 ++-- .../node-integration-tests/suites/anr/test.ts | 15 +-------------- 6 files changed, 11 insertions(+), 24 deletions(-) diff --git a/dev-packages/node-integration-tests/suites/anr/basic-session.js b/dev-packages/node-integration-tests/suites/anr/basic-session.js index 03c8c94fdadf..fe4190c8cc46 100644 --- a/dev-packages/node-integration-tests/suites/anr/basic-session.js +++ b/dev-packages/node-integration-tests/suites/anr/basic-session.js @@ -11,11 +11,11 @@ Sentry.init({ dsn: 'https://public@dsn.ingest.sentry.io/1337', release: '1.0', debug: true, - integrations: [new Sentry.Integrations.Anr({ captureStackTrace: true, anrThreshold: 200 })], + integrations: [new Sentry.Integrations.Anr({ captureStackTrace: true, anrThreshold: 100 })], }); function longWork() { - for (let i = 0; i < 100; i++) { + for (let i = 0; i < 20; i++) { const salt = crypto.randomBytes(128).toString('base64'); // eslint-disable-next-line no-unused-vars const hash = crypto.pbkdf2Sync('myPassword', salt, 10000, 512, 'sha512'); diff --git a/dev-packages/node-integration-tests/suites/anr/basic.js b/dev-packages/node-integration-tests/suites/anr/basic.js index 5e0323e2c6c5..097dec6c925c 100644 --- a/dev-packages/node-integration-tests/suites/anr/basic.js +++ b/dev-packages/node-integration-tests/suites/anr/basic.js @@ -12,11 +12,11 @@ Sentry.init({ release: '1.0', debug: true, autoSessionTracking: false, - integrations: [new Sentry.Integrations.Anr({ captureStackTrace: true, anrThreshold: 200 })], + integrations: [new Sentry.Integrations.Anr({ captureStackTrace: true, anrThreshold: 100 })], }); function longWork() { - for (let i = 0; i < 100; i++) { + for (let i = 0; i < 20; i++) { const salt = crypto.randomBytes(128).toString('base64'); // eslint-disable-next-line no-unused-vars const hash = crypto.pbkdf2Sync('myPassword', salt, 10000, 512, 'sha512'); diff --git a/dev-packages/node-integration-tests/suites/anr/basic.mjs b/dev-packages/node-integration-tests/suites/anr/basic.mjs index 17c8a2d460df..43a8d02a41ac 100644 --- a/dev-packages/node-integration-tests/suites/anr/basic.mjs +++ b/dev-packages/node-integration-tests/suites/anr/basic.mjs @@ -12,11 +12,11 @@ Sentry.init({ release: '1.0', debug: true, autoSessionTracking: false, - integrations: [new Sentry.Integrations.Anr({ captureStackTrace: true, anrThreshold: 200 })], + integrations: [new Sentry.Integrations.Anr({ captureStackTrace: true, anrThreshold: 100 })], }); function longWork() { - for (let i = 0; i < 100; i++) { + for (let i = 0; i < 20; i++) { const salt = crypto.randomBytes(128).toString('base64'); // eslint-disable-next-line no-unused-vars const hash = crypto.pbkdf2Sync('myPassword', salt, 10000, 512, 'sha512'); diff --git a/dev-packages/node-integration-tests/suites/anr/forked.js b/dev-packages/node-integration-tests/suites/anr/forked.js index 5e0323e2c6c5..097dec6c925c 100644 --- a/dev-packages/node-integration-tests/suites/anr/forked.js +++ b/dev-packages/node-integration-tests/suites/anr/forked.js @@ -12,11 +12,11 @@ Sentry.init({ release: '1.0', debug: true, autoSessionTracking: false, - integrations: [new Sentry.Integrations.Anr({ captureStackTrace: true, anrThreshold: 200 })], + integrations: [new Sentry.Integrations.Anr({ captureStackTrace: true, anrThreshold: 100 })], }); function longWork() { - for (let i = 0; i < 100; i++) { + for (let i = 0; i < 20; i++) { const salt = crypto.randomBytes(128).toString('base64'); // eslint-disable-next-line no-unused-vars const hash = crypto.pbkdf2Sync('myPassword', salt, 10000, 512, 'sha512'); diff --git a/dev-packages/node-integration-tests/suites/anr/legacy.js b/dev-packages/node-integration-tests/suites/anr/legacy.js index 46b6e1437b10..f91db4bec054 100644 --- a/dev-packages/node-integration-tests/suites/anr/legacy.js +++ b/dev-packages/node-integration-tests/suites/anr/legacy.js @@ -15,9 +15,9 @@ Sentry.init({ }); // eslint-disable-next-line deprecation/deprecation -Sentry.enableAnrDetection({ captureStackTrace: true, anrThreshold: 200 }).then(() => { +Sentry.enableAnrDetection({ captureStackTrace: true, anrThreshold: 100 }).then(() => { function longWork() { - for (let i = 0; i < 100; i++) { + for (let i = 0; i < 20; i++) { const salt = crypto.randomBytes(128).toString('base64'); // eslint-disable-next-line no-unused-vars const hash = crypto.pbkdf2Sync('myPassword', salt, 10000, 512, 'sha512'); diff --git a/dev-packages/node-integration-tests/suites/anr/test.ts b/dev-packages/node-integration-tests/suites/anr/test.ts index fb72e3ef364e..33ec18eed8da 100644 --- a/dev-packages/node-integration-tests/suites/anr/test.ts +++ b/dev-packages/node-integration-tests/suites/anr/test.ts @@ -26,7 +26,7 @@ const EXPECTED_ANR_EVENT = { values: [ { type: 'ApplicationNotResponding', - value: 'Application Not Responding for at least 200 ms', + value: 'Application Not Responding for at least 100 ms', mechanism: { type: 'ANR' }, stacktrace: { frames: expect.arrayContaining([ @@ -52,19 +52,6 @@ const EXPECTED_ANR_EVENT = { }; conditionalTest({ min: 16 })('should report ANR when event loop blocked', () => { - test('dummy test', done => { - // The first test in here always fails so maybe this fixes it? - createRunner(__dirname, 'basic.js') - .expect({ - event: _event => { - // - }, - }) - .start(); - - done(); - }); - // TODO (v8): Remove this old API and this test test('Legacy API', done => { createRunner(__dirname, 'legacy.js') From 9ac27d66485cc4a48fa9b10173e2682182c1ff9a Mon Sep 17 00:00:00 2001 From: Tim Fish Date: Wed, 10 Jan 2024 12:47:19 +0100 Subject: [PATCH 16/17] Let the runner optionally do the assertions --- .../node-integration-tests/package.json | 1 + .../node-integration-tests/suites/anr/test.ts | 57 ++----- .../suites/express/tracing/test.ts | 154 +++++++++--------- .../suites/public-api/LocalVariables/test.ts | 31 +--- .../node-integration-tests/utils/runner.ts | 30 +++- 5 files changed, 115 insertions(+), 158 deletions(-) diff --git a/dev-packages/node-integration-tests/package.json b/dev-packages/node-integration-tests/package.json index 2f28ebadf913..d7e9e10a4208 100644 --- a/dev-packages/node-integration-tests/package.json +++ b/dev-packages/node-integration-tests/package.json @@ -29,6 +29,7 @@ "@prisma/client": "3.15.2", "@sentry/node": "7.92.0", "@sentry/tracing": "7.92.0", + "@sentry/types": "7.92.0", "@types/mongodb": "^3.6.20", "@types/mysql": "^2.15.21", "@types/pg": "^8.6.5", diff --git a/dev-packages/node-integration-tests/suites/anr/test.ts b/dev-packages/node-integration-tests/suites/anr/test.ts index 33ec18eed8da..5edbe6dd2f78 100644 --- a/dev-packages/node-integration-tests/suites/anr/test.ts +++ b/dev-packages/node-integration-tests/suites/anr/test.ts @@ -1,5 +1,5 @@ import { conditionalTest } from '../../utils'; -import { assertSentryEvent, assertSentrySession, createRunner } from '../../utils/runner'; +import { createRunner } from '../../utils/runner'; const EXPECTED_ANR_EVENT = { // Ensure we have context @@ -54,44 +54,19 @@ const EXPECTED_ANR_EVENT = { conditionalTest({ min: 16 })('should report ANR when event loop blocked', () => { // TODO (v8): Remove this old API and this test test('Legacy API', done => { - createRunner(__dirname, 'legacy.js') - .expect({ - event: event => { - assertSentryEvent(event, EXPECTED_ANR_EVENT); - }, - }) - .start(done); + createRunner(__dirname, 'legacy.js').expect({ event: EXPECTED_ANR_EVENT }).start(done); }); test('CJS', done => { - createRunner(__dirname, 'basic.js') - .expect({ - event: event => { - assertSentryEvent(event, EXPECTED_ANR_EVENT); - }, - }) - .start(done); + createRunner(__dirname, 'basic.js').expect({ event: EXPECTED_ANR_EVENT }).start(done); }); test('ESM', done => { - createRunner(__dirname, 'basic.mjs') - .expect({ - event: event => { - assertSentryEvent(event, EXPECTED_ANR_EVENT); - }, - }) - .start(done); + createRunner(__dirname, 'basic.mjs').expect({ event: EXPECTED_ANR_EVENT }).start(done); }); test('With --inspect', done => { - createRunner(__dirname, 'basic.mjs') - .withFlags('--inspect') - .expect({ - event: event => { - assertSentryEvent(event, EXPECTED_ANR_EVENT); - }, - }) - .start(done); + createRunner(__dirname, 'basic.mjs').withFlags('--inspect').expect({ event: EXPECTED_ANR_EVENT }).start(done); }); test('should exit', done => { @@ -115,28 +90,16 @@ conditionalTest({ min: 16 })('should report ANR when event loop blocked', () => test('With session', done => { createRunner(__dirname, 'basic-session.js') .expect({ - session: session => { - assertSentrySession(session, { - status: 'abnormal', - abnormal_mechanism: 'anr_foreground', - }); - }, - }) - .expect({ - event: event => { - assertSentryEvent(event, EXPECTED_ANR_EVENT); + session: { + status: 'abnormal', + abnormal_mechanism: 'anr_foreground', }, }) + .expect({ event: EXPECTED_ANR_EVENT }) .start(done); }); test('from forked process', done => { - createRunner(__dirname, 'forker.js') - .expect({ - event: event => { - assertSentryEvent(event, EXPECTED_ANR_EVENT); - }, - }) - .start(done); + createRunner(__dirname, 'forker.js').expect({ event: EXPECTED_ANR_EVENT }).start(done); }); }); diff --git a/dev-packages/node-integration-tests/suites/express/tracing/test.ts b/dev-packages/node-integration-tests/suites/express/tracing/test.ts index 825bd2daf168..089a2ac16edf 100644 --- a/dev-packages/node-integration-tests/suites/express/tracing/test.ts +++ b/dev-packages/node-integration-tests/suites/express/tracing/test.ts @@ -1,32 +1,30 @@ -import { assertSentryTransaction, createRunner } from '../../../utils/runner'; +import { createRunner } from '../../../utils/runner'; test('should create and send transactions for Express routes and spans for middlewares.', done => { createRunner(__dirname, 'server.ts') .expect({ - transaction: transaction => { - assertSentryTransaction(transaction, { - contexts: { - trace: { - span_id: expect.any(String), - trace_id: expect.any(String), - data: { - url: '/test/express', - 'http.response.status_code': 200, - }, - op: 'http.server', - status: 'ok', - tags: { - 'http.status_code': '200', - }, + transaction: { + contexts: { + trace: { + span_id: expect.any(String), + trace_id: expect.any(String), + data: { + url: '/test/express', + 'http.response.status_code': 200, + }, + op: 'http.server', + status: 'ok', + tags: { + 'http.status_code': '200', }, }, - spans: [ - expect.objectContaining({ - description: 'corsMiddleware', - op: 'middleware.express.use', - }), - ], - }); + }, + spans: [ + expect.objectContaining({ + description: 'corsMiddleware', + op: 'middleware.express.use', + }), + ], }, }) .start(done) @@ -36,9 +34,39 @@ test('should create and send transactions for Express routes and spans for middl test('should set a correct transaction name for routes specified in RegEx', done => { createRunner(__dirname, 'server.ts') .expect({ - transaction: transaction => { - assertSentryTransaction(transaction, { - transaction: 'GET /\\/test\\/regex/', + transaction: { + transaction: 'GET /\\/test\\/regex/', + transaction_info: { + source: 'route', + }, + contexts: { + trace: { + trace_id: expect.any(String), + span_id: expect.any(String), + data: { + url: '/test/regex', + 'http.response.status_code': 200, + }, + op: 'http.server', + status: 'ok', + tags: { + 'http.status_code': '200', + }, + }, + }, + }, + }) + .start(done) + .makeRequest('get', '/test/regex'); +}); + +test.each([['array1'], ['array5']])( + 'should set a correct transaction name for routes consisting of arrays of routes', + ((segment: string, done: () => void) => { + createRunner(__dirname, 'server.ts') + .expect({ + transaction: { + transaction: 'GET /test/array1,/\\/test\\/array[2-9]', transaction_info: { source: 'route', }, @@ -47,7 +75,7 @@ test('should set a correct transaction name for routes specified in RegEx', done trace_id: expect.any(String), span_id: expect.any(String), data: { - url: '/test/regex', + url: `/test/${segment}`, 'http.response.status_code': 200, }, op: 'http.server', @@ -57,40 +85,6 @@ test('should set a correct transaction name for routes specified in RegEx', done }, }, }, - }); - }, - }) - .start(done) - .makeRequest('get', '/test/regex'); -}); - -test.each([['array1'], ['array5']])( - 'should set a correct transaction name for routes consisting of arrays of routes', - ((segment: string, done: () => void) => { - createRunner(__dirname, 'server.ts') - .expect({ - transaction: transaction => { - assertSentryTransaction(transaction, { - transaction: 'GET /test/array1,/\\/test\\/array[2-9]', - transaction_info: { - source: 'route', - }, - contexts: { - trace: { - trace_id: expect.any(String), - span_id: expect.any(String), - data: { - url: `/test/${segment}`, - 'http.response.status_code': 200, - }, - op: 'http.server', - status: 'ok', - tags: { - 'http.status_code': '200', - }, - }, - }, - }); }, }) .start(done) @@ -110,28 +104,26 @@ test.each([ ])('should handle more complex regexes in route arrays correctly', ((segment: string, done: () => void) => { createRunner(__dirname, 'server.ts') .expect({ - transaction: transaction => { - assertSentryTransaction(transaction, { - transaction: 'GET /test/arr/:id,/\\/test\\/arr[0-9]*\\/required(path)?(\\/optionalPath)?\\/(lastParam)?', - transaction_info: { - source: 'route', - }, - contexts: { - trace: { - trace_id: expect.any(String), - span_id: expect.any(String), - data: { - url: `/test/${segment}`, - 'http.response.status_code': 200, - }, - op: 'http.server', - status: 'ok', - tags: { - 'http.status_code': '200', - }, + transaction: { + transaction: 'GET /test/arr/:id,/\\/test\\/arr[0-9]*\\/required(path)?(\\/optionalPath)?\\/(lastParam)?', + transaction_info: { + source: 'route', + }, + contexts: { + trace: { + trace_id: expect.any(String), + span_id: expect.any(String), + data: { + url: `/test/${segment}`, + 'http.response.status_code': 200, + }, + op: 'http.server', + status: 'ok', + tags: { + 'http.status_code': '200', }, }, - }); + }, }, }) .start(done) 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 8b7e5fe280e4..75e95f09860c 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 @@ -1,7 +1,7 @@ import * as childProcess from 'child_process'; import * as path from 'path'; import { conditionalTest } from '../../../utils'; -import { assertSentryEvent, createRunner } from '../../../utils/runner'; +import { createRunner } from '../../../utils/runner'; const EXPECTED_LOCAL_VARIABLES_EVENT = { exception: { @@ -35,14 +35,9 @@ conditionalTest({ min: 18 })('LocalVariables integration', () => { .ignore('session') .expect({ event: event => { - const frames = event.exception?.values?.[0].stacktrace?.frames || []; - const lastFrame = frames[frames.length - 1]; - - expect(lastFrame.vars).toBeUndefined(); - - const penultimateFrame = frames[frames.length - 2]; - - expect(penultimateFrame.vars).toBeUndefined(); + for (const frame of event.exception?.values?.[0].stacktrace?.frames || []) { + expect(frame.vars).toBeUndefined(); + } }, }) .start(done); @@ -51,33 +46,21 @@ conditionalTest({ min: 18 })('LocalVariables integration', () => { test('Should include local variables when enabled', done => { createRunner(__dirname, 'local-variables.js') .ignore('session') - .expect({ - event: event => { - assertSentryEvent(event, EXPECTED_LOCAL_VARIABLES_EVENT); - }, - }) + .expect({ event: EXPECTED_LOCAL_VARIABLES_EVENT }) .start(done); }); test('Should include local variables with ESM', done => { createRunner(__dirname, 'local-variables-caught.mjs') .ignore('session') - .expect({ - event: event => { - assertSentryEvent(event, EXPECTED_LOCAL_VARIABLES_EVENT); - }, - }) + .expect({ event: EXPECTED_LOCAL_VARIABLES_EVENT }) .start(done); }); test('Includes local variables for caught exceptions when enabled', done => { createRunner(__dirname, 'local-variables-caught.js') .ignore('session') - .expect({ - event: event => { - assertSentryEvent(event, EXPECTED_LOCAL_VARIABLES_EVENT); - }, - }) + .expect({ event: EXPECTED_LOCAL_VARIABLES_EVENT }) .start(done); }); diff --git a/dev-packages/node-integration-tests/utils/runner.ts b/dev-packages/node-integration-tests/utils/runner.ts index bf7c748f3a58..2b89c9deb46f 100644 --- a/dev-packages/node-integration-tests/utils/runner.ts +++ b/dev-packages/node-integration-tests/utils/runner.ts @@ -30,13 +30,13 @@ export function assertSentryTransaction(actual: Event, expected: Partial) type Expected = | { - event: (event: Event) => void; + event: Partial | ((event: Event) => void); } | { - transaction: (event: Event) => void; + transaction: Partial | ((event: Event) => void); } | { - session: (event: SerializedSession) => void; + session: Partial | ((event: SerializedSession) => void); }; /** */ @@ -147,17 +147,35 @@ export function createRunner(...paths: string[]) { } if ('event' in expected) { - expected.event(item[1] as Event); + const event = item[1] as Event; + if (typeof expected.event === 'function') { + expected.event(event); + } else { + assertSentryEvent(event, expected.event); + } + expectCallbackCalled(); } if ('transaction' in expected) { - expected.transaction(item[1] as Event); + const event = item[1] as Event; + if (typeof expected.transaction === 'function') { + expected.transaction(event); + } else { + assertSentryTransaction(event, expected.transaction); + } + expectCallbackCalled(); } if ('session' in expected) { - expected.session(item[1] as SerializedSession); + const session = item[1] as SerializedSession; + if (typeof expected.session === 'function') { + expected.session(session); + } else { + assertSentrySession(session, expected.session); + } + expectCallbackCalled(); } } catch (e) { From ede8ad8578caa038875ba6318ac7725b11d1a9bc Mon Sep 17 00:00:00 2001 From: Tim Fish Date: Wed, 10 Jan 2024 13:03:31 +0100 Subject: [PATCH 17/17] Fix lint --- packages/core/test/lib/tracing/dynamicSamplingContext.test.ts | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/packages/core/test/lib/tracing/dynamicSamplingContext.test.ts b/packages/core/test/lib/tracing/dynamicSamplingContext.test.ts index 06a061612eda..da8bf1595e21 100644 --- a/packages/core/test/lib/tracing/dynamicSamplingContext.test.ts +++ b/packages/core/test/lib/tracing/dynamicSamplingContext.test.ts @@ -20,6 +20,7 @@ describe('getDynamicSamplingContextFromSpan', () => { }); test('returns the DSC provided during transaction creation', () => { + // eslint-disable-next-line deprecation/deprecation const transaction = new Transaction({ name: 'tx', metadata: { dynamicSamplingContext: { environment: 'myEnv' } }, @@ -67,6 +68,7 @@ describe('getDynamicSamplingContextFromSpan', () => { }); test('returns a new DSC, if no DSC was provided during transaction creation (via new Txn and deprecated metadata)', () => { + // eslint-disable-next-line deprecation/deprecation const transaction = new Transaction({ name: 'tx', metadata: { @@ -90,6 +92,7 @@ describe('getDynamicSamplingContextFromSpan', () => { describe('Including transaction name in DSC', () => { test('is not included if transaction source is url', () => { + // eslint-disable-next-line deprecation/deprecation const transaction = new Transaction({ name: 'tx', metadata: { @@ -106,6 +109,7 @@ describe('getDynamicSamplingContextFromSpan', () => { ['is included if transaction source is parameterized route/url', 'route'], ['is included if transaction source is a custom name', 'custom'], ])('%s', (_: string, source) => { + // eslint-disable-next-line deprecation/deprecation const transaction = new Transaction({ name: 'tx', metadata: {