Skip to content

node:http: rewrite the client on net/tls + llhttp (Node http suite: ~55% → 82.3%, +182 vendored tests, client proxy support) - #31587

Merged
cirospaciari merged 13 commits into
mainfrom
claude/http-client-node-port
Jun 17, 2026
Merged

cirospaciari merged 13 commits into
mainfrom
claude/http-client-node-port

Conversation

@cirospaciari

@cirospaciari cirospaciari commented May 29, 2026 •

Copy link
Copy Markdown
Member

What does this PR do?

Re-implements the node:http client as a port of Node.js's lib/_http_client.js, replacing the previous fetch-based ClientRequest. The client now runs on node:net / node:tls sockets, the llhttp HTTPParser binding (process.binding("http_parser")), and a real Agent socket pool, matching the upstream implementation as closely as possible (each ported file links to the upstream source it follows). It also includes the server-side fixes that were needed for the Node.js http test suite to pass against our server.

Stacked on claude/port-node-net-tls-tests-2 (uses the net/tls work as its base).

Test results

Measured on Node.js's complete upstream test-http-* + test-https-* suites (parallel + sequential, 447 tests, local run against a debug build):

fetch-based client (before) this PR
upstream test-http-* (402 tests) ~55% 331/402 (82.3%)
upstream test-https-* (45 vendored) — 45/45 (100%)
upstream client-proxy suite (62 tests) — 43/62 (69%)
combined — 419/509 (82.3%)

182 newly passing upstream test files are vendored into test/js/node/test/ and enforced by CI (116 test-http-*, 23 test-https-*, 43 client-proxy). Two vendored tests that had been adapted to the old client's behavior are restored to their verbatim upstream versions. The port follows the latest Node.js release (each ported file links to the upstream source file it follows), not unreleased main behavior.

Client (ported from Node.js)

  • ClientRequest goes through Agent.addRequest/createConnection, OutgoingMessage._storeHeader/_writeRaw and the llhttp RESPONSE parser: upgrade/CONNECT, 1xx information/continue events, keep-alive socket reuse and pooling, AbortSignal, timeouts, createConnection, maxHeadersCount, insecureHTTPParser, joinDuplicateHeaders, setDefaultHeaders, auth, and proxy header rewriting follow upstream.
  • OutgoingMessage, IncomingMessage, _http_agent and _http_common are now the upstream implementations (kOutHeaders storage, outputData buffering, chunked encoding, trailers, headersDistinct/trailersDistinct, kSkipPendingData, etc.). The native Bun.serve-based server keeps working through explicit ServerResponse/server-side construction paths.
  • https.Agent gets the upstream createConnection over tls.connect with TLS session caching and the upstream cache key; NODE_TLS_REJECT_UNAUTHORIZED is read per connection like Node.js, so setting it at runtime works.
  • Client proxy support: NODE_USE_ENV_PROXY wires HTTP_PROXY/HTTPS_PROXY/NO_PROXY into the global agents, HTTPS requests through a proxy establish a CONNECT tunnel (establishTunnel, proxy-authorization, ERR_PROXY_TUNNEL with the proxy status code, tunnel timeouts), and http.setGlobalProxyFromEnv() swaps the global agents and returns a restore function.
  • http.globalAgent assignment now changes the agent used by http.request, and overriding https.globalAgent works the same way.
  • node:dns lookup errors carry the looked-up hostname (err.hostname, and in the message) like Node.js's DNSException.

Server fixes needed by the client tests

  • Request headers are built from rawHeaders with Node's duplicate-handling rules (joining, cookie/set-cookie special cases, joinDuplicateHeaders) and a null prototype; native-server requests expose req.client.
  • ServerResponse stores headers in Node's kOutHeaders format and renders them to the native writeHead as a flat name/value array, so repeated header names (multiple Set-Cookie, duplicate Content-Length, …) are written as separate lines, header names keep the user's casing, getHeader returns arrays as set, and mutating headers after writeHead throws ERR_HTTP_HEADERS_SENT. writeHead() locks the status line like Node.js. The auto-generated Date/Connection/Keep-Alive defaults never pollute getHeaders() introspection.
  • Default Date, Connection: keep-alive|close and Keep-Alive: timeout=N[, max=M] response headers honoring res.sendDate, removeHeader(), server.keepAliveTimeout (5s, matching the latest Node.js release) and server.maxRequestsPerSocket; HTTP/1.0 requests are answered with Connection: close since the transport always closes them.
  • Node.js stream semantics on ServerResponse and the server socket: destroy(err) records res.errored and forwards the error to the socket, socket errors don't crash as unhandled, a response still attached to a dying connection emits 'close', res.writable stays true after finish, and write() accounts bytes against the socket's platform-dependent writableHighWaterMark so write loops and oversized writes report backpressure with correct 'drain' timing.
  • Upgrade requests follow Node.js: detected only with both Upgrade and Connection: upgrade headers, gated on the new shouldUpgradeCallback server option (default: an 'upgrade' listener exists), the listener gets the raw socket plus any bytes after the request head, and the socket is destroyed when the upgrade is accepted but unhandled.
  • Informational responses work: writeEarlyHints/writeProcessing reach the wire and ServerResponse.writeInformation is implemented; raw writes to the request socket are no longer dropped outside CONNECT streaming mode.
  • The connection is released for the next keep-alive request even when the request body stream was torn down and req.socket was cleared (this is what Next.js route handlers do — fixes "Socket already assigned" / "failed to pipe response" with Next.js).
  • The CONNECT socket is handed to the 'connect' listener with no internal listeners attached, like Node.js.
  • HTTP/1.0 requests report httpVersion/httpVersionMajor/httpVersionMinor correctly.
  • Native: the parser binding decodes header names/values/status as latin1 like Node; uWS no longer auto-writes Content-Length when an explicit Transfer-Encoding header was set, frames one-shot res.end(body) responses as chunked when an explicit Transfer-Encoding header is present, and the native writeHead accepts the flat pairs array; net.Socket tolerates user-supplied handles without resume/pause (needed by an upstream agent test).

Test updates

A few existing tests encoded behavior of the old fetch-based client and were updated to the Node.js-verified behavior (event order, req.socket being null until the socket event, explicit Content-Length + Transfer-Encoding both being sent, AbortSignal surfacing an AbortError error event, https-proxy-agent constructor options only applying to the proxy hop). Two unix-socket abort tests were dropped from the vendored set because they cannot listen on named pipes on Windows CI.

Also included (server features the suite needed)

server.getConnections(), server.maxHeadersCount, the optimizeEmptyRequests and shouldUpgradeCallback server options, standalone ServerResponse + assignSocket(writable) output, Node.js's connections-checking interval lifecycle, 204/304 and close-delimited response framing (native), close-on-finish connection semantics, 'http' performance entries (HttpClient/HttpRequest), the NODE_DEBUG=http sensitive-data warning, and a child_process.spawnSync fix so runtime process.env mutations reach children.

Known remaining gaps (follow-ups)

  • The fetch() global-dispatcher half of setGlobalProxyFromEnv (no undici dispatcher to swap), and a few proxy error-path tests that assert Node.js's exact uncaught-error formatting.

  • Original header-name casing in req.rawHeaders (the uWS parser lowercases field names in place) and request/response trailers on the server.

  • Server headersTimeout/requestTimeout (408), accept-time 'connection' events, raw socket I/O after 'upgrade', and flushing already-written response data when res.socket.destroy() is called immediately after a write.

  • The native server rejects pipelined requests separated by stray CRLFs and requests that carry both Content-Length and Transfer-Encoding, where Node.js's parser is more lenient/explicit (affects test-http-keep-alive-max-requests).

  • A handful of upstream tests require Node internals (--expose-internals, V8 natives, node:domain async context) and are not portable.

How did you verify your code works?

  • Ran the full vendored test/js/node/test/parallel + sequential test-http-* and test-https-* suites, the hand-written suites in test/js/node/http/, test/js/bun/test/parallel/test-http-*, test/js/bun/http/proxy.test.ts, and the Next.js (next-auth) integration test against a debug build; compared raw wire output against Node.js for the affected behaviors (keep-alive headers, duplicate header lines, HEAD + Transfer-Encoding, latin1 headers, informational responses, explicit Content-Length + Transfer-Encoding, client timeout and abort semantics, event ordering, backpressure/drain accounting).
  • The newly vendored tests were each verified to pass repeatedly before being added.

@robobun

robobun commented May 29, 2026 •

Copy link
Copy Markdown
Collaborator
Updated 12:47 PM PT - Jun 17th, 2026

❌ @cirospaciari, your commit c016beb has 2 failures in Build #63152 (All Failures):


🧪   To try this PR locally:

bunx bun-pr 31587

That installs a local version of the PR into your bun-31587 executable, so you can run:

bun-31587 --bun

Comment thread src/js/node/_http_server.ts
Comment thread src/js/node/_http_server.ts Outdated
Comment thread test/regression/issue/27061.test.ts Outdated
Comment thread src/js/node/_http_server.ts Outdated
Comment thread packages/bun-uws/src/HttpResponse.h
@cirospaciari
cirospaciari force-pushed the claude/port-node-net-tls-tests-2 branch from 83618ad to 036fff3 Compare May 30, 2026 21:33
@cirospaciari
cirospaciari force-pushed the claude/http-client-node-port branch from f9114ea to 5df4936 Compare May 30, 2026 23:46
Comment thread src/js/node/https.ts
Comment thread src/js/node/_http_incoming.ts Outdated
Comment thread src/js/node/_http_server.ts
Comment thread src/js/node/_http_server.ts
Comment thread src/js/node/_http_server.ts Outdated
@cirospaciari cirospaciari changed the title node:http: rewrite the client on net/tls sockets and the llhttp parser node:http: rewrite the client on net/tls + llhttp (Node http suite: ~55% → 80.5%, +124 vendored tests) Jun 1, 2026
Comment thread src/js/node/_http_server.ts
Comment thread src/js/node/_http_server.ts
@cirospaciari cirospaciari changed the title node:http: rewrite the client on net/tls + llhttp (Node http suite: ~55% → 80.5%, +124 vendored tests) node:http: rewrite the client on net/tls + llhttp (Node http suite: ~55% → 82.8%, +135 vendored tests) Jun 1, 2026
Comment thread packages/bun-uws/src/HttpResponseData.h
Comment thread src/js/node/_http_server.ts Outdated
Comment thread src/js/node/_http_server.ts Outdated
Comment thread test/js/node/test/parallel/test-http-unix-socket-keep-alive.js
Comment thread src/js/node/_http_server.ts Outdated
@cirospaciari
cirospaciari force-pushed the claude/port-node-net-tls-tests-2 branch from b4b0da6 to 3598acd Compare June 2, 2026 00:44
@cirospaciari
cirospaciari force-pushed the claude/http-client-node-port branch from 635ac06 to 7a0dca3 Compare June 2, 2026 16:53
Comment thread src/js/node/_http_client.ts Outdated
Comment thread src/which/lib.rs Outdated
Comment thread packages/bun-uws/src/HttpParser.h
Comment thread packages/bun-uws/src/HttpResponse.h
Comment thread src/js/node/_http_server.ts Outdated
Comment thread src/which/lib.rs Outdated
Comment thread packages/bun-uws/src/HttpParser.h
@cirospaciari cirospaciari changed the title node:http: rewrite the client on net/tls + llhttp (Node http suite: ~55% → 82.8%, +135 vendored tests) node:http: rewrite the client on net/tls + llhttp (Node http suite: ~55% → 82.3%, +182 vendored tests, client proxy support) Jun 2, 2026
Comment thread src/js/node/https.ts
Comment thread src/js/node/https.ts
Comment thread src/js/node/_http_incoming.ts
Comment thread src/js/node/_http_incoming.ts Outdated
@robobun

robobun commented Jun 4, 2026

Copy link
Copy Markdown
Collaborator

This PR fixes #31795 (tunnel.httpsOverHttp() being ignored) — the ported client goes through Agent.addRequest/onSocket, which is the exact path the tunnel package uses for its CONNECT tunnel. I closed #31798 (a narrower targeted fix) in favor of this.

One gap: I didn't find an explicit tunnel-package regression test on this branch. If useful, the two tests I wrote for #31795 exercise the exact issue scenario and could be vendored here as a guard — a 502 CONNECT failure surfacing tunneling socket could not be established, statusCode=502, and a full httpOverHttp tunnel success path asserting the proxy's connect handler actually ran. Happy to open a small PR adding just those against this branch if you want.

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.

2 participants