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

feat(node): Add option to capture local variables for caught exceptions via LocalVariables integration #6876

Merged
merged 5 commits into from Jan 23, 2023
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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
@@ -0,0 +1,36 @@
/* eslint-disable no-unused-vars */
const Sentry = require('@sentry/node');

Sentry.init({
dsn: 'https://public@dsn.ingest.sentry.io/1337',
includeLocalVariables: true,
integrations: [new Sentry.Integrations.LocalVariables({ captureAllExceptions: true })],
beforeSend: event => {
// eslint-disable-next-line no-console
console.log(JSON.stringify(event));
},
});

class Some {
two(name) {
throw new Error('Enough!');
}
}

function one(name) {
const arr = [1, '2', null];
const obj = {
name,
num: 5,
};

const ty = new Some();

ty.two(name);
}

try {
one('some name');
} catch (e) {
Sentry.captureException(e);
}
Expand Up @@ -51,4 +51,32 @@ describe('LocalVariables integration', () => {
done();
});
});

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: '<Some>',
});

done();
});
});
});
28 changes: 21 additions & 7 deletions packages/node/src/integrations/localvariables.ts
Expand Up @@ -6,7 +6,10 @@ import type { NodeClientOptions } from '../types';

export interface DebugSession {
/** Configures and connects to the debug session */
configureAndConnect(onPause: (message: InspectorNotification<Debugger.PausedEventDataType>) => void): void;
configureAndConnect(
onPause: (message: InspectorNotification<Debugger.PausedEventDataType>) => void,
captureAll: boolean,
): void;
/** Gets local variables for an objectId */
getLocalVariables(objectId: string): Promise<Record<string, unknown>>;
}
Expand All @@ -32,12 +35,15 @@ class AsyncSession implements DebugSession {
}

/** @inheritdoc */
public configureAndConnect(onPause: (message: InspectorNotification<Debugger.PausedEventDataType>) => void): void {
public configureAndConnect(
onPause: (message: InspectorNotification<Debugger.PausedEventDataType>) => void,
captureAll: boolean,
): void {
this._session.connect();
this._session.on('Debugger.paused', onPause);
this._session.post('Debugger.enable');
// We only want to pause on uncaught exceptions
this._session.post('Debugger.setPauseOnExceptions', { state: 'uncaught' });
this._session.post('Debugger.setPauseOnExceptions', { state: captureAll ? 'all' : 'uncaught' });
}

/** @inheritdoc */
Expand Down Expand Up @@ -164,7 +170,14 @@ export interface FrameVariables {

/** There are no options yet. This allows them to be added later without breaking changes */
// eslint-disable-next-line @typescript-eslint/no-empty-interface
interface Options {}
interface Options {
/**
* Capture local variables for both handled and unhandled exceptions
*
* Default: false - Only captures local variables for uncaught exceptions
*/
captureAllExceptions?: boolean;
}

/**
* Adds local variables to exception frames
Expand All @@ -177,7 +190,7 @@ export class LocalVariables implements Integration {
private readonly _cachedFrames: LRUMap<string, Promise<FrameVariables[]>> = new LRUMap(20);

public constructor(
_options: Options = {},
private readonly _options: Options = {},
private readonly _session: DebugSession | undefined = tryNewAsyncSession(),
) {}

Expand All @@ -194,8 +207,9 @@ export class LocalVariables implements Integration {
clientOptions: NodeClientOptions | undefined,
): void {
if (this._session && clientOptions?.includeLocalVariables) {
this._session.configureAndConnect(ev =>
this._handlePaused(clientOptions.stackParser, ev as InspectorNotification<PausedExceptionEvent>),
this._session.configureAndConnect(
ev => this._handlePaused(clientOptions.stackParser, ev as InspectorNotification<PausedExceptionEvent>),
!!this._options.captureAllExceptions,
);

addGlobalEventProcessor(async event => this._addLocalVariables(event));
Expand Down
5 changes: 4 additions & 1 deletion packages/node/test/integrations/localvariables.test.ts
Expand Up @@ -17,7 +17,10 @@ class MockDebugSession implements DebugSession {

constructor(private readonly _vars: Record<string, Record<string, unknown>>, private readonly _throwOn?: ThrowOn) {}

public configureAndConnect(onPause: (message: InspectorNotification<Debugger.PausedEventDataType>) => void): void {
public configureAndConnect(
onPause: (message: InspectorNotification<Debugger.PausedEventDataType>) => void,
_captureAll: boolean,
): void {
if (this._throwOn?.configureAndConnect) {
throw new Error('configureAndConnect should not be called');
}
Expand Down