net: add experimental net/promises API - #63965
Conversation
|
Review requested:
|
pimterry
left a comment
There was a problem hiding this comment.
One of the existing tests is unhappy, but otherwise this looks great to me ❤️
I'd be OK to ship experimental as-is, one note to consider: normal netServer.listen({ signal }) takes a signal which can be used to abort the server any time, but netPromises.listen({ signal }) takes a signal that can be used to cancel only the initial listen, and does nothing later on. The signal is linked only to the once(server, 'listening'... and is dropped from the server itself.
There's a specific comment confirming this is intentional, but then the docs here slightly disagree... I can see arguments both ways: this approach is more clearly linked to the promise flow, but then it does create an inconsistency between the APIs.
I'm happy either way tbh, just food for thought.
|
Shouldn't it be async iterable instead? for await (const req of listen({ port })) {
// ...
} |
|
|
||
| const server = await listen({ port: 8124 }); | ||
| for await (const socket of server) { | ||
| socket.end('hello world!'); |
There was a problem hiding this comment.
Not really a big fan of this API model for the server given that it's a bit of a footgun.... do too much on each iteration and you end up serializing all of your connections. Not opposed to having it, but I'd prefer if the example was more illustrative of a more correct approach. i.e. dispatching the handling of the socket to another async task allowing the loop to iterate to receive the next socket.
| const socket = lazy.connect({ ...options, signal: undefined }); | ||
|
|
||
| try { | ||
| await once(socket, 'connect', signal !== undefined ? { signal } : kEmptyObject); |
There was a problem hiding this comment.
If the socket errors before the connect event, would this end up hanging?
There was a problem hiding this comment.
No I believe this correctly rejects. The ECONNREFUSED test in test-net-promises-connect.js should be covering this (https://github.com/nodejs/node/pull/63965/changes#diff-20589a91d7b722a33615840a6188115a9a297c2942b6d49ce1615e6dd8dfec71R35)
|
I think I'm running into flake on the CI. Can someone verify? Would love to see this land soon as its been open a for a couple weeks now. |
Add an experimental `net/promises` namespace, accessible via
`require('node:net/promises')` or `net.promises`, mirroring the existing
`fs/promises` and `dns/promises` modules.
It provides promise-based variants of net's one-shot lifecycle
operations:
- `connect()` returns a promise that fulfills with a connected Socket
once the 'connect' event fires, and rejects on connection failure or
when an optional AbortSignal is aborted, destroying the socket.
- `listen()` creates a server, begins listening, and returns a promise
that fulfills with the listening Server once the 'listening' event
fires. It rejects if the server fails to bind or an optional
AbortSignal is aborted, closing the server.
The functions are named for the actions they await, forming a parallel
`connect()`/`listen()` pair. This is intentionally not the callback
API's factory taxonomy: there `createConnection()` is canonical because
it both creates a socket and initiates connecting, whereas
`createServer()` is a pure factory that does not listen. A
`createConnection`/`createServer` pairing would be inconsistent here
because `createServer` has no completion to await, so no
`createConnection` alias is provided.
Refs: nodejs#21482
Assisted-by: Claude Opus 4.8 <noreply@anthropic.com>
Signed-off-by: Ethan Arrowood <ethan@arrowood.dev>
Address review feedback on the net/promises API: - `listen()` now passes its `signal` through to `server.listen()` so the signal aborts the server for its entire lifetime rather than only the pending listen. Aborting before the server is listening rejects the returned promise; aborting afterwards closes the server, matching net.Server's own `signal` behavior. - `net.Server` is now async iterable: `for await (const socket of server)` yields each incoming connection as an alternative to the 'connection' event, mirroring readline.Interface. Also update test-repl-tab-complete-require and test-repl-tab-complete-import to account for the new `net/promises` builtin appearing in completions. Refs: nodejs#21482 Assisted-by: Claude Opus 4.8 <noreply@anthropic.com> Signed-off-by: Ethan Arrowood <ethan@arrowood.dev>
Address review feedback on the net/promises API: - Use `signal.throwIfAborted()` for the pre-connect and pre-listen abort checks instead of constructing an AbortError by hand. - Update the `server[Symbol.asyncIterator]()` documentation example to dispatch each connection to a separate async task, with a note that awaiting inline serializes connections. Refs: nodejs#21482 Assisted-by: Claude Opus 4.8 <noreply@anthropic.com> Signed-off-by: Ethan Arrowood <ethan@arrowood.dev>
f6adebd to
25fa21a
Compare
|
I've rebased on main to hopefully cleanup CI flake. I think I need a rereview before requesting CI again though. please and thank you 😄 |
|
Ugh stupid Claude it didn't commit like I told it to. One moment. |
listen() with no target — the `{}` default, or an options bag with only
non-target fields like `{ signal }` — passed a target-less object to
server.listen(), which throws ERR_INVALID_ARG_VALUE. Default to an
ephemeral port in that case, matching server.listen() with no arguments,
and honor other options such as `host`. Adds regression coverage.
Assisted-by: Claude Opus 4.8 <noreply@anthropic.com>
Signed-off-by: Ethan Arrowood <ethan@arrowood.dev>
e9915cc to
d5c13e8
Compare
Commit Queue failed- Loading data for nodejs/node/pull/63965 ✔ Done loading data for nodejs/node/pull/63965 ----------------------------------- PR info ------------------------------------ Title net: add experimental net/promises API (#63965) Author Ethan Arrowood <ethan@arrowood.dev> (@Ethan-Arrowood) Branch Ethan-Arrowood:net-promise -> nodejs:main Labels net, semver-minor, experimental, needs-ci, commit-queue-squash Commits 4 - net: add experimental net/promises API - net: tie listen() signal to server lifetime, async-iterable Server - net: use throwIfAborted and improve server iteration example - net: default net/promises listen() to an ephemeral port Committers 1 - Ethan Arrowood <ethan@arrowood.dev> PR-URL: https://github.com/nodejs/node/pull/63965 Refs: https://github.com/nodejs/node/issues/21482 Reviewed-By: Tim Perry <pimterry@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> ------------------------------ Generated metadata ------------------------------ PR-URL: https://github.com/nodejs/node/pull/63965 Refs: https://github.com/nodejs/node/issues/21482 Reviewed-By: Tim Perry <pimterry@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> -------------------------------------------------------------------------------- ℹ This PR was created on Wed, 17 Jun 2026 14:31:48 GMT ✔ Approvals: 2 ✔ - Tim Perry (@pimterry): https://github.com/nodejs/node/pull/63965#pullrequestreview-4774400378 ✔ - James M Snell (@jasnell) (TSC): https://github.com/nodejs/node/pull/63965#pullrequestreview-4696150709 ✔ Last GitHub CI successful ℹ Last Full PR CI on 2026-07-17T14:59:10Z: https://ci.nodejs.org/job/node-test-pull-request/74840/ ⚠ Commits were pushed after the last Full PR CI run: ⚠ - net: default net/promises listen() to an ephemeral port - Querying data for job/node-test-pull-request/74840/ ✔ Build data downloaded ✔ Last Jenkins CI successful -------------------------------------------------------------------------------- ✔ Aborted `git node land` session in /home/runner/work/node/node/.ncuhttps://github.com/nodejs/node/actions/runs/30277096239 |
Commit Queue failed- Loading data for nodejs/node/pull/63965 ✔ Done loading data for nodejs/node/pull/63965 ----------------------------------- PR info ------------------------------------ Title net: add experimental net/promises API (#63965) Author Ethan Arrowood <ethan@arrowood.dev> (@Ethan-Arrowood) Branch Ethan-Arrowood:net-promise -> nodejs:main Labels net, semver-minor, experimental, needs-ci Commits 4 - net: add experimental net/promises API - net: tie listen() signal to server lifetime, async-iterable Server - net: use throwIfAborted and improve server iteration example - net: default net/promises listen() to an ephemeral port Committers 1 - Ethan Arrowood <ethan@arrowood.dev> PR-URL: https://github.com/nodejs/node/pull/63965 Refs: https://github.com/nodejs/node/issues/21482 Reviewed-By: Tim Perry <pimterry@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> ------------------------------ Generated metadata ------------------------------ PR-URL: https://github.com/nodejs/node/pull/63965 Refs: https://github.com/nodejs/node/issues/21482 Reviewed-By: Tim Perry <pimterry@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> -------------------------------------------------------------------------------- ℹ This PR was created on Wed, 17 Jun 2026 14:31:48 GMT ✔ Approvals: 2 ✔ - Tim Perry (@pimterry): https://github.com/nodejs/node/pull/63965#pullrequestreview-4774400378 ✔ - James M Snell (@jasnell) (TSC): https://github.com/nodejs/node/pull/63965#pullrequestreview-4696150709 ✔ Last GitHub CI successful ℹ Last Full PR CI on 2026-07-28T14:28:40Z: https://ci.nodejs.org/job/node-test-pull-request/75255/ - Querying data for job/node-test-pull-request/75255/ ✔ Build data downloaded ✔ Last Jenkins CI successful -------------------------------------------------------------------------------- ✔ No git cherry-pick in progress ✔ No git am in progress ✔ No git rebase in progress -------------------------------------------------------------------------------- - Bringing origin/main up to date... From https://github.com/nodejs/node * branch main -> FETCH_HEAD ✔ origin/main is now up-to-date - Downloading patch for 63965 From https://github.com/nodejs/node * branch refs/pull/63965/merge -> FETCH_HEAD ✔ Fetched commits as b2a024b1ad33..d5c13e822782 -------------------------------------------------------------------------------- Auto-merging doc/api/net.md Auto-merging lib/net.js [main 907c990636] net: add experimental net/promises API Author: Ethan Arrowood <ethan@arrowood.dev> Date: Wed Jun 17 16:02:45 2026 +0200 6 files changed, 281 insertions(+) create mode 100644 lib/internal/net/promises.js create mode 100644 lib/net/promises.js create mode 100644 test/parallel/test-net-promises-connect.js create mode 100644 test/parallel/test-net-promises-listen.js Auto-merging doc/api/net.md Auto-merging lib/net.js [main 116667e49e] net: tie listen() signal to server lifetime, async-iterable Server Author: Ethan Arrowood <ethan@arrowood.dev> Date: Thu Jun 18 11:15:38 2026 +0200 7 files changed, 132 insertions(+), 16 deletions(-) create mode 100644 test/parallel/test-net-server-async-iterator.js Auto-merging doc/api/net.md [main 50af485967] net: use throwIfAborted and improve server iteration example Author: Ethan Arrowood <ethan@arrowood.dev> Date: Fri Jun 19 17:34:23 2026 +0200 2 files changed, 20 insertions(+), 14 deletions(-) [main 63654bbd5a] net: default net/promises listen() to an ephemeral port Author: Ethan Arrowood <ethan@arrowood.dev> Date: Fri Jul 24 08:53:28 2026 -0600 2 files changed, 28 insertions(+), 2 deletions(-) ✔ Patches applied There are 4 commits in the PR. Attempting autorebase. (node:366) [DEP0190] DeprecationWarning: Passing args to a child process with shell option true can lead to security vulnerabilities, as the arguments are not escaped, only concatenated. (Use `node --trace-deprecation ...` to show where the warning was created) Rebasing (2/8) Executing: git node land --amend --yes ⚠ Found Refs: https://github.com/nodejs/node/issues/21482, skipping.. --------------------------------- New Message ---------------------------------- net: add experimental net/promises API
The functions are named for the actions they await, forming a parallel Refs: #21482
|
|
Landed in 6d8baea |
|
omg finally. i swear everytime I try to land a PR I have to wrestle CI into submission lol |
Adds an experimental
net/promisesnamespace that provides promise-based variants ofnet's connection and listening operations, mirroring the existingfs/promisesanddns/promisesmodules. It is accessible viarequire('node:net/promises')orrequire('node:net').promises.Motivation
nethas no promise-based API. To await a connected socket or a listening server today you have to hand-wire the'connect'/'listening'and'error'events into anew Promise, or reach forevents.once. There is long-standing demand for a first-class promise API for these one-shot lifecycle moments (Refs: #21482).netis stream- and event-oriented, so only its one-shot async moments map cleanly to promises (establishing a connection and binding a server). The rest of the api remains the same.What's added
netPromises.connect(options)/connect(path)/connect(port[, host])—resolves with a connected
net.Socketonce its'connect'event fires;rejects on connection failure or abort, destroying the socket.
netPromises.listen([options])— creates a server, begins listening, andresolves with the listening
net.Serveronce its'listening'event fires;rejects if it fails to bind or is aborted before listening, closing the
server. Accepts a
connectionListeneroption and the usualnet.createServer/server.listenoptions. Thesignalaborts the serverfor its entire lifetime (aborting after it is listening closes it), matching
net.Server's ownsignalbehavior.net.Serveris now async iterable:for await (const socket of server)yields each incoming connection, as an alternative to the
'connection'event (mirrors
readline.Interface). Connections buffer if the consumer isslow;
server.maxConnectionsbounds concurrency.isIP,isIPv4,isIPv6,BlockList,SocketAddress) are re-exported for convenience.Both functions accept an optional
AbortSignaland reject with anAbortErrorwhen it is aborted, consistent withtimers/promisesandreadline/promises.Example:
A note on naming (
connect/listen, notcreateConnection/createServer)The naming here is intentional. This API promisifies actions that complete:
connect()resolves once the connection is established, andlisten()resolves once the server is listening. The function name is the action being awaited, and the two form a parallel pair.That is deliberately not the factory taxonomy of the callback API. There,
createConnection()is the canonical name because it both creates a socket and initiates connecting. Creation and the action are fused, so the factory name also implies the action.createServer(), by contrast, is a pure factory: it does not listen; listening is the separateserver.listen()step. So acreateConnection/createServerpairing would be inconsistent in a promise API.createServerhas no completion to await. Naming the functions for their actions (connect/listen) keeps the pair coherent, and there is no factory counterpart to alias, socreateConnectionis intentionally omitted.Notes
events.once(emitter, name, { signal }), so error and abort handling match the rest of core.Refs: #21482
Assisted-by: Claude Opus 4.8 noreply@anthropic.com