Skip to content

fix(node): Make NODE_VERSION properties required #9964

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Dec 22, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion packages/node/src/async/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
2 changes: 1 addition & 1 deletion packages/node/src/integrations/anr/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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');
}

Expand Down
2 changes: 1 addition & 1 deletion packages/node/src/integrations/http.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
2 changes: 1 addition & 1 deletion packages/node/src/integrations/localvariables.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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.');
Expand Down
2 changes: 1 addition & 1 deletion packages/node/src/integrations/undici/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand Down
2 changes: 1 addition & 1 deletion packages/node/src/integrations/utils/http.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 ||
Expand Down
2 changes: 1 addition & 1 deletion packages/node/src/nodeVersion.ts
Original file line number Diff line number Diff line change
@@ -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 };
4 changes: 2 additions & 2 deletions packages/node/test/integrations/http.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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';
}

Expand Down
2 changes: 1 addition & 1 deletion packages/node/test/integrations/localvariables.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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' },
Expand Down