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

deps: update undici to 5.26.0 #50145

Closed
wants to merge 1 commit into from

Conversation

nodejs-github-bot
Copy link
Collaborator

This is an automated update of undici to 5.26.0.

@nodejs-github-bot nodejs-github-bot added dependencies Pull requests that update a dependency file. lib / src Issues and PRs related to general changes in the lib or src directory. needs-ci PRs that need a full CI run. labels Oct 11, 2023
@panva panva added fast-track PRs that do not need to wait for 48 hours to land. request-ci Add this label to start a Jenkins CI on a PR. labels Oct 11, 2023
@github-actions

This comment was marked as outdated.

@panva

This comment was marked as outdated.

@panva
Copy link
Member

panva commented Oct 11, 2023

cc @nodejs/undici

@github-actions github-actions bot added request-ci-failed An error occurred while starting CI via request-ci label, and manual interventon is needed. and removed request-ci Add this label to start a Jenkins CI on a PR. labels Oct 11, 2023
@github-actions
Copy link
Contributor

Failed to start CI
node:internal/modules/cjs/loader:1028
  const err = new Error(message);
              ^

Error: Cannot find module 'call-bind'
Require stack:

  • /opt/hostedtoolcache/node/16.20.2/x64/lib/node_modules/@node-core/utils/node_modules/@ljharb/through/index.js
    at Function.Module._resolveFilename (node:internal/modules/cjs/loader:1028:15)
    at Function.Module._load (node:internal/modules/cjs/loader:873:27)
    at Module.require (node:internal/modules/cjs/loader:1100:19)
    at require (node:internal/modules/cjs/helpers:119:18)
    at Object. (/opt/hostedtoolcache/node/16.20.2/x64/lib/node_modules/@node-core/utils/node_modules/@ljharb/through/index.js:4:16)
    at Module._compile (node:internal/modules/cjs/loader:1198:14)
    at Object.Module._extensions..js (node:internal/modules/cjs/loader:1252:10)
    at Module.load (node:internal/modules/cjs/loader:1076:32)
    at Function.Module._load (node:internal/modules/cjs/loader:911:12)
    at ModuleWrap. (node:internal/modules/esm/translators:169:29) {
    code: 'MODULE_NOT_FOUND',
    requireStack: [
    '/opt/hostedtoolcache/node/16.20.2/x64/lib/node_modules/@node-core/utils/node_modules/@ljharb/through/index.js'
    ]
    }
https://github.com/nodejs/node/actions/runs/6483457798

@nodejs-github-bot
Copy link
Collaborator Author

@panva panva removed the fast-track PRs that do not need to wait for 48 hours to land. label Oct 11, 2023
@KhafraDev
Copy link
Member

I'll take a look at the errors, seem to be related

@panva
Copy link
Member

panva commented Oct 11, 2023

I'll take a look at the errors, seem to be related

It's coming from nodejs/undici#2310

@panva panva removed the request-ci-failed An error occurred while starting CI via request-ci label, and manual interventon is needed. label Oct 11, 2023
@panva
Copy link
Member

panva commented Oct 11, 2023

@KhafraDev
Copy link
Member

maybe we do need to use esbuild hooks

@mcollina
Copy link
Member

v5.26.2 incoming with security patch.

@panva
Copy link
Member

panva commented Oct 11, 2023

please just revert the user-agent changes entirely until the build steps are figured out.

@KhafraDev
Copy link
Member

I'll be home in an hour and a half, we won't be able to land until the esbuild issues are fixed

@Ethan-Arrowood
Copy link
Contributor

Ethan-Arrowood commented Oct 11, 2023

its cause its missing from the files list. adding now. releasing in 5.26.3 edit its not

@mcollina
Copy link
Member

its cause its missing from the files list. adding now. releasing in 5.26.3

No. There is a separate bug in the update script in Node, I'm opening a PR as soon as it builds.

@KhafraDev
Copy link
Member

The script is missing that was added to build it. I thought node would pull from the undici repo, not from npm. 😕

@Ethan-Arrowood
Copy link
Contributor

ty @mcollina for the assist: #50150

@panva
Copy link
Member

panva commented Oct 11, 2023

I believe both nodejs/undici@5351f1f and #50150 is needed if the script insists on pulling from npm and not git.

@Ethan-Arrowood
Copy link
Contributor

I already reverted my change: nodejs/undici@81e0a93 in undici. and didn't release 5.26.3.

I'll let you or Matteo take it from here. Feels like a too-many-cooks-in-the-kitchen situation.

@panva
Copy link
Member

panva commented Oct 11, 2023

I'm really merely observing.

My suggestion would be to revert to the previous build step and revert the user-agent changes entirely for now. You could use typeof __filename === 'undefined' to detect the require is coming from node instead and not bother with esbuild scripts and hooks and replaces and whatnot. Pending further testing and nodejs update script proofing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dependencies Pull requests that update a dependency file. lib / src Issues and PRs related to general changes in the lib or src directory. needs-ci PRs that need a full CI run.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants