Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: Only import inspector asynchronously #12231

Merged
merged 4 commits into from
May 27, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
import * as Sentry from '@sentry/node';
import Hook from 'import-in-the-middle';

Hook((_, name) => {
if (name === 'inspector') {
throw new Error('No inspector!');
}
if (name === 'node:inspector') {
throw new Error('No inspector!');
}
});

Sentry.init({});
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,14 @@ conditionalTest({ min: 18 })('LocalVariables integration', () => {
.start(done);
});

test('Should not import inspector when not in use', done => {
createRunner(__dirname, 'deny-inspector.mjs')
.withFlags('--import=@sentry/node/import')
.ensureNoErrorOutput()
.ignore('session')
.start(done);
});

test('Includes local variables for caught exceptions when enabled', done => {
createRunner(__dirname, 'local-variables-caught.js')
.ignore('session')
Expand Down
2 changes: 1 addition & 1 deletion packages/node/src/integrations/anr/index.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
import * as inspector from 'node:inspector';
import { Worker } from 'node:worker_threads';
import { defineIntegration, getCurrentScope, getGlobalScope, getIsolationScope, mergeScopeData } from '@sentry/core';
import type { Contexts, Event, EventHint, Integration, IntegrationFn, ScopeData } from '@sentry/types';
Expand Down Expand Up @@ -148,6 +147,7 @@ async function _startWorker(
};

if (options.captureStackTrace) {
const inspector = await import('node:inspector');
if (!inspector.url()) {
inspector.open(0);
}
Expand Down
2 changes: 1 addition & 1 deletion packages/node/src/integrations/anr/worker.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import { Session as InspectorSession } from 'node:inspector';
import { parentPort, workerData } from 'node:worker_threads';
import {
applyScopeDataToEvent,
Expand All @@ -15,7 +16,6 @@ import {
uuid4,
watchdogTimer,
} from '@sentry/utils';
import { Session as InspectorSession } from 'inspector';

import { makeNodeTransport } from '../../transports';
import { createGetModuleFromFilename } from '../../utils/module';
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ export const localVariablesAsyncIntegration = defineIntegration(((

async function startInspector(): Promise<void> {
// We load inspector dynamically because on some platforms Node is built without inspector support
const inspector = await import('inspector');
const inspector = await import('node:inspector');
if (!inspector.url()) {
inspector.open(0);
}
Expand Down
218 changes: 111 additions & 107 deletions packages/node/src/integrations/local-variables/local-variables-sync.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
import type { Debugger, InspectorNotification, Runtime } from 'node:inspector';
import { Session } from 'node:inspector';
import type { Debugger, InspectorNotification, Runtime, Session } from 'node:inspector';
import { defineIntegration, getClient } from '@sentry/core';
import type { Event, Exception, IntegrationFn, StackParser } from '@sentry/types';
import { LRUMap, logger } from '@sentry/utils';
Expand Down Expand Up @@ -75,11 +74,18 @@ export function createCallbackList<T>(complete: Next<T>): CallbackWrapper<T> {
* https://nodejs.org/docs/latest-v14.x/api/inspector.html
*/
class AsyncSession implements DebugSession {
private readonly _session: Session;

/** Throws if inspector API is not available */
public constructor() {
this._session = new Session();
private constructor(private readonly _session: Session) {
//
}

public static async create(orDefault?: DebugSession | undefined): Promise<DebugSession> {
if (orDefault) {
return orDefault;
}

const inspector = await import('node:inspector');
return new AsyncSession(new inspector.Session());
}

/** @inheritdoc */
Expand Down Expand Up @@ -194,85 +200,19 @@ class AsyncSession implements DebugSession {
}
}

/**
* When using Vercel pkg, the inspector module is not available.
* https://github.com/getsentry/sentry-javascript/issues/6769
*/
function tryNewAsyncSession(): AsyncSession | undefined {
try {
return new AsyncSession();
} catch (e) {
return undefined;
}
}

const INTEGRATION_NAME = 'LocalVariables';

/**
* Adds local variables to exception frames
*/
const _localVariablesSyncIntegration = ((
options: LocalVariablesIntegrationOptions = {},
session: DebugSession | undefined = tryNewAsyncSession(),
sessionOverride?: DebugSession,
) => {
const cachedFrames: LRUMap<string, FrameVariables[]> = new LRUMap(20);
let rateLimiter: RateLimitIncrement | undefined;
let shouldProcessEvent = false;

function handlePaused(
stackParser: StackParser,
{ params: { reason, data, callFrames } }: InspectorNotification<PausedExceptionEvent>,
complete: () => void,
): void {
if (reason !== 'exception' && reason !== 'promiseRejection') {
complete();
return;
}

rateLimiter?.();

// data.description contains the original error.stack
const exceptionHash = hashFromStack(stackParser, data?.description);

if (exceptionHash == undefined) {
complete();
return;
}

const { add, next } = createCallbackList<FrameVariables[]>(frames => {
cachedFrames.set(exceptionHash, frames);
complete();
});

// Because we're queuing up and making all these calls synchronously, we can potentially overflow the stack
// For this reason we only attempt to get local variables for the first 5 frames
for (let i = 0; i < Math.min(callFrames.length, 5); i++) {
const { scopeChain, functionName, this: obj } = callFrames[i];

const localScope = scopeChain.find(scope => scope.type === 'local');

// obj.className is undefined in ESM modules
const fn = obj.className === 'global' || !obj.className ? functionName : `${obj.className}.${functionName}`;

if (localScope?.object.objectId === undefined) {
add(frames => {
frames[i] = { function: fn };
next(frames);
});
} else {
const id = localScope.object.objectId;
add(frames =>
session?.getLocalVariables(id, vars => {
frames[i] = { function: fn, vars };
next(frames);
}),
);
}
}

next([]);
}

function addLocalVariablesToException(exception: Exception): void {
const hash = hashFrames(exception?.stacktrace?.frames);

Expand Down Expand Up @@ -330,44 +270,108 @@ const _localVariablesSyncIntegration = ((
const client = getClient<NodeClient>();
const clientOptions = client?.getOptions();

if (session && clientOptions?.includeLocalVariables) {
// Only setup this integration if the Node version is >= v18
// https://github.com/getsentry/sentry-javascript/issues/7697
const unsupportedNodeVersion = NODE_MAJOR < 18;
if (!clientOptions?.includeLocalVariables) {
return;
}

if (unsupportedNodeVersion) {
logger.log('The `LocalVariables` integration is only supported on Node >= v18.');
return;
}
// Only setup this integration if the Node version is >= v18
// https://github.com/getsentry/sentry-javascript/issues/7697
const unsupportedNodeVersion = NODE_MAJOR < 18;

const captureAll = options.captureAllExceptions !== false;

session.configureAndConnect(
(ev, complete) =>
handlePaused(clientOptions.stackParser, ev as InspectorNotification<PausedExceptionEvent>, complete),
captureAll,
);

if (captureAll) {
const max = options.maxExceptionsPerSecond || 50;

rateLimiter = createRateLimiter(
max,
() => {
logger.log('Local variables rate-limit lifted.');
session?.setPauseOnExceptions(true);
},
seconds => {
logger.log(
`Local variables rate-limit exceeded. Disabling capturing of caught exceptions for ${seconds} seconds.`,
);
session?.setPauseOnExceptions(false);
},
if (unsupportedNodeVersion) {
logger.log('The `LocalVariables` integration is only supported on Node >= v18.');
return;
}

AsyncSession.create(sessionOverride).then(
session => {
function handlePaused(
stackParser: StackParser,
{ params: { reason, data, callFrames } }: InspectorNotification<PausedExceptionEvent>,
complete: () => void,
): void {
if (reason !== 'exception' && reason !== 'promiseRejection') {
complete();
return;
}

rateLimiter?.();

// data.description contains the original error.stack
const exceptionHash = hashFromStack(stackParser, data?.description);

if (exceptionHash == undefined) {
complete();
return;
}

const { add, next } = createCallbackList<FrameVariables[]>(frames => {
cachedFrames.set(exceptionHash, frames);
complete();
});

// Because we're queuing up and making all these calls synchronously, we can potentially overflow the stack
// For this reason we only attempt to get local variables for the first 5 frames
for (let i = 0; i < Math.min(callFrames.length, 5); i++) {
const { scopeChain, functionName, this: obj } = callFrames[i];

const localScope = scopeChain.find(scope => scope.type === 'local');

// obj.className is undefined in ESM modules
const fn =
obj.className === 'global' || !obj.className ? functionName : `${obj.className}.${functionName}`;

if (localScope?.object.objectId === undefined) {
add(frames => {
frames[i] = { function: fn };
next(frames);
});
} else {
const id = localScope.object.objectId;
add(frames =>
session?.getLocalVariables(id, vars => {
frames[i] = { function: fn, vars };
next(frames);
}),
);
}
}

next([]);
}

const captureAll = options.captureAllExceptions !== false;

session.configureAndConnect(
(ev, complete) =>
handlePaused(clientOptions.stackParser, ev as InspectorNotification<PausedExceptionEvent>, complete),
captureAll,
);
}

shouldProcessEvent = true;
}
if (captureAll) {
const max = options.maxExceptionsPerSecond || 50;

rateLimiter = createRateLimiter(
max,
() => {
logger.log('Local variables rate-limit lifted.');
session?.setPauseOnExceptions(true);
},
seconds => {
logger.log(
`Local variables rate-limit exceeded. Disabling capturing of caught exceptions for ${seconds} seconds.`,
);
session?.setPauseOnExceptions(false);
},
);
}

shouldProcessEvent = true;
},
error => {
logger.log('The `LocalVariables` integration failed to start.', error);
},
);
},
processEvent(event: Event): Event {
if (shouldProcessEvent) {
Expand Down
Loading