Skip to content

feat: add IP prioritization hints for HTTP/1.1 and HTTP/2#4831

Merged
mcollina merged 1 commit intonodejs:mainfrom
amyssnippet:main
Mar 8, 2026
Merged

feat: add IP prioritization hints for HTTP/1.1 and HTTP/2#4831
mcollina merged 1 commit intonodejs:mainfrom
amyssnippet:main

Conversation

@amyssnippet
Copy link
Contributor

This relates to...

IP Prioritization support for HTTP/1.1 and HTTP/2 requests.

Rationale

This PR introduces the ability to set IP-level prioritization hints for requests. This is particularly useful for applications running in environments that utilize Quality of Service (QoS) markings or differentiated services (DiffServ) at the network layer, allowing for fine-grained control over packet priority directly from the HTTP client.

Changes

  • lib/core/request.js: Updated the Request constructor to accept a hints option (validated as a number).
  • lib/dispatcher/client-h1.js: Updated the HTTP/1.1 dispatcher to apply the hints value to the socket using socket.setPriority() if available.
  • lib/core/connect.js: Verified that hints are passed through buildConnector to net.connect options.
  • types/dispatcher.d.ts & types/connector.d.ts: Added TypeScript definitions for the hints option.
  • test/ip-prioritization.js: Added new tests to verify:
    • socket.setPriority is called with the correct value for HTTP/1.1.
    • hints are correctly passed to the connection options (net.connect) for HTTP/2.

Features

  • Added support for hints in DispatchOptions to set IP prioritization.
  • Supported for both HTTP/1.1 (per-request via socket.setPriority) and HTTP/2 (per-connection via connect options).

Bug Fixes

N/A

Breaking Changes and Deprecations

N/A

Status

@amyssnippet
Copy link
Contributor Author

@ronag , after the merge of the nodejs/node#61503 , i saw this reference and i thought to contribute here too.
kindly review the changes

@metcoder95
Copy link
Member

Is not the same API as the one added in the upstream PR

Copy link
Contributor

@Uzlopak Uzlopak left a comment

Choose a reason for hiding this comment

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

hints is imho a too generic term. We should need a more specific term.
If I understand you correctly you want to pass the value down as typeOfService

So i guess hints has to be renamed to typeOfService and setPriority to setTypeOfService ?

Also typeOfService has to be a number between 0 and 255?

I

@amyssnippet
Copy link
Contributor Author

hints is imho a too generic term. We should need a more specific term. If I understand you correctly you want to pass the value down as typeOfService

So i guess hints has to be renamed to typeOfService and setPriority to setTypeOfService ?

Also typeOfService has to be a number between 0 and 255?

I

i renamed all of them

@amyssnippet amyssnippet requested review from Uzlopak and ronag March 4, 2026 03:54
@codecov-commenter
Copy link

codecov-commenter commented Mar 4, 2026

Codecov Report

❌ Patch coverage is 84.61538% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 93.15%. Comparing base (0a23610) to head (dd508fd).
⚠️ Report is 35 commits behind head on main.

Files with missing lines Patch % Lines
lib/core/request.js 75.00% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #4831      +/-   ##
==========================================
- Coverage   93.19%   93.15%   -0.04%     
==========================================
  Files         109      109              
  Lines       34222    34254      +32     
==========================================
+ Hits        31893    31911      +18     
- Misses       2329     2343      +14     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 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.

@amyssnippet
Copy link
Contributor Author

the test failure is flaky, and i have a question, is this pr will be automerged when all ci checks passes? is yes then what about this flaky test? what should i do next, pls guide!

@amyssnippet
Copy link
Contributor Author

@ronag @metcoder95

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 merged commit ab3b0f2 into nodejs:main Mar 8, 2026
36 of 37 checks passed
@github-actions github-actions bot mentioned this pull request Mar 12, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants