http2: add raw header array support to h2Session.request()

[pimterry]

This change allows you to call h2Session.request(['k1', 'v1', 'k2', 'v2']), just like in the HTTP/1 APIs.

Raw header support like this is useful for intermediaries, who generally already have headers in raw format already and would really like to preserve them exactly as received (without any unnecessary reordering or duplicate handling), and for anybody would wants to avoid the effort/perf to objectify headers unnecessarily.

I've added this just to request() for now, but I'd like to add support in the various other HTTP/2 methods - this is a first step to discuss any debatable points here up front.

There's a few notable details in here:

• This changes error behaviour in some cases: previously, passing some invalid HTTP header values to request() would destroy the entire underlying session with an error (and would not throw an error)). Not all though - e.g. key header::name would destroy the session with an error, but :unknown-pseudo-header, multiple :method headers or setting HTTP/2-invalid connection/keep-alive/etc headers would throw (all without destroying the session - just failing to create a new stream). AFAICT there is no good reason for this, and it's very awkward & surprising behaviour. I've removed it, and the parts of the tests (test/parallel/test-http2-invalidheaderfield.js) which checked it - this is arguably a breaking change in error handling, let me know what you think.
• This does not set stream[kSentHeaders], used by request.sentHeaders. That means that property remains undefined - this matches the behaviour of other APIs like http.request, which doesn't set kOutHeaders for HTTP/1.1 if you pass raw headers. That avoids some work and keeps things consistent, but it's a bit ugly. Opinions welcome - we could easily lazily instantiate sentHeaders instead to avoid any perf hit on the fast path anyway. We could even consider changing HTTP/1.1 to do the same!
• There's a fair bit of refactoring here, which notably drops the header name validation at the start of request() - that's because I think the same checks are all run again afterwards within buildNgHeaderString (previously called mapToHeaders) so I'm fairly sure this was adding nothing.

[nodejs-github-bot]

Review requested:

• @nodejs/http
• @nodejs/http2
• @nodejs/net

[codecov]

Codecov Report

Attention: Patch coverage is 89.91228% with 23 lines in your changes missing coverage. Please review.

Project coverage is 90.24%. Comparing base (33d8e03) to head (f20cf7d).
Report is 56 commits behind head on main.

Files with missing lines	Patch %	Lines
lib/internal/http2/util.js	90.44%	17 Missing ⚠️
lib/internal/http2/core.js	88.00%	6 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main   #57917      +/-   ##
==========================================
- Coverage   90.25%   90.24%   -0.01%     
==========================================
  Files         630      630              
  Lines      185880   186072     +192     
  Branches    36434    36461      +27     
==========================================
+ Hits       167757   167921     +164     
- Misses      11010    11034      +24     
- Partials     7113     7117       +4     

Files with missing lines	Coverage Δ
src/node_http_common.h	81.11% <ø> (ø)
lib/internal/http2/core.js	95.34% <88.00%> (-0.22%)	⬇️
lib/internal/http2/util.js	92.35% <90.44%> (-0.65%)	⬇️

... and 27 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.

[mcollina]

lgtm

[nodejs-github-bot]

CI: https://ci.nodejs.org/job/node-test-pull-request/66415/

[pimterry]

This has a passing CI & one approval, and it's been open for more than 7 days, so it's now mergable! It is a bit fiddly & sensitive, and I want to check for any disagreement before I open more PRs like this for the other header API methods, so I'll leave it a few more days in case anybody else in @nodejs/http2 has opinions - if not, I'll land it sometime next week.

[jasnell]

Landed in 4cd8e19