From cd1e3bcdb56e9ec9c1da2ea4e27e7c39495602d6 Mon Sep 17 00:00:00 2001 From: Tim Fish Date: Fri, 22 Dec 2023 15:24:23 +0100 Subject: [PATCH] fix(node): Make `NODE_VERSION` properties required (#9964) These types have been annoying me for a while. We can safely assume the node version can be parsed. --- 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 | 2 +- packages/node/test/integrations/http.test.ts | 4 ++-- packages/node/test/integrations/localvariables.test.ts | 2 +- 9 files changed, 10 insertions(+), 10 deletions(-) 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..1574237f3fb4 100644 --- a/packages/node/src/nodeVersion.ts +++ b/packages/node/src/nodeVersion.ts @@ -1,3 +1,3 @@ import { parseSemver } from '@sentry/utils'; -export const NODE_VERSION: ReturnType = parseSemver(process.versions.node); +export const NODE_VERSION = parseSemver(process.versions.node) as { major: number; minor: number; patch: number }; 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' },