Skip to content

Commit

Permalink
fix: Disable ANR and Local Variables if debugger is enabled via CLI a…
Browse files Browse the repository at this point in the history
…rgs (#14643)
  • Loading branch information
timfish authored Dec 12, 2024
1 parent d9d3b06 commit 0aa7d95
Show file tree
Hide file tree
Showing 5 changed files with 70 additions and 5 deletions.
33 changes: 31 additions & 2 deletions dev-packages/node-integration-tests/suites/anr/test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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: {
Expand Down Expand Up @@ -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);
});

Expand Down
8 changes: 7 additions & 1 deletion packages/node/src/integrations/anr/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -98,9 +99,14 @@ const _anrIntegration = ((options: Partial<AnrIntegrationOptions> = {}) => {
});
}
},
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());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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';

Expand Down Expand Up @@ -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(),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -289,7 +290,7 @@ const _localVariablesSyncIntegration = ((

return {
name: INTEGRATION_NAME,
setupOnce() {
async setupOnce() {
const client = getClient<NodeClient>();
const clientOptions = client?.getOptions();

Expand All @@ -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(
Expand Down
18 changes: 18 additions & 0 deletions packages/node/src/utils/debug.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
let cachedDebuggerEnabled: boolean | undefined;

/**
* Was the debugger enabled when this function was first called?
*/
export async function isDebuggerEnabled(): Promise<boolean> {
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;
}

0 comments on commit 0aa7d95

Please sign in to comment.