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.
getsentry · Jan 3, 2024 · cd1e3bc · cd1e3bc
1 parent cdf574e
commit cd1e3bc
Show file tree

Hide file tree

Showing 9 changed files with 10 additions and 10 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -1,3 +1,3 @@
 import { parseSemver } from '@sentry/utils';
 
-export const NODE_VERSION: ReturnType<typeof parseSemver> = 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
@@ -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://<PROXY_URL>: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
@@ -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' },