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.25.4 #50025

Merged
merged 1 commit into from Oct 8, 2023
Merged

Conversation

nodejs-github-bot
Copy link
Collaborator

@nodejs-github-bot nodejs-github-bot commented Oct 3, 2023

This is an automated update of undici to 5.25.4.

@nodejs-github-bot nodejs-github-bot added dependencies Pull requests that update a dependency file. needs-ci PRs that need a full CI run. labels Oct 3, 2023
@KhafraDev
Copy link
Member

test failures seem like a weird issue with require where it's trying to require internal deps?

@KhafraDev
Copy link
Member

@fastify/busboy imports node:stream which causes the following error:

Missing internal module 'internal/deps/node:stream'
TypeError: Missing internal module 'internal/deps/node:stream'

@nodejs-github-bot nodejs-github-bot changed the title deps: update undici to 5.25.3 deps: update undici to 5.25.4 Oct 4, 2023
@mcollina
Copy link
Member

mcollina commented Oct 4, 2023

I think we need to revert the node:stream lookup in @fastify/busboy.

@KhafraDev
Copy link
Member

it seems like a bug in the wpt runner, @panva maybe?

@panva
Copy link
Member

panva commented Oct 4, 2023

@KhafraDev it doesn't seem like it to me

Missing internal module 'internal/deps/node:stream'
TypeError: Missing internal module 'internal/deps/node:stream'
    at requireBuiltin (node:internal/bootstrap/realm:422:19)
    at requireWithFallbackInDeps (node:internal/bootstrap/realm:432:10)
    at node_modules/@fastify/busboy/lib/main.js (node:internal/deps/undici/undici:3392:26)

@KhafraDev
Copy link
Member

KhafraDev commented Oct 4, 2023

I don't understand why it's trying to require internal/deps/node:stream when @fastify/busboy is only importing node:stream. Is that expected?

https://github.com/nodejs/node/pull/50025/files#diff-f516ab824a7722da938a4c7c851520d39731ddeb4f7198dff4e932c5d4f8fdf7R3392

(sorry I didn't know who else to ping)

@mcollina
Copy link
Member

mcollina commented Oct 4, 2023

I think we can revert that change in @fastify/busboy.

@KhafraDev
Copy link
Member

KhafraDev commented Oct 4, 2023

this is actually a bug in node, but removing the node: prefix will also work.

function requireWithFallbackInDeps(request) {
if (!BuiltinModule.map.has(request)) {
request = `internal/deps/${request}`;
}
return requireBuiltin(request);
}

@panva panva added the request-ci Add this label to start a Jenkins CI on a PR. label Oct 6, 2023
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Oct 6, 2023
@nodejs-github-bot
Copy link
Collaborator Author

@nodejs-github-bot
Copy link
Collaborator Author

@mcollina
Copy link
Member

mcollina commented Oct 7, 2023

The CI is not flaky, there is a tiny bug here.

@targos targos force-pushed the actions/tools-update-undici branch from a705b57 to 9253851 Compare October 7, 2023 09:06
@targos
Copy link
Member

targos commented Oct 7, 2023

Rebased to include 6b599a3 and make a single commit.

@mcollina mcollina added the request-ci Add this label to start a Jenkins CI on a PR. label Oct 7, 2023
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Oct 7, 2023
@nodejs-github-bot
Copy link
Collaborator Author

@nodejs-github-bot
Copy link
Collaborator Author

@panva panva added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Oct 8, 2023
Copy link
Member

@mcollina mcollina left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@mcollina mcollina added commit-queue Add this label to land a pull request using GitHub Actions. and removed needs-ci PRs that need a full CI run. labels Oct 8, 2023
@nodejs-github-bot nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Oct 8, 2023
@nodejs-github-bot nodejs-github-bot merged commit 588784e into main Oct 8, 2023
57 checks passed
@nodejs-github-bot nodejs-github-bot deleted the actions/tools-update-undici branch October 8, 2023 06:39
@nodejs-github-bot
Copy link
Collaborator Author

Landed in 588784e

alexfernandez pushed a commit to alexfernandez/node that referenced this pull request Nov 1, 2023
PR-URL: nodejs#50025
Reviewed-By: Filip Skokan <panva.ip@gmail.com>
Reviewed-By: Matthew Aitken <maitken033380023@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
@targos targos added the dont-land-on-v20.x PRs that should not land on the v20.x-staging branch and should not be released in v20.x. label Nov 11, 2023
@targos targos added the backported-to-v18.x PRs backported to the v18.x-staging branch. label Nov 26, 2023
debadree25 pushed a commit to debadree25/node that referenced this pull request Apr 15, 2024
PR-URL: nodejs#50025
Reviewed-By: Filip Skokan <panva.ip@gmail.com>
Reviewed-By: Matthew Aitken <maitken033380023@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
author ready PRs that have at least one approval, no pending requests for changes, and a CI started. backported-to-v18.x PRs backported to the v18.x-staging branch. dependencies Pull requests that update a dependency file. dont-land-on-v20.x PRs that should not land on the v20.x-staging branch and should not be released in v20.x.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants