From f765d07822f1f224ca11e731bcefb3a1a6614cbf Mon Sep 17 00:00:00 2001 From: Tim Fish Date: Fri, 22 Mar 2024 21:58:38 +0000 Subject: [PATCH 1/5] feat(node): Add scope to ANR events improve comments test isolated scope minor simplify test longer other contexts --- .../suites/anr/basic-session.js | 3 ++ .../suites/anr/basic.js | 3 ++ .../suites/anr/basic.mjs | 3 ++ .../suites/anr/forked.js | 3 ++ .../suites/anr/isolated.mjs | 53 +++++++++++++++++++ .../suites/anr/stop-and-start.js | 3 ++ .../node-integration-tests/suites/anr/test.ts | 39 ++++++++++++++ packages/node/src/integrations/anr/index.ts | 48 +++++++++++++---- packages/node/src/integrations/anr/worker.ts | 46 ++++++++++++---- 9 files changed, 181 insertions(+), 20 deletions(-) create mode 100644 dev-packages/node-integration-tests/suites/anr/isolated.mjs 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 fe4190c8cc46..1dfa3eda5391 100644 --- a/dev-packages/node-integration-tests/suites/anr/basic-session.js +++ b/dev-packages/node-integration-tests/suites/anr/basic-session.js @@ -14,6 +14,9 @@ Sentry.init({ integrations: [new Sentry.Integrations.Anr({ captureStackTrace: true, anrThreshold: 100 })], }); +Sentry.setUser({ email: 'person@home.com' }); +Sentry.addBreadcrumb({ message: 'important message!' }); + function longWork() { for (let i = 0; i < 20; i++) { const salt = crypto.randomBytes(128).toString('base64'); diff --git a/dev-packages/node-integration-tests/suites/anr/basic.js b/dev-packages/node-integration-tests/suites/anr/basic.js index 097dec6c925c..a1e39ef53223 100644 --- a/dev-packages/node-integration-tests/suites/anr/basic.js +++ b/dev-packages/node-integration-tests/suites/anr/basic.js @@ -15,6 +15,9 @@ Sentry.init({ integrations: [new Sentry.Integrations.Anr({ captureStackTrace: true, anrThreshold: 100 })], }); +Sentry.setUser({ email: 'person@home.com' }); +Sentry.addBreadcrumb({ message: 'important message!' }); + function longWork() { for (let i = 0; i < 20; i++) { const salt = crypto.randomBytes(128).toString('base64'); diff --git a/dev-packages/node-integration-tests/suites/anr/basic.mjs b/dev-packages/node-integration-tests/suites/anr/basic.mjs index 43a8d02a41ac..7a787a5c7ccb 100644 --- a/dev-packages/node-integration-tests/suites/anr/basic.mjs +++ b/dev-packages/node-integration-tests/suites/anr/basic.mjs @@ -15,6 +15,9 @@ Sentry.init({ integrations: [new Sentry.Integrations.Anr({ captureStackTrace: true, anrThreshold: 100 })], }); +Sentry.setUser({ email: 'person@home.com' }); +Sentry.addBreadcrumb({ message: 'important message!' }); + function longWork() { for (let i = 0; i < 20; i++) { const salt = crypto.randomBytes(128).toString('base64'); diff --git a/dev-packages/node-integration-tests/suites/anr/forked.js b/dev-packages/node-integration-tests/suites/anr/forked.js index 097dec6c925c..a1e39ef53223 100644 --- a/dev-packages/node-integration-tests/suites/anr/forked.js +++ b/dev-packages/node-integration-tests/suites/anr/forked.js @@ -15,6 +15,9 @@ Sentry.init({ integrations: [new Sentry.Integrations.Anr({ captureStackTrace: true, anrThreshold: 100 })], }); +Sentry.setUser({ email: 'person@home.com' }); +Sentry.addBreadcrumb({ message: 'important message!' }); + function longWork() { for (let i = 0; i < 20; i++) { const salt = crypto.randomBytes(128).toString('base64'); diff --git a/dev-packages/node-integration-tests/suites/anr/isolated.mjs b/dev-packages/node-integration-tests/suites/anr/isolated.mjs new file mode 100644 index 000000000000..d9b179c63e71 --- /dev/null +++ b/dev-packages/node-integration-tests/suites/anr/isolated.mjs @@ -0,0 +1,53 @@ +import * as assert from 'assert'; +import * as crypto from 'crypto'; + +import * as Sentry from '@sentry/node'; + +setTimeout(() => { + process.exit(); +}, 10000); + +Sentry.init({ + dsn: 'https://public@dsn.ingest.sentry.io/1337', + release: '1.0', + debug: true, + autoSessionTracking: false, + integrations: [Sentry.anrIntegration({ captureStackTrace: true, anrThreshold: 100 })], +}); + +async function longWork() { + await new Promise(resolve => setTimeout(resolve, 1000)); + + for (let i = 0; i < 20; i++) { + const salt = crypto.randomBytes(128).toString('base64'); + const hash = crypto.pbkdf2Sync('myPassword', salt, 10000, 512, 'sha512'); + assert.ok(hash); + } +} + +function neverResolve() { + return new Promise(() => { + // + }); +} + +const fns = [ + neverResolve, + neverResolve, + neverResolve, + neverResolve, + neverResolve, + longWork, // [5] + neverResolve, + neverResolve, + neverResolve, + neverResolve, +]; + +for (let id = 0; id < 10; id++) { + Sentry.withIsolationScope(async () => { + Sentry.setUser({ id }); + + await fns[id](); + }); +} diff --git a/dev-packages/node-integration-tests/suites/anr/stop-and-start.js b/dev-packages/node-integration-tests/suites/anr/stop-and-start.js index 9de453abf23d..4f9fc9bc64db 100644 --- a/dev-packages/node-integration-tests/suites/anr/stop-and-start.js +++ b/dev-packages/node-integration-tests/suites/anr/stop-and-start.js @@ -17,6 +17,9 @@ Sentry.init({ integrations: [anr], }); +Sentry.setUser({ email: 'person@home.com' }); +Sentry.addBreadcrumb({ message: 'important message!' }); + function longWorkIgnored() { for (let i = 0; i < 20; i++) { const salt = crypto.randomBytes(128).toString('base64'); diff --git a/dev-packages/node-integration-tests/suites/anr/test.ts b/dev-packages/node-integration-tests/suites/anr/test.ts index 9600978fb097..241a75e1d832 100644 --- a/dev-packages/node-integration-tests/suites/anr/test.ts +++ b/dev-packages/node-integration-tests/suites/anr/test.ts @@ -21,6 +21,15 @@ const EXPECTED_ANR_EVENT = { timezone: expect.any(String), }, }, + user: { + email: 'person@home.com', + }, + breadcrumbs: [ + { + timestamp: expect.any(Number), + message: 'important message!', + }, + ], // and an exception that is our ANR exception: { values: [ @@ -110,4 +119,34 @@ conditionalTest({ min: 16 })('should report ANR when event loop blocked', () => test('worker can be stopped and restarted', done => { createRunner(__dirname, 'stop-and-start.js').expect({ event: EXPECTED_ANR_EVENT }).start(done); }); + + const EXPECTED_ISOLATED_EVENT = { + user: { + id: 5, + }, + exception: { + values: [ + { + type: 'ApplicationNotResponding', + value: 'Application Not Responding for at least 100 ms', + mechanism: { type: 'ANR' }, + stacktrace: { + frames: expect.arrayContaining([ + { + colno: expect.any(Number), + lineno: expect.any(Number), + filename: expect.stringMatching(/isolated.mjs$/), + function: 'longWork', + in_app: true, + }, + ]), + }, + }, + ], + }, + }; + + test('fetches correct isolated scope', done => { + createRunner(__dirname, 'isolated.mjs').expect({ event: EXPECTED_ISOLATED_EVENT }).start(done); + }); }); diff --git a/packages/node/src/integrations/anr/index.ts b/packages/node/src/integrations/anr/index.ts index bf59b719fb18..0e27977fd2fb 100644 --- a/packages/node/src/integrations/anr/index.ts +++ b/packages/node/src/integrations/anr/index.ts @@ -1,6 +1,13 @@ // TODO (v8): This import can be removed once we only support Node with global URL import { URL } from 'url'; -import { convertIntegrationFnToClass, defineIntegration, getCurrentScope } from '@sentry/core'; +import { + convertIntegrationFnToClass, + defineIntegration, + getCurrentScope, + getGlobalScope, + getIsolationScope, + mergeScopeData, +} from '@sentry/core'; import type { Client, Contexts, @@ -9,9 +16,9 @@ import type { Integration, IntegrationClass, IntegrationFn, - IntegrationFnResult, + ScopeData, } from '@sentry/types'; -import { dynamicRequire, logger } from '@sentry/utils'; +import { GLOBAL_OBJ, dynamicRequire, logger } from '@sentry/utils'; import type { Worker, WorkerOptions } from 'worker_threads'; import type { NodeClient } from '../../client'; import { NODE_VERSION } from '../../nodeVersion'; @@ -31,6 +38,24 @@ function log(message: string, ...args: unknown[]): void { logger.log(`[ANR] ${message}`, ...args); } +function globalWithScopeFetchFn(): typeof GLOBAL_OBJ & { __SENTRY_GET_SCOPES__?: () => ScopeData } { + return GLOBAL_OBJ; +} + +/** Fetches merged scope data */ +function getScopeData(): ScopeData { + const scope = getGlobalScope().getScopeData(); + mergeScopeData(scope, getIsolationScope().getScopeData()); + mergeScopeData(scope, getCurrentScope().getScopeData()); + + // We remove attachments because they likely won't serialize well as json + scope.attachments = []; + // We can't serialize event processor functions + scope.eventProcessors = []; + + return scope; +} + /** * We need to use dynamicRequire because worker_threads is not available in node < v12 and webpack error will when * targeting those versions @@ -64,9 +89,18 @@ const INTEGRATION_NAME = 'Anr'; type AnrInternal = { startWorker: () => void; stopWorker: () => void }; const _anrIntegration = ((options: Partial = {}) => { + if (NODE_VERSION.major < 16 || (NODE_VERSION.major === 16 && NODE_VERSION.minor < 17)) { + throw new Error('ANR detection requires Node 16.17.0 or later'); + } + let worker: Promise<() => void> | undefined; let client: NodeClient | undefined; + // Hookup the scope fetch function to the global object so that it can be called from the worker thread via the + // debugger when it pauses + const gbl = globalWithScopeFetchFn(); + gbl.__SENTRY_GET_SCOPES__ = getScopeData; + return { name: INTEGRATION_NAME, // TODO v8: Remove this @@ -90,20 +124,16 @@ const _anrIntegration = ((options: Partial = {}) => { } }, setup(initClient: NodeClient) { - if (NODE_VERSION.major < 16 || (NODE_VERSION.major === 16 && NODE_VERSION.minor < 17)) { - throw new Error('ANR detection requires Node 16.17.0 or later'); - } - client = initClient; // setImmediate is used to ensure that all other integrations have had their setup called first. // This allows us to call into all integrations to fetch the full context setImmediate(() => this.startWorker()); }, - } as IntegrationFnResult & AnrInternal; + } as Integration & AnrInternal; }) satisfies IntegrationFn; -type AnrReturn = (options?: Partial) => IntegrationFnResult & AnrInternal; +type AnrReturn = (options?: Partial) => Integration & AnrInternal; export const anrIntegration = defineIntegration(_anrIntegration) as AnrReturn; diff --git a/packages/node/src/integrations/anr/worker.ts b/packages/node/src/integrations/anr/worker.ts index cff2c4c515ec..1c1b632cb7e8 100644 --- a/packages/node/src/integrations/anr/worker.ts +++ b/packages/node/src/integrations/anr/worker.ts @@ -1,11 +1,12 @@ import { + applyScopeDataToEvent, createEventEnvelope, createSessionEnvelope, getEnvelopeEndpointWithUrlEncodedAuth, makeSession, updateSession, } from '@sentry/core'; -import type { Event, Session, StackFrame, TraceContext } from '@sentry/types'; +import type { Event, ScopeData, Session, StackFrame } from '@sentry/types'; import { callFrameToStackFrame, normalizeUrlToBase, @@ -87,7 +88,23 @@ function prepareStackFrames(stackFrames: StackFrame[] | undefined): StackFrame[] return strippedFrames; } -async function sendAnrEvent(frames?: StackFrame[], traceContext?: TraceContext): Promise { +function applyScopeToEvent(event: Event, scope: ScopeData): void { + applyScopeDataToEvent(event, scope); + + if (!event.contexts?.trace) { + const { traceId, spanId, parentSpanId } = scope.propagationContext; + event.contexts = { + trace: { + trace_id: traceId, + span_id: spanId, + parent_span_id: parentSpanId, + }, + ...event.contexts, + }; + } +} + +async function sendAnrEvent(frames?: StackFrame[], scope?: ScopeData): Promise { if (hasSentAnrEvent) { return; } @@ -100,7 +117,7 @@ async function sendAnrEvent(frames?: StackFrame[], traceContext?: TraceContext): const event: Event = { event_id: uuid4(), - contexts: { ...options.contexts, trace: traceContext }, + contexts: options.contexts, release: options.release, environment: options.environment, dist: options.dist, @@ -120,8 +137,12 @@ async function sendAnrEvent(frames?: StackFrame[], traceContext?: TraceContext): tags: options.staticTags, }; + if (scope) { + applyScopeToEvent(event, scope); + } + const envelope = createEventEnvelope(event, options.dsn, options.sdkMetadata); - // Log the envelope so to aid in testing + // Log the envelope to aid in testing log(JSON.stringify(envelope)); await transport.send(envelope); @@ -172,20 +193,23 @@ if (options.captureStackTrace) { 'Runtime.evaluate', { // Grab the trace context from the current scope - expression: - 'var __sentry_ctx = __SENTRY__.hub.getScope().getPropagationContext(); __sentry_ctx.traceId + "-" + __sentry_ctx.spanId + "-" + __sentry_ctx.parentSpanId', + expression: 'global.__SENTRY_GET_SCOPES__();', // Don't re-trigger the debugger if this causes an error silent: true, + // Serialize the result to json otherwise only primitives are supported + returnByValue: true, }, - (_, param) => { - const traceId = param && param.result ? (param.result.value as string) : '--'; - const [trace_id, span_id, parent_span_id] = traceId.split('-') as (string | undefined)[]; + (err, param) => { + if (err) { + log(`Error executing script: '${err.message}'`); + } + + const scopes = param && param.result ? (param.result.value as ScopeData) : undefined; session.post('Debugger.resume'); session.post('Debugger.disable'); - const context = trace_id?.length && span_id?.length ? { trace_id, span_id, parent_span_id } : undefined; - sendAnrEvent(stackFrames, context).then(null, () => { + sendAnrEvent(stackFrames, scopes).then(null, () => { log('Sending ANR event failed.'); }); }, From 037bf150e307ab701a85cd9f132447504ef41479 Mon Sep 17 00:00:00 2001 From: Tim Fish Date: Mon, 25 Mar 2024 13:30:37 +0000 Subject: [PATCH 2/5] fix build --- packages/node/src/integrations/anr/index.ts | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/packages/node/src/integrations/anr/index.ts b/packages/node/src/integrations/anr/index.ts index 0e27977fd2fb..61d9d1169fec 100644 --- a/packages/node/src/integrations/anr/index.ts +++ b/packages/node/src/integrations/anr/index.ts @@ -16,6 +16,7 @@ import type { Integration, IntegrationClass, IntegrationFn, + IntegrationFnResult, ScopeData, } from '@sentry/types'; import { GLOBAL_OBJ, dynamicRequire, logger } from '@sentry/utils'; @@ -130,10 +131,10 @@ const _anrIntegration = ((options: Partial = {}) => { // This allows us to call into all integrations to fetch the full context setImmediate(() => this.startWorker()); }, - } as Integration & AnrInternal; + } as IntegrationFnResult & AnrInternal; }) satisfies IntegrationFn; -type AnrReturn = (options?: Partial) => Integration & AnrInternal; +type AnrReturn = (options?: Partial) => IntegrationFnResult & AnrInternal; export const anrIntegration = defineIntegration(_anrIntegration) as AnrReturn; From 8c032ad93d5176f2265e0b25bc97dc14dfe3662e Mon Sep 17 00:00:00 2001 From: Tim Fish Date: Thu, 28 Mar 2024 17:27:10 +0100 Subject: [PATCH 3/5] Fix legacy issue? --- packages/node/src/integrations/anr/legacy.ts | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/packages/node/src/integrations/anr/legacy.ts b/packages/node/src/integrations/anr/legacy.ts index d8b4ff1bc6dc..b2bc45c82bcb 100644 --- a/packages/node/src/integrations/anr/legacy.ts +++ b/packages/node/src/integrations/anr/legacy.ts @@ -1,6 +1,6 @@ import { getClient } from '@sentry/core'; -import { Anr } from '.'; import type { NodeClient } from '../../client'; +import { anrIntegration } from './index'; // TODO (v8): Remove this entire file and the `enableAnrDetection` export @@ -26,8 +26,7 @@ interface LegacyOptions { */ export function enableAnrDetection(options: Partial): Promise { const client = getClient() as NodeClient; - // eslint-disable-next-line deprecation/deprecation - const integration = new Anr(options); - integration.setup(client); + const integration = anrIntegration(options); + integration.setup?.(client); return Promise.resolve(); } From a8af8bc3b4430f31c62996ebe140e8c3974fce3e Mon Sep 17 00:00:00 2001 From: Tim Fish Date: Thu, 28 Mar 2024 17:36:50 +0100 Subject: [PATCH 4/5] Revert "Fix legacy issue?" This reverts commit 8c032ad93d5176f2265e0b25bc97dc14dfe3662e. --- packages/node/src/integrations/anr/legacy.ts | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/packages/node/src/integrations/anr/legacy.ts b/packages/node/src/integrations/anr/legacy.ts index b2bc45c82bcb..d8b4ff1bc6dc 100644 --- a/packages/node/src/integrations/anr/legacy.ts +++ b/packages/node/src/integrations/anr/legacy.ts @@ -1,6 +1,6 @@ import { getClient } from '@sentry/core'; +import { Anr } from '.'; import type { NodeClient } from '../../client'; -import { anrIntegration } from './index'; // TODO (v8): Remove this entire file and the `enableAnrDetection` export @@ -26,7 +26,8 @@ interface LegacyOptions { */ export function enableAnrDetection(options: Partial): Promise { const client = getClient() as NodeClient; - const integration = anrIntegration(options); - integration.setup?.(client); + // eslint-disable-next-line deprecation/deprecation + const integration = new Anr(options); + integration.setup(client); return Promise.resolve(); } From 48afea8e12c23138d90ab08c59c0de52fb4afac9 Mon Sep 17 00:00:00 2001 From: Tim Fish Date: Thu, 28 Mar 2024 17:38:28 +0100 Subject: [PATCH 5/5] Dont include scope with legacy test --- .../node-integration-tests/suites/anr/test.ts | 52 ++++++++++++++++++- 1 file changed, 51 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 241a75e1d832..bdbfd9cf822a 100644 --- a/dev-packages/node-integration-tests/suites/anr/test.ts +++ b/dev-packages/node-integration-tests/suites/anr/test.ts @@ -65,9 +65,59 @@ conditionalTest({ min: 16 })('should report ANR when event loop blocked', () => cleanupChildProcesses(); }); + const EXPECTED_LEGACY_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 100 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, + }, + ]), + }, + }, + ], + }, + }; + // TODO (v8): Remove this old API and this test test('Legacy API', done => { - createRunner(__dirname, 'legacy.js').expect({ event: EXPECTED_ANR_EVENT }).start(done); + createRunner(__dirname, 'legacy.js').expect({ event: EXPECTED_LEGACY_ANR_EVENT }).start(done); }); test('CJS', done => {