Skip to content
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

make SSI not crash on node 12.0.0, 18.0.0, et.c #4934

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open
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
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
Loading