Skip to content

Commit

Permalink
make SSI not crash on node 12.0.0, 18.0.0, et.c (#4934)
Browse files Browse the repository at this point in the history
* make it not crash on 12.0.0

* wider ranged test matrix for init.spec.js

* make 18.0.0 not crash, even though ESM can't be supported in it

* isolate old/weird node version tests to the init test
  • Loading branch information
bengl authored Nov 25, 2024
1 parent 7571290 commit cd0b922
Show file tree
Hide file tree
Showing 4 changed files with 47 additions and 11 deletions.
4 changes: 2 additions & 2 deletions .github/workflows/project.yml
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ jobs:
# setting fail-fast to false in an attempt to prevent this from happening
fail-fast: false
matrix:
version: [18, 20, latest]
version: [18, 20, 22, latest]
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v4
Expand All @@ -34,7 +34,7 @@ jobs:
integration-guardrails:
strategy:
matrix:
version: [12, 14, 16]
version: [12.0.0, 12, 14.0.0, 14, 16.0.0, 16, 18.0.0, 18.1.0, 20.0.0, 22.0.0]
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v4
Expand Down
6 changes: 4 additions & 2 deletions init.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,9 @@

/* eslint-disable no-var */

var NODE_MAJOR = require('./version').NODE_MAJOR
var nodeVersion = require('./version')
var NODE_MAJOR = nodeVersion.NODE_MAJOR
var NODE_MINOR = nodeVersion.NODE_MINOR

// We use several things that are not supported by older versions of Node:
// - AsyncLocalStorage
Expand All @@ -11,7 +13,7 @@ var NODE_MAJOR = require('./version').NODE_MAJOR
// - Mocha (for testing)
// and probably others.
// TODO: Remove all these dependencies so that we can report telemetry.
if (NODE_MAJOR >= 12) {
if ((NODE_MAJOR === 12 && NODE_MINOR >= 17) || NODE_MAJOR > 12) {
var path = require('path')
var Module = require('module')
var semver = require('semver')
Expand Down
9 changes: 7 additions & 2 deletions initialize.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -31,11 +31,16 @@ ${result.source}`
return result
}

const [NODE_MAJOR, NODE_MINOR] = process.versions.node.split('.').map(x => +x)

const brokenLoaders = NODE_MAJOR === 18 && NODE_MINOR === 0

export async function load (...args) {
return insertInit(await origLoad(...args))
const loadHook = brokenLoaders ? args[args.length - 1] : origLoad
return insertInit(await loadHook(...args))
}

export const resolve = origResolve
export const resolve = brokenLoaders ? undefined : origResolve

export const getFormat = origGetFormat

Expand Down
39 changes: 34 additions & 5 deletions integration-tests/init.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ const telemetryGood = ['complete', 'injection_forced:false']
const { engines } = require('../package.json')
const supportedRange = engines.node
const currentVersionIsSupported = semver.satisfies(process.versions.node, supportedRange)
const currentVersionCanLog = semver.satisfies(process.versions.node, '>=12.17.0')

// These are on by default in release tests, so we'll turn them off for
// more fine-grained control of these variables in these tests.
Expand Down Expand Up @@ -83,7 +84,30 @@ function testRuntimeVersionChecks (arg, filename) {
}
}

if (!currentVersionIsSupported) {
if (!currentVersionCanLog) {
context('when node version is too low for AsyncLocalStorage', () => {
useEnv({ NODE_OPTIONS })

it('should initialize the tracer, if no DD_INJECTION_ENABLED', () =>
doTest('false\n'))
context('with DD_INJECTION_ENABLED', () => {
useEnv({ DD_INJECTION_ENABLED })

context('without debug', () => {
it('should not initialize the tracer', () => doTest('false\n'))
it('should not, if DD_INJECT_FORCE', () => doTestForced('false\n'))
})
context('with debug', () => {
useEnv({ DD_TRACE_DEBUG })

it('should not initialize the tracer', () =>
doTest('false\n'))
it('should initialize the tracer, if DD_INJECT_FORCE', () =>
doTestForced('false\n'))
})
})
})
} else if (!currentVersionIsSupported) {
context('when node version is less than engines field', () => {
useEnv({ NODE_OPTIONS })

Expand Down Expand Up @@ -165,17 +189,22 @@ describe('init.js', () => {
testRuntimeVersionChecks('require', 'init.js')
})

// ESM is not supportable prior to Node.js 12
if (semver.satisfies(process.versions.node, '>=12')) {
// ESM is not supportable prior to Node.js 12.17.0, 14.13.1 on the 14.x line,
// or on 18.0.0 in particular.
if (
semver.satisfies(process.versions.node, '>=12.17.0') &&
semver.satisfies(process.versions.node, '>=14.13.1')
) {
describe('initialize.mjs', () => {
useSandbox()
stubTracerIfNeeded()

context('as --loader', () => {
testInjectionScenarios('loader', 'initialize.mjs', true)
testInjectionScenarios('loader', 'initialize.mjs',
process.versions.node !== '18.0.0')
testRuntimeVersionChecks('loader', 'initialize.mjs')
})
if (Number(process.versions.node.split('.')[0]) >= 18) {
if (semver.satisfies(process.versions.node, '>=20.6.0')) {
context('as --import', () => {
testInjectionScenarios('import', 'initialize.mjs', true)
testRuntimeVersionChecks('loader', 'initialize.mjs')
Expand Down

0 comments on commit cd0b922

Please sign in to comment.