Skip to content

fix(node:http,https): Server() callable alias + sync bind for address().port (#2132) - #2197

Merged
proggeramlug merged 2 commits into
mainfrom
fix-2132-server-alias-and-bind
May 28, 2026
Merged

fix(node:http,https): Server() callable alias + sync bind for address().port (#2132)#2197
proggeramlug merged 2 commits into
mainfrom
fix-2132-server-alias-and-bind

Conversation

@proggeramlug

Copy link
Copy Markdown
Contributor

Summary

Two related fixes for the runtime-fail cluster in #2132 (stacked after PR #2184):

  1. http.Server(handler) / https.Server(opts, handler) are now callable. Node treats Server and createServer as interchangeable — the same constructor backs both, with or without new. Many Node test fixtures use the callable form. Without a dispatch row, Perry returned undefined and the next .listen(...) crashed with Cannot read properties of undefined (reading 'listen'). Adds Server rows to the codegen native table aliased to js_node_http_create_server / js_node_https_create_server.

  2. server.address().port returned the requested port. The listen() FFI wrote bound_port = port (e.g. 0) and then spawned an async task to actually TcpListener::bind(...).await. The listen(port, cb) callback fired before the async bind resolved, so server.address().port inside the callback was the pre-OS-assignment port — 0 for listen(0, cb). Downstream http.get({ port: 0, ... }) then couldn't reach the server. Bind synchronously with std::net::TcpListener::bind, capture local_addr().port(), write the actual port into bound_port, then hand the listener to tokio::net::TcpListener::from_std for the async accept loop.

Same fix mirrored across http, https, and http2 server FFIs. The synchronous bind also matches Node's observable timing: the listen() callback runs after the socket is actually bound.

Verified

  • cargo test --release -p perry-ext-http-server --lib: 8 passed.
  • cargo fmt --all -- --check: clean.
  • Direct re-run of the 4 cited "Cannot read 'listen'" diff tests against local radar staging:
    • test-https-agent.js: was runtime-fail, now pass.
    • test-https-agent-servername.js: was runtime-fail, now pass.
    • test-https-agent-additional-options.js: was runtime-fail, now pass.
    • test-http-abort-client.js: moves past listen crash to surface a distinct downstream gap (serverRes.on undefined) — separate follow-up.

Scope

Second cluster from #2132 after the PEM-Buffer cluster in #2184. Remaining buckets surfaced by the radar (Agent.getName / addRequest missing methods, (no output) silent exits, UnsupportedCertVersion cert-fixture incompat) will be follow-up PRs.

Test plan

  • cargo test --release -p perry-ext-http-server --lib
  • cargo fmt --all -- --check
  • Local node-core radar comparison on the 4 cited diff tests
  • CI green

proggeramlug pushed a commit that referenced this pull request May 28, 2026
CI's `every_dispatch_entry_has_manifest_counterpart` drift test on
PR #2197 caught that the new `Server` rows added to
`NATIVE_MODULE_TABLE` had no corresponding entries in
`API_MANIFEST` — drift here would make the #463 unimplemented-API
gate error on a real implementation. Add `method("http", "Server")`
and `method("https", "Server")` next to the existing `createServer`
rows.

Regenerate `docs/api/perry.d.ts` and `docs/src/api/reference.md` for
the api-docs-drift gate. The diff is exactly the two new `export
function Server(...)` lines under the `http` and `https` modules.
Ralph Küpper added 2 commits May 28, 2026 14:55
…().port (#2132)

Two related fixes for the runtime-fail cluster in #2132:

1. **`http.Server(handler)` / `https.Server(opts, handler)` as
   callable constructors.** Node treats `Server` and `createServer`
   as interchangeable — the same C++ constructor backs both, with or
   without `new`. Many Node test fixtures (`test-http-abort-client`,
   `test-https-agent`, `test-https-agent-servername`,
   `test-https-agent-additional-options`) use the `Server(...)` form
   directly. Without a dispatch row, Perry returned undefined and
   the next `.listen(...)` crashed with `Cannot read properties of
   undefined (reading 'listen')`. Add `Server` rows to the codegen
   native table aliased to `js_node_http_create_server` /
   `js_node_https_create_server`.

2. **`server.address().port` returned the *requested* port.** The
   listen() FFI wrote `bound_port = port` (e.g. 0) and then spawned
   an async task to actually `TcpListener::bind(...).await`. The
   `listen(port, cb)` callback fired *before* the async bind
   resolved, so `server.address().port` inside the callback was the
   pre-OS-assignment port — `0` for `listen(0, cb)`. Downstream
   `http.get({ port: 0, ... })` connections then couldn't reach the
   server. Bind synchronously with `std::net::TcpListener::bind`,
   read `local_addr().port()`, write the *actual* port into
   `bound_port`, then hand the listener to
   `tokio::net::TcpListener::from_std` for the async accept loop.

Same fix mirrored across `http`, `https`, and `http2` server FFIs.
The synchronous bind also matches Node's observable timing: the
`listen()` callback runs after the socket is actually bound.

After this PR, 3 of the 4 "Cannot read 'listen'" runtime-fail tests
flip to pass (test-https-agent, test-https-agent-servername,
test-https-agent-additional-options); the fourth (test-http-abort-client)
moves past the listen crash to surface a distinct downstream gap
(serverRes.on undefined) — separate follow-up.
CI's `every_dispatch_entry_has_manifest_counterpart` drift test on
PR #2197 caught that the new `Server` rows added to
`NATIVE_MODULE_TABLE` had no corresponding entries in
`API_MANIFEST` — drift here would make the #463 unimplemented-API
gate error on a real implementation. Add `method("http", "Server")`
and `method("https", "Server")` next to the existing `createServer`
rows.

Regenerate `docs/api/perry.d.ts` and `docs/src/api/reference.md` for
the api-docs-drift gate. The diff is exactly the two new `export
function Server(...)` lines under the `http` and `https` modules.
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.

1 participant