nodejs
http: add setDefaultHeaders option to http.request
When sending an HTTP request, for some use cases you want full control of the request headers: controlling the raw format directly and setting only the headers provided, with no other headers automatically included whatsoever.
That is not currently possible, because:
- The only way to disable various default headers like Connection & Transfer-Encoding is to call
req.removeHeaderfor each of them individually on a request instance, before the headers are sent. Otherwise these are set automatically, even if you pass a raw header array. - The only way to set raw headers (i.e. passing a header array instead of an object, to directly control order, casing & duplicate handling etc) is by passing them upfront in the
http.request()method, and when you do so the headers are passed to_storeHeaderand serialized into_headerimmediately, meaning you can't callremoveHeaderafterwards.
That means these two use cases (removing default headers & setting raw header formats) are mutually exclusive, which isn't great since they're closely related.
This PR fixes that by adding a setDefaultHeaders option to http.request, defaulting to true. If you pass setDefaultHeaders: false then you take responsibility for providing all required headers and Node gets out of the way and gives you full control.
With this, you can now control all headers, in raw format, with:
http.request({
/* ... */
setDefaultHeaders: false,
headers: [/* raw header array */]
})
Alternative ideas I've avoided, but might be interesting:
- Adding separate
setContentLength,setTransferEncodingetc parameters, analogous to the existingsetHostoption. Results in too many options, and I think in the case where this is necessary you almost always want to control all headers. In fact, I think if this PR is merged we could consider deprecatingsetHostentirely, and suggesting users migrate to this option instead (I expect mostsetHost: falsecases will want this too) but I haven't looked into that for now, we can wait and see how this is used instead. - Extending
setHeadersto allow setting raw headers after initial request construction, to handle this use case with justremoveHeader+setHeaderscalls. This would require that eithersetHeadersimmediately serialize the raw headers to_headersin this case (blocking any further changes like an implicit flushHeaders, but only in the raw header argument case, with potentially weird differences between setHeaders behaviour for requests vs responses), or some fairly major refactoring to change all internal HTTP header representation to support incrementally adding raw headers (all current raw header APIs -writeHeadandhttp.request- always serialize & fix the headers immediately). The former is quite an awkward API, the latter is messy and risky, and I'd rather not. - Adding a new
request.writeHeadersmethod analogous toresponse.writeHead. This works, but it's yet another headers API with subtly different behaviour that would really only exist for this one use case. - Removing implicit headers automatically if you provide a full set of raw headers in
http.request- this seems sensible to me as an API, but it's a huge breaking change that isn't really practical imo.
Codecov Report
All modified and coverable lines are covered by tests :white_check_mark:
Project coverage is 88.53%. Comparing base (
d5d1e80) to head (ddfdc0f). Report is 311 commits behind head on main.
Additional details and impacted files
@@ Coverage Diff @@
## main #56112 +/- ##
==========================================
+ Coverage 88.00% 88.53% +0.52%
==========================================
Files 656 657 +1
Lines 189002 189864 +862
Branches 36003 36447 +444
==========================================
+ Hits 166335 168091 +1756
+ Misses 15838 14981 -857
+ Partials 6829 6792 -37
| Files with missing lines | Coverage Δ | |
|---|---|---|
| lib/_http_client.js | 97.99% <100.00%> (+0.01%) |
:arrow_up: |
![codecov[bot] avatar](https://avatars.githubusercontent.com/in/254?v=4 "Published by a pub.dev verified publisher"){kind=small}
I'd love a couple of @nodejs/http reviews here when anybody has a minute. Tests all passing in CI already.