googleapis / gaxios

An HTTP request client that provides an axios like interface over top of node-fetch. Super lightweight. Supports proxies and all sorts of other stuff.
Apache License 2.0
797 stars 60 forks source link

v6.7.0 broke Node.js 14 compatibility #637

Closed trentm closed 4 months ago

trentm commented 4 months ago

The upgrade of the uuid dependency to ^10.0.0 in #629 means gaxios@6.7.0 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

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 gaxios@6.7.0:

$ npm install gaxios@6.7.0
$ 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
asdf.20240712t083619@1.0.0 /Users/trentm/tmp/asdf.20240712T083619
└─┬ gaxios@6.7.0
  ├── extend@3.0.2
  ├─┬ https-proxy-agent@7.0.5
  │ ├─┬ agent-base@7.1.1
  │ │ └── debug@4.3.5 deduped
  │ └─┬ debug@4.3.5
  │   └── ms@2.1.2
  ├── is-stream@2.0.1
  ├─┬ node-fetch@2.7.0
  │ └─┬ whatwg-url@5.0.0
  │   ├── tr46@0.0.3
  │   └── webidl-conversions@3.0.1
  └── uuid@10.0.0
danielbankhead commented 4 months ago

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 commented 4 months ago

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".