From 0aa7d955b651d0edfe0ec5587add5e8cf2de05f8 Mon Sep 17 00:00:00 2001 From: Tim Fish Date: Thu, 12 Dec 2024 15:14:10 +0100 Subject: [PATCH] fix: Disable ANR and Local Variables if debugger is enabled via CLI args (#14643) --- .../node-integration-tests/suites/anr/test.ts | 33 +++++++++++++++++-- packages/node/src/integrations/anr/index.ts | 8 ++++- .../local-variables/local-variables-async.ts | 8 ++++- .../local-variables/local-variables-sync.ts | 8 ++++- packages/node/src/utils/debug.ts | 18 ++++++++++ 5 files changed, 70 insertions(+), 5 deletions(-) create mode 100644 packages/node/src/utils/debug.ts diff --git a/dev-packages/node-integration-tests/suites/anr/test.ts b/dev-packages/node-integration-tests/suites/anr/test.ts index fd15df4bd0b8..b1750b308d28 100644 --- a/dev-packages/node-integration-tests/suites/anr/test.ts +++ b/dev-packages/node-integration-tests/suites/anr/test.ts @@ -52,6 +52,35 @@ const ANR_EVENT = { }, }; +const ANR_EVENT_WITHOUT_STACKTRACE = { + // Ensure we have context + contexts: { + 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: {}, + }, + ], + }, +}; + const ANR_EVENT_WITH_SCOPE = { ...ANR_EVENT, user: { @@ -98,11 +127,11 @@ conditionalTest({ min: 16 })('should report ANR when event loop blocked', () => createRunner(__dirname, 'indefinite.mjs').withMockSentryServer().expect({ event: ANR_EVENT }).start(done); }); - test('With --inspect', done => { + test("With --inspect the debugger isn't used", done => { createRunner(__dirname, 'basic.mjs') .withMockSentryServer() .withFlags('--inspect') - .expect({ event: ANR_EVENT_WITH_SCOPE }) + .expect({ event: ANR_EVENT_WITHOUT_STACKTRACE }) .start(done); }); diff --git a/packages/node/src/integrations/anr/index.ts b/packages/node/src/integrations/anr/index.ts index b93fcfd66612..f0cef4d60831 100644 --- a/packages/node/src/integrations/anr/index.ts +++ b/packages/node/src/integrations/anr/index.ts @@ -14,6 +14,7 @@ import { } from '@sentry/core'; import { NODE_VERSION } from '../../nodeVersion'; import type { NodeClient } from '../../sdk/client'; +import { isDebuggerEnabled } from '../../utils/debug'; import type { AnrIntegrationOptions, WorkerStartData } from './common'; const { isPromise } = types; @@ -98,9 +99,14 @@ const _anrIntegration = ((options: Partial = {}) => { }); } }, - setup(initClient: NodeClient) { + async setup(initClient: NodeClient) { client = initClient; + if (options.captureStackTrace && (await isDebuggerEnabled())) { + logger.warn('ANR captureStackTrace has been disabled because the debugger was already enabled'); + options.captureStackTrace = false; + } + // 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()); diff --git a/packages/node/src/integrations/local-variables/local-variables-async.ts b/packages/node/src/integrations/local-variables/local-variables-async.ts index 89d92e46bd59..e1e0ebadf755 100644 --- a/packages/node/src/integrations/local-variables/local-variables-async.ts +++ b/packages/node/src/integrations/local-variables/local-variables-async.ts @@ -2,6 +2,7 @@ import { Worker } from 'node:worker_threads'; import type { Event, EventHint, Exception, IntegrationFn } from '@sentry/core'; import { defineIntegration, logger } from '@sentry/core'; import type { NodeClient } from '../../sdk/client'; +import { isDebuggerEnabled } from '../../utils/debug'; import type { FrameVariables, LocalVariablesIntegrationOptions, LocalVariablesWorkerArgs } from './common'; import { LOCAL_VARIABLES_KEY, functionNamesMatch } from './common'; @@ -101,13 +102,18 @@ export const localVariablesAsyncIntegration = defineIntegration((( return { name: 'LocalVariablesAsync', - setup(client: NodeClient) { + async setup(client: NodeClient) { const clientOptions = client.getOptions(); if (!clientOptions.includeLocalVariables) { return; } + if (await isDebuggerEnabled()) { + logger.warn('Local variables capture has been disabled because the debugger was already enabled'); + return; + } + const options: LocalVariablesWorkerArgs = { ...integrationOptions, debug: logger.isEnabled(), diff --git a/packages/node/src/integrations/local-variables/local-variables-sync.ts b/packages/node/src/integrations/local-variables/local-variables-sync.ts index 4de0fe8aa478..3416dbf47347 100644 --- a/packages/node/src/integrations/local-variables/local-variables-sync.ts +++ b/packages/node/src/integrations/local-variables/local-variables-sync.ts @@ -3,6 +3,7 @@ import type { Event, Exception, IntegrationFn, StackFrame, StackParser } from '@ import { LRUMap, defineIntegration, getClient, logger } from '@sentry/core'; import { NODE_MAJOR } from '../../nodeVersion'; import type { NodeClient } from '../../sdk/client'; +import { isDebuggerEnabled } from '../../utils/debug'; import type { FrameVariables, LocalVariablesIntegrationOptions, @@ -289,7 +290,7 @@ const _localVariablesSyncIntegration = (( return { name: INTEGRATION_NAME, - setupOnce() { + async setupOnce() { const client = getClient(); const clientOptions = client?.getOptions(); @@ -306,6 +307,11 @@ const _localVariablesSyncIntegration = (( return; } + if (await isDebuggerEnabled()) { + logger.warn('Local variables capture has been disabled because the debugger was already enabled'); + return; + } + AsyncSession.create(sessionOverride).then( session => { function handlePaused( diff --git a/packages/node/src/utils/debug.ts b/packages/node/src/utils/debug.ts new file mode 100644 index 000000000000..71df5e761230 --- /dev/null +++ b/packages/node/src/utils/debug.ts @@ -0,0 +1,18 @@ +let cachedDebuggerEnabled: boolean | undefined; + +/** + * Was the debugger enabled when this function was first called? + */ +export async function isDebuggerEnabled(): Promise { + if (cachedDebuggerEnabled === undefined) { + try { + // Node can be built without inspector support + const inspector = await import('node:inspector'); + cachedDebuggerEnabled = !!inspector.url(); + } catch (_) { + cachedDebuggerEnabled = false; + } + } + + return cachedDebuggerEnabled; +}