Skip to content

http: add 'sendingHeaders' event for response header manipulation#63996

Open
bjohansebas wants to merge 2 commits into
nodejs:mainfrom
bjohansebas:on-header-solution
Open

http: add 'sendingHeaders' event for response header manipulation#63996
bjohansebas wants to merge 2 commits into
nodejs:mainfrom
bjohansebas:on-header-solution

Conversation

@bjohansebas

@bjohansebas bjohansebas commented Jun 19, 2026

Copy link
Copy Markdown
Member

Okay, after so much time I can finally find time for this. Meanwhile, I’m still testing locally because building Node is quite heavy.

In summary, this serves as a replacement for on-headers, which is a package that does monkey-patching. The idea is to have an event so that middlewares like compression or express-session can use it to modify headers, and for now also, in the case of compression, to transform it into a gzip stream and send the data. However, we will still need two more events to fully remove the monkey-patching of res.write and res.end used in compression, express-session, and many other middlewares

cc: @blakeembrey

@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

Review requested:

  • @nodejs/http
  • @nodejs/net

@nodejs-github-bot nodejs-github-bot added http Issues or PRs related to the http subsystem. needs-ci PRs that need a full CI run. labels Jun 19, 2026
Comment thread doc/api/http.md
@bjohansebas bjohansebas requested a review from mcollina June 19, 2026 15:37

@bjohansebas bjohansebas left a comment

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Okay, continuing with this, Express would have an interesting situation here. While on-headers would run first and then this new event would run afterward, it could cause issues with things like compression, since it might overwrite work that was already done internally by on-headers, potentially breaking applications.

In other words, this would affect compression and other middleware. From the Express side that might not be a huge problem, but I'm not sure about the rest of the ecosystem. That's why I'm not sure whether this should be considered a semver-major change @nodejs/http @nodejs/tsc?

@Ethan-Arrowood Ethan-Arrowood left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Nice work. I'm +1 on landing something like this to eliminate the monkey-patching in on-headers package.

I think this should be semver-minor btw.

Comment thread lib/_http_server.js
Comment thread lib/_http_server.js Outdated
Comment thread test/parallel/test-http-sending-headers.js
Comment thread lib/internal/http2/compat.js Outdated
@bjohansebas

Copy link
Copy Markdown
Member Author

Okay, I added the safeguard you mentioned—good catch. Also, statusMessage is now assigned after the hook, so it now respects whatever the developer sets.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

This is to prevent regressions while making the changes

@codecov

codecov Bot commented Jul 14, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 94.33962% with 3 lines in your changes missing coverage. Please review.
✅ Project coverage is 90.25%. Comparing base (3ac95f2) to head (7b8a916).
⚠️ Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
lib/_http_server.js 92.10% 3 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main   #63996      +/-   ##
==========================================
+ Coverage   90.23%   90.25%   +0.01%     
==========================================
  Files         741      741              
  Lines      241692   241742      +50     
  Branches    45541    45558      +17     
==========================================
+ Hits       218097   218178      +81     
+ Misses      15113    15103      -10     
+ Partials     8482     8461      -21     
Files with missing lines Coverage Δ
lib/internal/http2/compat.js 97.25% <100.00%> (+0.03%) ⬆️
lib/_http_server.js 96.38% <92.10%> (-0.12%) ⬇️

... and 33 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

http Issues or PRs related to the http subsystem. needs-ci PRs that need a full CI run.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants