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

v6.7.0 broke Node.js 14 compatibility #637

Closed
trentm opened this issue Jul 12, 2024 · 2 comments · Fixed by #638
Closed

v6.7.0 broke Node.js 14 compatibility #637

trentm opened this issue Jul 12, 2024 · 2 comments · Fixed by #638
Labels
priority: p2 Moderately-important priority. Fix may not be included in next release. type: bug Error or flaw in code with unintended results or allowing sub-optimal usage patterns.

Comments

@trentm
Copy link

trentm commented Jul 12, 2024

The upgrade of the uuid dependency to ^10.0.0 in #629 means [email protected] crashes on load in Node.js v14 versions before 14.18.0.

gaxios' package.json has:

  "engines": {
    "node": ">=14"
  },

which, granted, is advisory. Technically that range includes Node.js 14.0.0 and up.
gaxios' CI is testing with version "14", which is just the latest Node.js v14 release, so this won't get caught by CI.

The reason I hit this, is because CI in a project I work on specifically tests with Node.js v14.17.0 as its lowest supported version. That project uses @opentelemetry/resource-detector-gcp (https://github.com/open-telemetry/opentelemetry-js-contrib/blob/main/detectors/node/opentelemetry-resource-detector-gcp/package.json#L60) which uses gcp-metadata, which uses gaxios.

Options:

  1. Revert back to uuid@9 until there is a major rev of gaxios that drops Node v14 support.
  2. Bump the min-supported Node.js to "node": ">=14.18.0" in package.json.
  3. State that the intent with having "engines": { "node": ">=14" } is support for the latest release of Node.js 14, close this issue, and hope that a uuid@10 minor release doesn't break Node.js v14.21.3. (I realize Node.js v14 is ancient. :)

Thanks.

Environment details

  • OS: macOS, linux
  • Node.js version: v14.17.0
  • npm version: 6.14.13 (the default version that comes with node v14.17.0)
  • gaxios version: 6.7.0

Steps to reproduce

Get a version of Node.js earlier than 14.18.0 somehow, e.g.:

$ nvm install 14.17.0
$ nvm use 14.17.0

Then attempt to use [email protected]:

$ npm install [email protected]
$ node
Welcome to Node.js v14.17.0.
Type ".help" for more information.
> require('gaxios')
Uncaught Error: Cannot find module 'node:crypto'
Require stack:
- /Users/trentm/tmp/asdf.20240712T083619/node_modules/uuid/dist/rng.js
- /Users/trentm/tmp/asdf.20240712T083619/node_modules/uuid/dist/v1.js
- /Users/trentm/tmp/asdf.20240712T083619/node_modules/uuid/dist/index.js
- /Users/trentm/tmp/asdf.20240712T083619/node_modules/gaxios/build/src/gaxios.js
- /Users/trentm/tmp/asdf.20240712T083619/node_modules/gaxios/build/src/index.js
- <repl>
    at Function.Module._resolveFilename (internal/modules/cjs/loader.js:885:15)
    at Function.Module._load (internal/modules/cjs/loader.js:730:27)
    at Module.require (internal/modules/cjs/loader.js:957:19)
    at require (internal/modules/cjs/helpers.js:88:18) {
  code: 'MODULE_NOT_FOUND',
  requireStack: [
    '/Users/trentm/tmp/asdf.20240712T083619/node_modules/uuid/dist/rng.js',
    '/Users/trentm/tmp/asdf.20240712T083619/node_modules/uuid/dist/v1.js',
    '/Users/trentm/tmp/asdf.20240712T083619/node_modules/uuid/dist/index.js',
    '/Users/trentm/tmp/asdf.20240712T083619/node_modules/gaxios/build/src/gaxios.js',
    '/Users/trentm/tmp/asdf.20240712T083619/node_modules/gaxios/build/src/index.js',
    '<repl>'
  ]
}
>

$ npm ls
[email protected] /Users/trentm/tmp/asdf.20240712T083619
└─┬ [email protected]
  ├── [email protected]
  ├─┬ [email protected]
  │ ├─┬ [email protected]
  │ │ └── [email protected] deduped
  │ └─┬ [email protected]
  │   └── [email protected]
  ├── [email protected]
  ├─┬ [email protected]
  │ └─┬ [email protected]
  │   ├── [email protected]
  │   └── [email protected]
  └── [email protected]
@trentm trentm added priority: p2 Moderately-important priority. Fix may not be included in next release. type: bug Error or flaw in code with unintended results or allowing sub-optimal usage patterns. labels Jul 12, 2024
trentm added a commit to elastic/elastic-otel-node that referenced this issue Jul 12, 2024
The transitive [email protected] dep breaks on Node.js versions before
v14.18.0. (Actually uuid@10 is being used, which technically no longer
supports Node.js v14, but still works with later minors.)

Refs: googleapis/gaxios#637
@danielbankhead
Copy link
Contributor

Thanks for this; I'll create a revert PR in a sec.

Recently we've recently enabled constraintsFiltering in our renovate config to prevent this from happening in the future:

@trentm
Copy link
Author

trentm commented Jul 12, 2024

Thanks for this; I'll create a revert PR in a sec.

Thanks.

Recently we've recently enabled constraintsFiltering in our renovate config to prevent this from happening in the future:

TIL. My naive guess is that this would not have caught this particular change, because the uuid package doesn't set "engines".

trentm added a commit to elastic/elastic-otel-node that referenced this issue Aug 1, 2024
The transitive [email protected] dep breaks on Node.js versions before
v14.18.0. (Actually uuid@10 is being used, which technically no longer
supports Node.js v14, but still works with later minors.)

Also:
* fix the conditional for installing an npm that supports workspaces
* while we are changing Node versions we test with, the IITM fix came a while back, so we can test with latest v18 now

Refs: googleapis/gaxios#637
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
priority: p2 Moderately-important priority. Fix may not be included in next release. type: bug Error or flaw in code with unintended results or allowing sub-optimal usage patterns.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants