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

TAV failing for [email protected] #3926

Closed
david-luna opened this issue Mar 21, 2024 · 4 comments · Fixed by #3927
Closed

TAV failing for [email protected] #3926

david-luna opened this issue Mar 21, 2024 · 4 comments · Fixed by #3927
Assignees

Comments

@david-luna
Copy link
Member

[email protected] is using a feature not supported in Node v14.

node_tests-1  | -- installing ["[email protected]"]
node_tests-1  | -- running test "node test/instrumentation/modules/tedious.test.js" with tedious (env: {})
node_tests-1  | /app/node_modules/@azure/core-rest-pipeline/dist/commonjs/policies/multipartPolicy.js:99
node_tests-1  |             boundary ??= parsedBoundary;
node_tests-1  |                      ^^^
node_tests-1  | 
node_tests-1  | SyntaxError: Unexpected token '??='
node_tests-1  |     at wrapSafe (internal/modules/cjs/loader.js:1029:16)
node_tests-1  |     at Module._compile (internal/modules/cjs/loader.js:1078:27)
node_tests-1  |     at Object.Module._extensions..js (internal/modules/cjs/loader.js:1143:10)
node_tests-1  |     at Module.load (internal/modules/cjs/loader.js:979:32)

Ref: https://github.com/elastic/apm-agent-nodejs/actions/runs/8354734062/job/22868580858#step:3:364

@david-luna
Copy link
Member Author

npm info tedious time

...
  '15.1.1': '2022-11-01T14:49:39.714Z',
  '15.1.2': '2022-11-09T13:13:18.399Z',
  '15.1.3': '2023-02-12T23:20:33.835Z',
  '16.0.0': '2023-04-07T20:42:21.850Z',
  '16.1.0': '2023-05-07T16:50:04.608Z',
...

it's weird that didn't fail before

@trentm
Copy link
Member

trentm commented Mar 21, 2024

Tedious v15 has this:

$ cat node_modules/tedious/package.json
...
  "engines": {
    "node": ">=14"
  },
...
  "dependencies": {
    "@azure/identity": "^2.0.4",
    "@azure/keyvault-keys": "^4.4.0",

@azure/core-rest-pipeline is a transitive dep of both of those azure packages.
The failure started for us as of @azure/[email protected] (released '2024-03-13T23:11:24.832Z'):

% npm install @azure/[email protected]
% rg -g "*" '\?\?='
...
node_modules/@azure/core-rest-pipeline/dist/commonjs/policies/multipartPolicy.js
99:            boundary ??= parsedBoundary;

I believe that ??= syntax was in the core-rest-pipeline source code in earlier versions -- added in commit 1691bef2d0 - [core] core-rest-pipeline multipartPolicy (#27487) (4 months ago) -- but wasn't in the released build until Azure/azure-sdk-for-js#26238, which was first released in v1.15.0.

I'd say @azure/[email protected] is no longer maintained (the current major is 4.x, the last 2.x release was '2.1.0': '2022-07-09T03:42:35.373Z'). As of that [email protected] the "engines": { "node": ">=12.0.0" }, suggestion is broken.

It looks like some (many? all?) @azure/... npm packages are dropping support for older Node.js versions without bumping major versions. And they have range deps on other @azure/... deps that do it... so there is no longer-term way to use a @azure/... package as a dependency that is safe against dropping Node.js versions without using a package-lock to forever pin transitive deps.

@trentm trentm self-assigned this Mar 21, 2024
@trentm
Copy link
Member

trentm commented Mar 21, 2024

This same breakage happens to tedious@13 and tedious@14 as well.

% node --version
v14.21.2

% TAV=tedious ./node_modules/.bin/tav --quiet --compat
Testing compatibility with tedious:
...
✖ 14.7.0
✖ 14.0.0
✖ 13.2.0
✖ 13.0.0
✔ 12.3.0
✔ 12.0.0

✔ 11.8.0
✔ 11.0.0

✔ 6.7.1
✔ 10.0.0
✔ 9.2.3
✔ 8.3.1
✔ 7.0.0
✔ 5.0.3
✔ 4.2.0
✔ 3.0.1
✔ 2.7.1
✔ 1.15.0
✔ 1.9.0

Effectively those tedious versions are all now node >=16 -- and might be node >=18 now or at some point, given that many of the @azure/... packages now have "engines": {"node": ">=18"},

@trentm
Copy link
Member

trentm commented Mar 21, 2024

I would think this would break tedious@12 as well, because it deps on the same version of azure/core-rest-pipeline:

└─┬ [email protected]
  ├─┬ @azure/[email protected]
  │ ├─┬ @azure/[email protected]
  │ │ └── @azure/[email protected] deduped
  │ └── @azure/[email protected]
  └─┬ @azure/[email protected]
    ├─┬ @azure/[email protected]
    │ └── @azure/[email protected] deduped
    └── @azure/[email protected] deduped

However the tests are passing with node v14 for it. I'm not sure why.
I guess tedious@12 and/or azure/identity v1 do not hit that code path.

In general I would say that all versions of tedious going back to tedious@11 -- when a dep on @azure/identity was added -- now cannot be relied upon to work with node <18... and perhaps that keeps moving up as newer versions of @azure/... packages drop support for Node.js versions without a major version bump.

trentm added a commit that referenced this issue Mar 21, 2024
This was necessitated by a transitive dep of tedious breaking support
for Node.js versions earlier than 16.

Closes: #3926
trentm added a commit that referenced this issue Mar 22, 2024
…14 (#3927)

This was necessitated by a transitive dep of tedious (`@azure/core-rest-pipeline`)
breaking support for Node.js versions earlier than 16.

Closes: #3926
PeterEinberger pushed a commit to fpm-git/apm-agent-nodejs that referenced this issue Aug 20, 2024
…14 (elastic#3927)

This was necessitated by a transitive dep of tedious (`@azure/core-rest-pipeline`)
breaking support for Node.js versions earlier than 16.

Closes: elastic#3926
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants