Skip to content

Conversation

@nodejs-github-bot
Copy link
Collaborator

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

This is an automated update of undici to 5.25.4.

@nodejs-github-botnodejs-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-botnodejs-github-bot changed the title deps: update undici to 5.25.3deps: update undici to 5.25.4Oct 4, 2023
@mcollina
Copy link
Member

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

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.

functionrequireWithFallbackInDeps(request){
if(!BuiltinModule.map.has(request)){
request=`internal/deps/${request}`;
}
returnrequireBuiltin(request);
}

@panvapanva added the request-ci Add this label to start a Jenkins CI on a PR. label Oct 6, 2023
@github-actionsgithub-actionsbot removed the request-ci Add this label to start a Jenkins CI on a PR. label Oct 6, 2023
@nodejs-github-bot
Copy link
CollaboratorAuthor

@nodejs-github-bot
Copy link
CollaboratorAuthor

@mcollina
Copy link
Member

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

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

Rebased to include 6b599a3 and make a single commit.

@mcollinamcollina added the request-ci Add this label to start a Jenkins CI on a PR. label Oct 7, 2023
@github-actionsgithub-actionsbot removed the request-ci Add this label to start a Jenkins CI on a PR. label Oct 7, 2023
@nodejs-github-bot
Copy link
CollaboratorAuthor

@nodejs-github-bot
Copy link
CollaboratorAuthor

@panvapanva 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

@mcollinamcollina left a comment

Choose a reason for hiding this comment

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

lgtm

@mcollinamcollina 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-botnodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Oct 8, 2023
@nodejs-github-botnodejs-github-bot merged commit 588784e into mainOct 8, 2023
@nodejs-github-botnodejs-github-bot deleted the actions/tools-update-undici branch October 8, 2023 06:39
@nodejs-github-bot
Copy link
CollaboratorAuthor

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 <[email protected]> Reviewed-By: Matthew Aitken <[email protected]> Reviewed-By: Matteo Collina <[email protected]>
@targostargos 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
debadree25 pushed a commit to debadree25/node that referenced this pull request Apr 15, 2024
PR-URL: nodejs#50025 Reviewed-By: Filip Skokan <[email protected]> Reviewed-By: Matthew Aitken <[email protected]> Reviewed-By: Matteo Collina <[email protected]>
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

author readyPRs that have at least one approval, no pending requests for changes, and a CI started.dependenciesPull requests that update a dependency file.dont-land-on-v20.xPRs 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.

6 participants

@nodejs-github-bot@KhafraDev@mcollina@panva@targos