node:tls: let connect() re-dial a TLSSocket whose connection is still open

[robobun]

Problem

• socket.connect(port, host) on a socket returned by tls.connect() throws synchronously while the previous connection is still open (still connected, or half-closed after end()):
  TypeError: socket must be an instance of net.Socket or Duplex (src/js/node/net.ts:1982 on main). Once the socket has been destroyed the same call reconnects fine, and a plain net.Socket re-dials in this situation (node:net: handle socket.connect() on a socket that still has a live native handle #32739).
• Cause: TLSSocket.prototype[buntls] returns socket: this._handle (src/js/node/tls.ts:1121) so that new TLSSocket(stream).connect() can wrap the stream the constructor parked in _handle. After a connect, _handle is the native handle of the current connection; Socket.prototype.connect takes it as the stream to wrap (net.ts:1969) and the net.Socket/Duplex check rejects it.
• The same TypeError came out of connect() on a TLSSocket built over a caller-supplied stream (tls.connect({ socket: duplex }), new TLSSocket(netSocket).connect(port)); tls.connect({ socket: netSocket }) instead emitted a bare Error: Invalid socket from trying to adopt the same net.Socket a second time.

Fix

• tls.ts: [buntls] only forwards _handle as the stream to wrap when it is a Duplex. Everything the wrap path accepts is a Duplex (net.Socket included), so new TLSSocket(stream).connect() is unchanged; a native handle is simply not offered any more.
• A tls.connect(port) socket therefore falls through to the handle-reuse path net.Socket already uses (net.ts:2135, doConnect(this._handle, ...)): the native side closes the previous connection (detach_for_reconnect, src/runtime/socket/socket_body.rs:1299) and dials again on the same wrapper, and connect() has already reset the handshake state (net.ts:1986), so the socket emits secureConnect (and runs the connect listener) for the new connection. This is the same code that serves TCP since node:net: handle socket.connect() on a socket that still has a live native handle #32739 and that TLS reconnect-after-destroy already goes through; only the misrouting in front of it was TLS specific.
• net.ts: a TLSSocket whose current connection was built over a caller-supplied transport (this[kupgraded] set and the handle still live) that is asked to connect() without a new socket option is destroyed on the next tick with ExceptionWithHostPort(UV_EISCONN, "connect", host, port) instead of falling through.
  • The reuse path can only replace a connection the socket dialed itself: for a stream-engine handle detach_for_reconnect returns without detaching (no ext slot), which trips connect_finish's debug_assert!(prev.socket.get().is_detached()) (src/runtime/socket/Listener.rs:1557) and in release would alias two connections onto one wrapper; for an adopted-fd pair it would close the TLS half without the raw twin ever seeing the close (socket_body.rs:2064). Before this PR those sockets never reached that code because of the TypeError; without the guard they would.
  • There is also nothing to re-dial: the transport belongs to the caller. EISCONN is what node emits for connect() on a socket that is already connected (async 'error' with code: "EISCONN", syscall: "connect", then 'close' with hadError), and the object is built the way internalConnect builds its connect errors (net.ts:3161), so the three constructions now fail the same way. An explicit connect({ socket }) on such a socket keeps its current behavior.
• end() then reconnect: the reconnect itself works with this PR (secureConnect, data from the new connection is readable); the first write on it still fails with ERR_STREAM_WRITE_AFTER_END because the writable side was finished. That reset is node:net: reset the stream state when connect() reuses a half-closed socket #38980's (net-level) change; with its net.ts hunk applied on top of this branch the write goes through as well.
• Tests: test/js/node/tls/node-tls-connect.test.ts, describe("connect() on a TLSSocket whose previous connection is still open"): re-dial to a second server with the new arguments (greeting from the second server, echo over the new connection, connect listener), re-dial after end() against an allowHalfOpen server, and EISCONN for tls.connect({ socket: net.Socket }), tls.connect({ socket: Duplex }) and new TLSSocket(net.Socket).connect(port). Without the src change four of them throw the TypeError above and the net.Socket one gets Invalid socket; with it all five pass.
• Also run with the change: the rest of node-tls-connect.test.ts (42 pass, 18 skipped network tests); node-tls-upgrade, node-tls-server, node-tls-socket-allow-half-open-option, tls-connect-socket-churn, node-tls-duplex-close-throw-uaf, node-tls-internals, node-tls-context; net socket-reconnect-live, net-mongodb-pattern-leak, double-connect, node-net (its failures here are the localhost/::1 environment ones and fail identically without this change); and 51 ported node tests (test-tls-connect-, test-tls-socket-, test-tls-wrap-econnreset-, test-tls-starttls-server, test-tls-delayed-attach, test-https-agent-*, ...), all passing.

Background

• [buntls] (Symbol.for("::buntls::")) is the method net.ts's shared Socket.prototype.connect calls on a TLSSocket to get the TLS options for an attempt. Its socket field is a private channel meaning "run the handshake over this stream instead of dialing"; it exists for new TLSSocket(stream), whose constructor stores the stream in _handle until connect() wraps it.
• _handle is otherwise the native socket wrapper the JS socket owns. doConnect(handle, opts) passes it as the previous socket: connect_finish closes whatever connection it still holds and connects the same wrapper again. So in bun connect() on a live socket replaces the connection, where node re-runs uv_tcp_connect on the existing fd and gets EISCONN (on Linux the first retry reports a spurious 'connect' instead, see details).
• Wrapping a caller's transport produces one of two handle kinds: the TLS half of an adopted-fd pair (upgradeTLS; the raw half stays attached to the caller's net.Socket as the TLS half's twin), or a stream-engine handle (upgradeDuplexToTLS) that moves bytes through the Duplex. Every such path stores the transport in kupgraded, which net.ts already consults in open(), pause() and _destroy to treat these sockets differently.

Node v26.3.0 behavior, related open PRs, and observations left as is

Node, same scripts:

• tls.connect(port) socket, connect() after end(): 'error' connect EISCONN 127.0.0.1:port - Local (...), then 'close' with hadError = true.
• connect() while fully open: on Linux the kernel reports the completion of the earlier non-blocking connect once more, so node emits a second 'connect' on the same connection (same local port, the server sees one connection); a further connect() gets EISCONN. No new connection in either case.
• tls.connect({ socket: netSocket }) then connect(): ERR_INTERNAL_ASSERTION in afterConnect (Linux quirk above). tls.connect({ socket: duplex }) then connect(): nothing happens, the socket stays connecting.

Related open PRs:

• node:tls: make client-side new TLSSocket(socket) upgrade the socket it wraps #37664 (constructor wraps upgrade in the constructor) deletes the socket: line this PR changes; whichever lands second has a one-line conflict. It does not cover reconnects, and with tls.socket gone it would route the transport-backed cases into the reuse path, which is what the net.ts guard here prevents. If it lands first, the new TLSSocket(net.Socket).connect(port) case in the new tests becomes a reconnect of an already-wrapped socket (no secureConnect for a standalone wrap) and needs to be rewritten in those terms.
• node:net: reset the stream state when connect() reuses a half-closed socket #38980 resets the stream state on reuse (the write-after-end() half, see above). node:net: reject connect() while a connect is already in flight with EALREADY #32812 adds EALREADY in kConnectDispatch; with this PR a TLS socket reconnected mid-connect reaches that check too. No hunk overlap with either.
• node:tls: attach the onConnectEnd 'end' listener once per connect() #38007 makes each connect() register onConnectEnd once. Today every connect registers it twice and the handshake removes one, so a socket re-dialed in a loop keeps one extra inert 'end' listener per connect and hits MaxListenersExceededWarning on the 10th re-dial (pre-existing; before this PR the re-dial threw instead). With node:tls: attach the onConnectEnd 'end' listener once per connect() #38007's net.ts change applied on top of this branch the loop holds exactly one copy at each secureConnect and no warning; it applies without touching this PR's hunks and the tests here still pass, so that fix stays in node:tls: attach the onConnectEnd 'end' listener once per connect() #38007.

Left as is:

• A server-accepted TLSSocket can now be connect()ed as a client (it takes the reuse path), which is what a server-accepted net.Socket already does.
• tls.connect({ socket }) socket destroyed and then connect(port): unchanged, it re-enters the wrap path for the dead transport and never connects (node makes a plain TCP connection there). Not reachable through anything this PR changes.

[coderabbitai]

Warning

Review limit reached

@robobun, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 10 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: ccc2b477-2f33-40d8-aab8-75258e41c9ec

📥 Commits

Reviewing files that changed from the base of the PR and between 732491c and 4c7fe5c.

📒 Files selected for processing (3)

• src/js/node/net.ts
• src/js/node/tls.ts
• test/js/node/tls/node-tls-connect.test.ts

---

_{Comment @coderabbitai help to get the list of available commands.}

[robobun]

Status: reproduced and fixed, waiting on CI.

Reproduced on bun 1.4.0 and a debug build of main with a tls.connect() client that calls socket.connect(port, host) again, both from inside the connect callback and from end()'s callback against an allowHalfOpen server: both throw TypeError: socket must be an instance of net.Socket or Duplex; the same script on node v26.3.0 reports an asynchronous EISCONN (or, while still fully open on Linux, a second 'connect'), never a synchronous throw.

With this branch the tls.connect(port) cases re-dial and emit secureConnect for the new connection, and the transport-backed constructions (tls.connect({ socket }), new TLSSocket(socket)) report connect EISCONN asynchronously. The five new tests in test/js/node/tls/node-tls-connect.test.ts fail on the unfixed build and pass with the change; the related tls/net suites and the ported node tls tests listed in the description pass as well.

[robobun]

Re the onConnectEnd accumulation raised in the review: confirmed. On this branch a tls.connect() socket re-dialed in a loop holds 2, 3, 4, ... copies of onConnectEnd on 'end' at each secureConnect (one is removed after each handshake, so one more stays behind per connect) and MaxListenersExceededWarning fires on the 10th re-dial. It is the double registration (Socket.prototype.connect plus internalConnect/internalConnectMultiple) that #38007 removes, so I am leaving it to that PR rather than duplicating it here. Checked the two together: with #38007's net.ts change applied on top of this branch the same loop reports exactly 1 copy at every secureConnect and no warning, the patch applies without touching any hunk of this PR, and the five tests added here still pass. Added a note to the description.