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

fix: undici incorrectly copies headers onto fetches #41019

Merged
merged 1 commit into from Jan 24, 2024

Conversation

codebytere
Copy link
Member

Description of Change

Closes #41005.
Refs nodejs/node#50153
Refs nodejs/undici#2359

Fixes an issue where undici in migrating build scripts accidentally regressed constructor name exports.

This is fixed in Node.js v20.10.0 and beyond by nodejs/node#50274 but as that PR is very large, this cherry-picks out just the fix for the constructor issue by running the build script with the missing keepNames argument and copying the resulting undici-fetch.js output to deps/undici/undici.js, as Node.js themselves do in their update script

Checklist

Release Notes

Notes: Fixes an issue where Request objects did not correctly copy headers into fetches.

@codebytere codebytere added backport This is a backport PR backport-check-skip Skip trop's backport validity checking 28-x-y target/28-x-y PR should also be added to the "28-x-y" branch. 29-x-y labels Jan 17, 2024
@codebytere codebytere requested a review from a team as a code owner January 17, 2024 12:33
Copy link
Member

@ckerr ckerr left a comment

Choose a reason for hiding this comment

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

oof, that's a big cherry-pick 🍒

Copy link
Contributor

@jkleinsc jkleinsc left a comment

Choose a reason for hiding this comment

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

Patch fails to apply

@codebytere
Copy link
Member Author

@jkleinsc i got some weird CRLF errors in console initially - i think it might be related. I'll check it out - it applied locally and i didn't make it too hackily so idk what else it could be 🤔

@codebytere codebytere added semver/patch backwards-compatible bug fixes and removed 28-x-y labels Jan 17, 2024
@codebytere codebytere linked an issue Jan 17, 2024 that may be closed by this pull request
3 tasks
@jkleinsc jkleinsc merged commit dd142b0 into 29-x-y Jan 24, 2024
13 checks passed
@jkleinsc jkleinsc deleted the fix-fetch-headers-not-set branch January 24, 2024 16:32
Copy link

release-clerk bot commented Jan 24, 2024

Release Notes Persisted

Fixes an issue where Request objects did not correctly copy headers into fetches.

@trop
Copy link
Contributor

trop bot commented Jan 24, 2024

I have automatically backported this PR to "28-x-y", please check out #41103

@trop trop bot added in-flight/28-x-y merged/28-x-y PR was merged to the "28-x-y" branch. and removed target/28-x-y PR should also be added to the "28-x-y" branch. in-flight/28-x-y labels Jan 24, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
29-x-y backport This is a backport PR backport-check-skip Skip trop's backport validity checking merged/28-x-y PR was merged to the "28-x-y" branch. semver/patch backwards-compatible bug fixes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug]: Error expected when instantiating Request
3 participants