From 1e5687d84549482cf1dc9d9c1004dc7c4d0ebdc3 Mon Sep 17 00:00:00 2001 From: Tim Fish Date: Thu, 21 Dec 2023 21:31:12 +0000 Subject: [PATCH 1/2] fix(node): Make NODE_VERSION properties required --- packages/node-experimental/src/sdk/init.ts | 2 +- packages/node/src/async/index.ts | 2 +- packages/node/src/integrations/anr/index.ts | 2 +- packages/node/src/integrations/http.ts | 2 +- packages/node/src/integrations/localvariables.ts | 2 +- packages/node/src/integrations/undici/index.ts | 2 +- packages/node/src/integrations/utils/http.ts | 2 +- packages/node/src/nodeVersion.ts | 4 +++- packages/node/test/integrations/http.test.ts | 4 ++-- packages/node/test/integrations/localvariables.test.ts | 2 +- 10 files changed, 13 insertions(+), 11 deletions(-) diff --git a/packages/node-experimental/src/sdk/init.ts b/packages/node-experimental/src/sdk/init.ts index 821757a9a246..cea85514d449 100644 --- a/packages/node-experimental/src/sdk/init.ts +++ b/packages/node-experimental/src/sdk/init.ts @@ -37,7 +37,7 @@ export const defaultIntegrations: Integration[] = [ ]; // Only add NodeFetch if Node >= 16, as previous versions do not support it -if (NODE_VERSION.major && NODE_VERSION.major >= 16) { +if (NODE_VERSION.major >= 16) { defaultIntegrations.push(new NodeFetch()); } diff --git a/packages/node/src/async/index.ts b/packages/node/src/async/index.ts index c8bb8e2ebb9f..a9563e4f0ce6 100644 --- a/packages/node/src/async/index.ts +++ b/packages/node/src/async/index.ts @@ -9,7 +9,7 @@ import { setHooksAsyncContextStrategy } from './hooks'; * Node.js < 14 uses domains */ export function setNodeAsyncContextStrategy(): void { - if (NODE_VERSION.major && NODE_VERSION.major >= 14) { + if (NODE_VERSION.major >= 14) { setHooksAsyncContextStrategy(); } else { setDomainAsyncContextStrategy(); diff --git a/packages/node/src/integrations/anr/index.ts b/packages/node/src/integrations/anr/index.ts index 4a623fb3ff0b..cf1f1cd6cab7 100644 --- a/packages/node/src/integrations/anr/index.ts +++ b/packages/node/src/integrations/anr/index.ts @@ -65,7 +65,7 @@ export class Anr implements Integration { /** @inheritdoc */ public setup(client: NodeClient): void { - if ((NODE_VERSION.major || 0) < 16) { + if (NODE_VERSION.major < 16) { throw new Error('ANR detection requires Node 16 or later'); } diff --git a/packages/node/src/integrations/http.ts b/packages/node/src/integrations/http.ts index 5d6b8857f93b..f0734142a3e1 100644 --- a/packages/node/src/integrations/http.ts +++ b/packages/node/src/integrations/http.ts @@ -132,7 +132,7 @@ export class Http implements Integration { // NOTE: Prior to Node 9, `https` used internals of `http` module, thus we don't patch it. // If we do, we'd get double breadcrumbs and double spans for `https` calls. // It has been changed in Node 9, so for all versions equal and above, we patch `https` separately. - if (NODE_VERSION.major && NODE_VERSION.major > 8) { + if (NODE_VERSION.major > 8) { // eslint-disable-next-line @typescript-eslint/no-var-requires const httpsModule = require('https'); const wrappedHttpsHandlerMaker = _createWrappedRequestMethodFactory( diff --git a/packages/node/src/integrations/localvariables.ts b/packages/node/src/integrations/localvariables.ts index 1ba9907c4806..f1a410501b4e 100644 --- a/packages/node/src/integrations/localvariables.ts +++ b/packages/node/src/integrations/localvariables.ts @@ -353,7 +353,7 @@ export class LocalVariables implements Integration { 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; + const unsupportedNodeVersion = NODE_VERSION.major < 18; if (unsupportedNodeVersion) { logger.log('The `LocalVariables` integration is only supported on Node >= v18.'); diff --git a/packages/node/src/integrations/undici/index.ts b/packages/node/src/integrations/undici/index.ts index 4f67993f7321..d7111a6076de 100644 --- a/packages/node/src/integrations/undici/index.ts +++ b/packages/node/src/integrations/undici/index.ts @@ -96,7 +96,7 @@ export class Undici implements Integration { */ public setupOnce(_addGlobalEventProcessor: (callback: EventProcessor) => void): void { // Requires Node 16+ to use the diagnostics_channel API. - if (NODE_VERSION.major && NODE_VERSION.major < 16) { + if (NODE_VERSION.major < 16) { return; } diff --git a/packages/node/src/integrations/utils/http.ts b/packages/node/src/integrations/utils/http.ts index fc2e6a1e88bd..76a826801814 100644 --- a/packages/node/src/integrations/utils/http.ts +++ b/packages/node/src/integrations/utils/http.ts @@ -183,7 +183,7 @@ export function normalizeRequestArgs( // as it will always return `http`, even when using `https` module. // // See test/integrations/http.test.ts for more details on Node <=v8 protocol issue. - if (NODE_VERSION.major && NODE_VERSION.major > 8) { + if (NODE_VERSION.major > 8) { requestOptions.protocol = (httpModule?.globalAgent as any)?.protocol || (requestOptions.agent as any)?.protocol || diff --git a/packages/node/src/nodeVersion.ts b/packages/node/src/nodeVersion.ts index e67fa3f1b2b9..fa4bbdde2b43 100644 --- a/packages/node/src/nodeVersion.ts +++ b/packages/node/src/nodeVersion.ts @@ -1,3 +1,5 @@ import { parseSemver } from '@sentry/utils'; -export const NODE_VERSION: ReturnType = parseSemver(process.versions.node); +type SemVerNotOptional = Required, 'major' | 'minor' | 'patch'>>; + +export const NODE_VERSION = parseSemver(process.versions.node) as SemVerNotOptional; diff --git a/packages/node/test/integrations/http.test.ts b/packages/node/test/integrations/http.test.ts index 2055aefeca39..42eb9391ec52 100644 --- a/packages/node/test/integrations/http.test.ts +++ b/packages/node/test/integrations/http.test.ts @@ -648,7 +648,7 @@ describe('default protocols', () => { // intercepting a https:// request with http:// mock. It's a safe bet // because the latest versions of nock no longer support Node v8 and lower, // so won't bother dealing with this old Node edge case. - if (NODE_VERSION.major && NODE_VERSION.major < 9) { + if (NODE_VERSION.major < 9) { nockProtocol = 'http'; } nock(`${nockProtocol}://${key}.ingest.sentry.io`).get('/api/123122332/store/').reply(200); @@ -671,7 +671,7 @@ describe('default protocols', () => { const proxy = 'http://:3128'; const agent = HttpsProxyAgent(proxy); - if (NODE_VERSION.major && NODE_VERSION.major < 9) { + if (NODE_VERSION.major < 9) { nockProtocol = 'http'; } diff --git a/packages/node/test/integrations/localvariables.test.ts b/packages/node/test/integrations/localvariables.test.ts index 027ccc391c6d..b5fcc051c411 100644 --- a/packages/node/test/integrations/localvariables.test.ts +++ b/packages/node/test/integrations/localvariables.test.ts @@ -150,7 +150,7 @@ const exceptionEvent100Frames = { }, }; -describeIf((NODE_VERSION.major || 0) >= 18)('LocalVariables', () => { +describeIf(NODE_VERSION.major >= 18)('LocalVariables', () => { it('Adds local variables to stack frames', async () => { const session = new MockDebugSession({ '-6224981551105448869.1.2': { name: 'tim' }, From 8db5e0e48e742f74afd512ae8d67574f25cb54f8 Mon Sep 17 00:00:00 2001 From: Tim Fish Date: Thu, 21 Dec 2023 21:51:12 +0000 Subject: [PATCH 2/2] Fix build --- packages/node-experimental/src/sdk/init.ts | 2 +- packages/node/src/nodeVersion.ts | 4 +--- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/packages/node-experimental/src/sdk/init.ts b/packages/node-experimental/src/sdk/init.ts index cea85514d449..821757a9a246 100644 --- a/packages/node-experimental/src/sdk/init.ts +++ b/packages/node-experimental/src/sdk/init.ts @@ -37,7 +37,7 @@ export const defaultIntegrations: Integration[] = [ ]; // Only add NodeFetch if Node >= 16, as previous versions do not support it -if (NODE_VERSION.major >= 16) { +if (NODE_VERSION.major && NODE_VERSION.major >= 16) { defaultIntegrations.push(new NodeFetch()); } diff --git a/packages/node/src/nodeVersion.ts b/packages/node/src/nodeVersion.ts index fa4bbdde2b43..1574237f3fb4 100644 --- a/packages/node/src/nodeVersion.ts +++ b/packages/node/src/nodeVersion.ts @@ -1,5 +1,3 @@ import { parseSemver } from '@sentry/utils'; -type SemVerNotOptional = Required, 'major' | 'minor' | 'patch'>>; - -export const NODE_VERSION = parseSemver(process.versions.node) as SemVerNotOptional; +export const NODE_VERSION = parseSemver(process.versions.node) as { major: number; minor: number; patch: number };