Skip to content

Commit

Permalink
fix(node): Disable LocalVariables integration on Node < v18 (#7748)
Browse files Browse the repository at this point in the history
Co-authored-by: Abhijeet Prasad <[email protected]>
  • Loading branch information
timfish and AbhiPrasad authored Apr 5, 2023
1 parent 1d7f18f commit 41ba98a
Show file tree
Hide file tree
Showing 3 changed files with 18 additions and 7 deletions.
Original file line number Diff line number Diff line change
@@ -1,13 +1,10 @@
import type { Event } from '@sentry/node';
import { parseSemver } from '@sentry/utils';
import * as childProcess from 'child_process';
import * as path from 'path';

const nodeMajor = parseSemver(process.version.slice(1)).major || 1;
import { conditionalTest } from '../../../utils';

const testIf = (condition: boolean, t: jest.It) => (condition ? t : t.skip);

describe('LocalVariables integration', () => {
conditionalTest({ min: 18 })('LocalVariables integration', () => {
test('Should not include local variables by default', done => {
expect.assertions(2);

Expand Down Expand Up @@ -57,7 +54,7 @@ describe('LocalVariables integration', () => {
});
});

testIf(nodeMajor > 10, test)('Should include local variables with ESM', done => {
test('Should include local variables with ESM', done => {
expect.assertions(4);

const testScriptPath = path.resolve(__dirname, 'local-variables-caught.mjs');
Expand Down
11 changes: 11 additions & 0 deletions packages/node/src/integrations/localvariables.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
import type { Event, EventProcessor, Exception, Hub, Integration, StackFrame, StackParser } from '@sentry/types';
import { logger } from '@sentry/utils';
import type { Debugger, InspectorNotification, Runtime, Session } from 'inspector';
import { LRUMap } from 'lru_map';

import { NODE_VERSION } from '../nodeVersion';
import type { NodeClientOptions } from '../types';

type Variables = Record<string, unknown>;
Expand Down Expand Up @@ -282,6 +284,15 @@ export class LocalVariables implements Integration {
clientOptions: NodeClientOptions | undefined,
): void {
if (this._session && clientOptions?.includeLocalVariables) {
// Only setup this integration if the Node version is >= v18
// https://github.com/getsentry/sentry-javascript/issues/7697
const unsupportedNodeVersion = (NODE_VERSION.major || 0) < 18;

if (unsupportedNodeVersion) {
logger.log('The `LocalVariables` integration is only supported on Node >= v18.');
return;
}

this._session.configureAndConnect(
(ev, complete) =>
this._handlePaused(clientOptions.stackParser, ev as InspectorNotification<PausedExceptionEvent>, complete),
Expand Down
5 changes: 4 additions & 1 deletion packages/node/test/integrations/localvariables.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,11 @@ import type { LRUMap } from 'lru_map';
import { defaultStackParser } from '../../src';
import type { DebugSession, FrameVariables } from '../../src/integrations/localvariables';
import { createCallbackList, LocalVariables } from '../../src/integrations/localvariables';
import { NODE_VERSION } from '../../src/nodeVersion';
import { getDefaultNodeClientOptions } from '../../test/helper/node-client-options';

const describeIf = (condition: boolean) => (condition ? describe : describe.skip);

interface ThrowOn {
configureAndConnect?: boolean;
getLocalVariables?: boolean;
Expand Down Expand Up @@ -145,7 +148,7 @@ const exceptionEvent100Frames = {
},
};

describe('LocalVariables', () => {
describeIf(NODE_VERSION >= 18)('LocalVariables', () => {
it('Adds local variables to stack frames', async () => {
expect.assertions(7);

Expand Down

0 comments on commit 41ba98a

Please sign in to comment.