feat(node:http2) Implement HTTP2 server support - #14286
Merged
Merged
Conversation
Collaborator
cirospaciari
force-pushed
the
ciro/http2-server
branch
4 times, most recently
from
October 8, 2024 18:45
13d248c to
4f6b936
Compare
cirospaciari
force-pushed
the
ciro/http2-server
branch
from
October 10, 2024 18:34
f30f9e5 to
d65f356
Compare
cirospaciari
changed the title
cirospaciari
force-pushed
the
ciro/http2-server
branch
from
October 11, 2024 21:48
0ae142a to
4860442
Compare
cirospaciari
marked this pull request as ready for review
October 11, 2024 21:48
cirospaciari
changed the title
cirospaciari
force-pushed
the
ciro/http2-server
branch
from
October 12, 2024 05:13
68957f2 to
0182b18
Compare
|
please merge this. waiting with fingers crossed 🤞 |
|
I tested this PR against my demo repository to reproduce #14249 last Friday (oct 11) using the prebuilt binary with These are the two binaries I've tested: -rwxr-xr-x 1 97M oct 14 15:44 bun-b30e238d9be315441a07e3ebab8c9e6f6de55037-pr14286
-rwxr-xr-x 1 96M oct 11 07:55 bun-e16c1fcc052a188724fb3a019bada915077c4fa3-pr14286 |
cirospaciari
force-pushed
the
ciro/http2-server
branch
from
October 14, 2024 20:56
ed19c49 to
ca7dd0e
Compare
cirospaciari
force-pushed
the
ciro/http2-server
branch
from
October 15, 2024 22:42
1ea33f2 to
b1582f4
Compare
This was referenced Oct 16, 2024
|
@cirospaciari: everything seems to be working well when I am using |
|
I still get this on firebase
built with: when testing locally with jest (no bun build) everything works fine. |
|
Everything works fine with |
robobun
pushed a commit
that referenced
this pull request
Nov 24, 2025
The tests for TXT service config resolution were using `grpctest.kleinsch.com`, which is no longer available. These tests were originally skipped when added in #14286, but were accidentally un-skipped in #20051. This restores the skip status to match upstream grpc-node, and fixes the flaky "should not keep repeating failed resolutions" test. ## To re-enable these tests in the future Bun could set up its own DNS TXT record. According to the gRPC spec (A2), the record should be: - DNS name: `_grpc_config.grpctest.bun.sh` (for server `grpctest.bun.sh`) - Record type: TXT - Value: `grpc_config=[{"serviceConfig":{"loadBalancingPolicy":"round_robin","methodConfig":[{"name":[{"service":"MyService","method":"Foo"}],"waitForReady":true}]}}]` The server also needs an A record pointing to any valid IP. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
1 task
cirospaciari
pushed a commit
that referenced
this pull request
Nov 24, 2025
…25039) ## Summary - Skip 2 tests that use `grpctest.kleinsch.com` (domain no longer exists) - Fix flaky "should not keep repeating failed resolutions" test These tests were originally skipped when added in #14286, but were accidentally un-skipped in #20051. This restores them to match upstream grpc-node. ## To re-enable these tests in the future Bun could set up its own DNS TXT record at `*.bun.sh`. According to the [gRPC A2 spec](https://github.com/grpc/proposal/blob/master/A2-service-configs-in-dns.md): **DNS Setup needed:** 1. A record: `grpctest.bun.sh` → any valid IP (e.g., `127.0.0.1`) 2. TXT record: `_grpc_config.grpctest.bun.sh` with value: ``` grpc_config=[{"serviceConfig":{"loadBalancingPolicy":"round_robin","methodConfig":[{"name":[{"service":"MyService","method":"Foo"}],"waitForReady":true}]}}] ``` Then update the tests to use `grpctest.bun.sh` instead. ## Test plan - [x] `bun bd test test/js/third_party/grpc-js/test-resolver.test.ts` passes (20 pass, 3 skip, 1 todo, 0 fail) 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Bot <claude-bot@bun.sh> Co-authored-by: Claude <noreply@anthropic.com>
Jarred-Sumner
added a commit
that referenced
this pull request
Aug 18, 2026
…ockets, and the builtin-name tables (#38900) Removes 775 lines that nothing references (8 lines of signature, import and comment adjustments added), across node:http2, the HTTP/2 frame parser, the JSC/WebCore bindings, uSockets, four Rust crates, the builtin-name tables and scripts/. No behavior change. A 153-line source lint pins every removed symbol. ### Problem - `src/js/node/http2.ts` binds every entry of `constants` as a local (`const { ... } = constants;`); 178 of the 240 bindings have been unused since the block was added in #14286 (every call site reads `constants.X`). `const Socket = net.Socket` and the `#url` / `#isServer` fields of `ServerHttp2Session` are never read either (`tsc --noUnusedLocals`, re-checked by grep). - `_nativeAssertSettings` (`$newRustFunction("h2_frame_parser.rs", "jsAssertSettings")`) has been unused since #28074 gave http2.ts its own `assertSettings()`. That made `js_assert_settings` in `src/runtime/api/bun/h2_frame_parser.rs` (139 lines) and its facade re-export in `src/runtime/api.rs` unreachable: the js2native codegen was the only thing that named it. The three `BUN_DECLARE_HOST_FUNCTION(BUN__HTTP2_*)` lines in `ZigGlobalObject.cpp` are declarations of the Zig-era exports for the same helpers and have no definition anywhere. - `InspectorHTTPServerAgent::{startListening, stopListening, getRequestBody, getResponseBody}` are TODO stubs that nothing can dispatch to: the generated `HTTPServerBackendDispatcherHandler` in the pinned WebKit only declares `enable()` / `disable()`, so these `virtual ... final` methods override nothing and have no callers. - Declaration-only leftovers with no definition anywhere: `Bun__DNSResolver__new` / `Bun__DNSResolver__cancel` (`BunObject.cpp`), `jsBufferConstructorFunction_isBuffer` (`JSBuffer.cpp`, `Buffer.isBuffer` is a JS builtin), `getStringOption` (`CryptoUtil.h`). Plus the `IsIDLEnumeration` helper template (`IDLTypes.h`) and the `globalBuiltinFunction` macro (`ZigGlobalObject.cpp`), each with zero uses. - uSockets: `us_internal_ssl_sni_userdata` and `us_internal_ssl_handshake_abort` were added in #29932 and never called (`us_socket_server_name_userdata`, which the first one wrapped, stays). `us_cleanup_security_framework` (darwin) had no caller; the loader's failure paths are the only thing that ever frees a `SecurityFramework`, and they still do. The `SecTrustSettingsResult` typedef next to it was unused too. - Rust items no crate reaches (hawk `dead_public`, intersected over linux-gnu, linux-musl, android, freebsd, darwin and windows-msvc, then re-checked by hand): `ExprData::is_e_string`, `ArrayHashMap::get_adapted`, the debug-only `Behavior::eq`, and `js_parser::FunctionKind`, whose `Stmt` variant was never constructed. `validate_function_name` only ever ran for function expressions, so the parameter is gone and the one call site passes nothing; the error text already said "function expression". - Builtin-name tables: `Loader`, `byobRequest`, `controller`, `post`, `resume`, `started`, `state`, `textDecoder` and `view` in `BunBuiltinNames.h` have no `$name` / `@name` use in any builtin and no `namePublicName()` / `namePrivateName()` use in C++ (each entry costs two identifiers at VM startup). Their `builtins.d.ts` declarations go with them, as do the six `$stream*` `--define`s in `replacements.ts` (no builtin reads them; their only trace was the `.d.ts`), the `"Loader"` entry of `globalsToPrefix` (the only bare `Loader` in src/js is inside a block comment), and a duplicated `"Buffer"` entry. - `scripts/find-dead-exports.ts` (303 lines, #31254) was a textual approximation of the analysis `tools/hawk/` + `hawk.toml` now do (#36184); nothing references it. ### Fix - Deletes the items above; the only additions are the shorter `validate_function_name` signature, one comment in `semver/lib.rs` that named `get_adapted` (now names the surviving `get_index_adapted`), and the re-export line in `api.rs`. - Removing the `$newRustFunction` call is what makes the Rust function removable: `generate-js2native.ts` emits the `crate::api::bun::h2_frame_parser::js_assert_settings` thunk only for calls it finds in src/js, so after this change nothing generated names it either. `bundle-functions.ts` regenerates `BunBuiltinNames+extras.h` from the names builtins actually use, so a builtin-name entry that were still needed would be re-added by the build rather than break it; none was. - Verified: `bun bd` builds; `bun run rust:check-all` passes on all ten target triples; `cargo fmt --check` and prettier are clean. `bun bd test` passes on `test/js/node/http2/node-http2.test.js` (357), `test/js/node/http2/node-http2-continuation.test.ts`, `test/js/node/buffer.test.js`, `test/js/node/crypto/node-crypto.test.js`, `test/js/node/tls/node-tls-connect.test.ts`, `test/js/web/streams/streams.test.js` and `test/bundler/bundler_minify.test.ts`; `test/js/node/tls/node-tls-server.test.ts` passes 69/70, the remaining one (`SNICallback runs even when the requested servername matches the bind hostname`) fails identically with the released binary in this container (it binds `localhost` and connects to 127.0.0.1). The four function-name diagnostics `validate_function_name` emits are unchanged between the released binary and this build for both statement and expression positions. `bun test test/internal/source-lints/` passes (158 tests); the new `dead-symbols-http2-settings-inspector-stubs.test.ts` fails on main and passes here. - Cross-checked against the 22 open dead-code PRs at the symbol level (their diffs, not just file lists): nothing here is removed by any of them. Two hunks are adjacent to open ones and will need a trivial rebase on whichever side lands second: the `BUN__HTTP2_*` lines sit right under the `Bun__NodeUtil__jsParseArgs` line #37788 removes, and the `InspectorHTTPServerAgent` hunks are a few lines above the event dispatchers #37454 removes. One item found dead here, `JSValkeyClient::close_subscription_ctx`, is the renamed form of the `SubscriptionCtx::close` that #38439 already removes, so it was left to that PR. ### Background - js2native: `$rust("file.rs", "name")` / `$newRustFunction(...)` calls in src/js are collected at build time by `src/codegen/generate-js2native.ts`, which emits one thunk per call into `generated_js2native.rs` / `GeneratedJS2Native.h`. A Rust host function is therefore reachable only while some builtin names it; the `api.rs` facade module exists so the generated thunks have a stable path to call. - `BunBuiltinNames.h` is the list of private identifiers (`@name` in builtin JS, `builtinNames.namePrivateName()` in C++) that `BunBuiltinNames` interns when a VM starts. `bundle-functions.ts` diffs the names builtins use against this list and writes the difference to `BunBuiltinNames+extras.h`, which is why removing an entry can only ever remove startup work, never break a builtin. - Inspector agents: the WebKit inspector protocol generator turns each domain's JSON into a `*BackendDispatcherHandler` interface; a command reaches an agent only through that interface. The HTTPServer domain bun ships in its WebKit defines `enable` and `disable`, so methods an agent declares beyond those are unreachable by construction. - hawk (`tools/hawk/README.md`) is the workspace-wide reachability analysis the repo uses for `pub` items, since rustc's `dead_code` treats every `pub` item of a library crate as a root. Its per-target reports were intersected so that anything live under some `cfg` on any shipped platform was kept; the repr(C) FFI struct fields and flag/errno table entries it also reports were left alone on purpose, as in #38703. <details> <summary>Found dead but left alone (follow-ups, not in this diff)</summary> - `bun_sys::ErrorCase::LeakFdOnFail` is never constructed (every `make_lib_uv_owned_for_syscall` caller passes `CloseOnFail`), but removing it means dropping the parameter at 13 call sites, several of them in files open PRs are editing. - `libusockets.h` still declares `us_udp_socket_receive`, `us_udp_buffer_set_packet_payload`, `us_create_udp_packet_buffer` and `us_udp_socket_bind` with no definitions, and `udp.c` carries a commented-out `us_udp_packet_buffer_ecn`; both spots are adjacent to hunks in #37454. - `NapiEnv::currentFinalizer()` became unused in #37075 a week ago; left for that work to settle, as #37454 did with its siblings. - `replacements.ts` also emits a `$ImportKindLabelToId` define nothing reads, but it comes out of the same loop as three live tables. - `ws.js` `WebSocket.prototype.setSocket` (a throwing stub) and `async_hooks` `AsyncResource.emitBefore/emitAfter` have no in-tree users but are on exported classes. - `scripts/packer/build-image.pkr.hcl` and `scripts/trace.sh` have no invoker, but they are CI infrastructure rather than code; flagged for whoever owns those. - Everything else this pass turned up (about 270 C++ symbols and about 100 Rust items) is already deleted by one of the open dead-code PRs, and the roughly 400 never-read Rust fields hawk reports are all repr(C) mirrors of C structs. </details> --------- Co-authored-by: Jarred Sumner <jarred@jarredsumner.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
8 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What does this PR do?
Major Client compatibility fixes + Server implementation
Fix: #8823
Fix: #14249
Fix: #8549
Fix: #9228
Notes
Push (not supported in most browsers):
HTTP1 compatibility aka ALPN negotiation (blocked on http module implementation):
WebSocket Proxing using RFC8441 (disable by default on node.js);
ALTSVC using RFC7838:
In-Progress:
Progress:
Class: ServerHttp2Session
Class: ClientHttp2Session
Class: Http2Stream
Class: ClientHttp2Stream
Class: ServerHttp2Stream
Class: Http2Server
Class: Http2SecureServer
Module:
Compatibility API
Class: http2.Http2ServerRequest
Class: http2.Http2ServerResponse
How did you verify your code works?