From 8d2ecec6f1e896fba79e811825ae788e3131ddbd Mon Sep 17 00:00:00 2001 From: Tim Fish Date: Wed, 5 Apr 2023 10:11:30 +0200 Subject: [PATCH 1/5] Disable `LocalVariables` on Node < v18 --- .../suites/public-api/LocalVariables/test.ts | 9 +++------ packages/node/src/integrations/localvariables.ts | 7 ++++++- 2 files changed, 9 insertions(+), 7 deletions(-) diff --git a/packages/node-integration-tests/suites/public-api/LocalVariables/test.ts b/packages/node-integration-tests/suites/public-api/LocalVariables/test.ts index aae488d50475..d07959e9d10d 100644 --- a/packages/node-integration-tests/suites/public-api/LocalVariables/test.ts +++ b/packages/node-integration-tests/suites/public-api/LocalVariables/test.ts @@ -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); @@ -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'); diff --git a/packages/node/src/integrations/localvariables.ts b/packages/node/src/integrations/localvariables.ts index 0b5f8865e1ca..07ac9ba1580e 100644 --- a/packages/node/src/integrations/localvariables.ts +++ b/packages/node/src/integrations/localvariables.ts @@ -1,4 +1,5 @@ import type { Event, EventProcessor, Exception, Hub, Integration, StackFrame, StackParser } from '@sentry/types'; +import { parseSemver } from '@sentry/utils'; import type { Debugger, InspectorNotification, Runtime, Session } from 'inspector'; import { LRUMap } from 'lru_map'; @@ -281,7 +282,11 @@ export class LocalVariables implements Integration { addGlobalEventProcessor: (callback: EventProcessor) => void, 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 supportedNodeVersion = (parseSemver(process.version).major || 0) >= 18; + + if (this._session && clientOptions?.includeLocalVariables && supportedNodeVersion) { this._session.configureAndConnect( (ev, complete) => this._handlePaused(clientOptions.stackParser, ev as InspectorNotification, complete), From e0c10bf2b04b0a1600909dbd6c09e465151b3b36 Mon Sep 17 00:00:00 2001 From: Tim Fish Date: Wed, 5 Apr 2023 12:23:53 +0200 Subject: [PATCH 2/5] Use NODE_VERSION --- packages/node/src/integrations/localvariables.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/node/src/integrations/localvariables.ts b/packages/node/src/integrations/localvariables.ts index 07ac9ba1580e..7675d69dce9d 100644 --- a/packages/node/src/integrations/localvariables.ts +++ b/packages/node/src/integrations/localvariables.ts @@ -1,8 +1,8 @@ import type { Event, EventProcessor, Exception, Hub, Integration, StackFrame, StackParser } from '@sentry/types'; -import { parseSemver } 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; @@ -284,7 +284,7 @@ export class LocalVariables implements Integration { ): void { // Only setup this integration if the Node version is >= v18 // https://github.com/getsentry/sentry-javascript/issues/7697 - const supportedNodeVersion = (parseSemver(process.version).major || 0) >= 18; + const supportedNodeVersion = (NODE_VERSION.major || 0) >= 18; if (this._session && clientOptions?.includeLocalVariables && supportedNodeVersion) { this._session.configureAndConnect( From d8c93f87340717186331bc7af00d349acebf1ed5 Mon Sep 17 00:00:00 2001 From: Tim Fish Date: Wed, 5 Apr 2023 12:47:17 +0200 Subject: [PATCH 3/5] Log when node unsupported --- packages/node/src/integrations/localvariables.ts | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/packages/node/src/integrations/localvariables.ts b/packages/node/src/integrations/localvariables.ts index 7675d69dce9d..6d075099f6b2 100644 --- a/packages/node/src/integrations/localvariables.ts +++ b/packages/node/src/integrations/localvariables.ts @@ -4,6 +4,7 @@ import { LRUMap } from 'lru_map'; import { NODE_VERSION } from '../nodeVersion'; import type { NodeClientOptions } from '../types'; +import { logger } from '@sentry/utils'; type Variables = Record; type OnPauseEvent = InspectorNotification; @@ -282,11 +283,16 @@ export class LocalVariables implements Integration { addGlobalEventProcessor: (callback: EventProcessor) => void, clientOptions: NodeClientOptions | undefined, ): void { - // Only setup this integration if the Node version is >= v18 - // https://github.com/getsentry/sentry-javascript/issues/7697 - const supportedNodeVersion = (NODE_VERSION.major || 0) >= 18; + 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; + } - if (this._session && clientOptions?.includeLocalVariables && supportedNodeVersion) { this._session.configureAndConnect( (ev, complete) => this._handlePaused(clientOptions.stackParser, ev as InspectorNotification, complete), From ca908f0988ee3d47409d28ac3b55370a9b2eeba9 Mon Sep 17 00:00:00 2001 From: Tim Fish Date: Wed, 5 Apr 2023 13:52:05 +0200 Subject: [PATCH 4/5] Skip unit tests on older node versions --- packages/node/test/integrations/localvariables.test.ts | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/packages/node/test/integrations/localvariables.test.ts b/packages/node/test/integrations/localvariables.test.ts index 37e980c9cb96..6cd3527b9dae 100644 --- a/packages/node/test/integrations/localvariables.test.ts +++ b/packages/node/test/integrations/localvariables.test.ts @@ -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; @@ -145,7 +148,7 @@ const exceptionEvent100Frames = { }, }; -describe('LocalVariables', () => { +describeIf(NODE_VERSION >= 18)('LocalVariables', () => { it('Adds local variables to stack frames', async () => { expect.assertions(7); From 18764daf7dd1d55b456f872c0cf92a2cc7fcca29 Mon Sep 17 00:00:00 2001 From: Abhijeet Prasad Date: Wed, 5 Apr 2023 14:33:46 +0200 Subject: [PATCH 5/5] Update packages/node/src/integrations/localvariables.ts --- packages/node/src/integrations/localvariables.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/node/src/integrations/localvariables.ts b/packages/node/src/integrations/localvariables.ts index 6d075099f6b2..f98011d94832 100644 --- a/packages/node/src/integrations/localvariables.ts +++ b/packages/node/src/integrations/localvariables.ts @@ -1,10 +1,10 @@ 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'; -import { logger } from '@sentry/utils'; type Variables = Record; type OnPauseEvent = InspectorNotification;