Skip to content

[Bug] ClusterClient.connect() clears the hello timeout in onError without settling once the socket has opened, so a post-handshake reset hangs the promise forever and ensureConnected memoises it for every later send #938

Description

@pathosDev

Problem

ClusterClient.connect() can produce a promise that never settles, and ensureConnected() memoises it. From that moment every send() and every ask() on the client hangs forever — including their own timeouts, which are armed after the await.

The mechanism is two lines. onError clears the hello deadline unconditionally, but only rejects when the socket had not opened yet:

  • socket opens → openSock is set → onOpen writes hello
  • peer resets before hello-ackonError fires → clearTimeout(timer) runs, openSock !== null, so the handler logs a warning and returns
  • the new Promise<TcpSocketLike> now has no path to resolve and no path to reject. onClose follows, but onSocketClose() only tears down the pending-ask map — it never settles the connect promise either.

connect() therefore never returns, so ensureConnected()'s finally { this.connectingPromise = null } never runs, so the dead promise stays in connectingPromise and if (this.connectingPromise) return this.connectingPromise hands it to every subsequent caller. There is no re-dial, no failover to the next contact point, and no error — the class is pitched at REST frontends and cron jobs, which will simply stop.

Evidence

src/cluster/ClusterClient.ts:269-274
              onClose: (_s) => this.onSocketClose(),
              onError: (_s, err) => {
                clearTimeout(timer);
                if (openSock === null) reject(err);
                else this.log.warn(`ClusterClient socket error`, err);
              },
src/cluster/ClusterClient.ts:219-229
  private async ensureConnected(): Promise<void> {
    if (this.stopped) throw new Error('ClusterClient is closed');
    if (this.socket && this.contactPointPeer) return;
    if (this.connectingPromise) return this.connectingPromise;
    this.connectingPromise = this.connect();
    try {
      await this.connectingPromise;
    } finally {
      this.connectingPromise = null;
    }
  }

ask's own timeout cannot save the caller — it is created inside the Promise executor that runs only after await this.ensureConnected() returns:

src/cluster/ClusterClient.ts:180-194
    await this.ensureConnected();
    const askId = nextAskId();
    const env: ClusterClientEnvelopeMessage = {
      kind: 'cluster-client-envelope',
      from: this.identity.toJSON(),
      to: targetPath,
      askId,
      body: message,
    };
    return new Promise<R>((resolve, reject) => {
      const ms = timeoutMs ?? this.askTimeoutMs;
      const timer = setTimeout(() => {
        this.pending.delete(askId);
        reject(new Error(`ClusterClient.ask timed out after ${ms}ms (path=${targetPath})`));
      }, ms);

Proposal

Settle the promise on every terminal socket event, not just the pre-open ones:

  • onError rejects unconditionally when the handshake has not completed (this.contactPointPeer === null for this attempt); only a post-handshake error belongs in the warn branch, and this promise is by construction pre-handshake.
  • onClose must also reject a connect attempt that has not yet resolved — a peer that accepts the socket and then sends FIN without hello-ack currently only fails because the 5 s deadline is still armed, which is accidental.
  • Do not clear the deadline in a branch that does not settle. Clearing it is what removes the last backstop.
  • Belt and braces: give ensureConnected an unconditional finally that survives a hung connect() — e.g. race the memoised promise against the deadline, or clear connectingPromise from the settle paths rather than from await's continuation.

close() should also reject the in-flight connect promise; today it flips stopped (which blocks new calls) but leaves callers already parked on ensureConnected parked forever.

Distinct from #689, which asks for reconnect-with-backoff on the onSocketClose path — that is about an established connection dropping. This is about a connect attempt that never terminates and is then handed to everyone. #689's retry loop would inherit the same hang unless this is fixed first.

Acceptance sketch

  • A contact point that accepts the TCP connection and then resets before hello-ack makes the pending ask reject, not hang.
  • The rejection happens within the ask timeout, and no later than HELLO_TIMEOUT_MS.
  • After such a failure a subsequent send()/ask() dials again (round-robin to the next contact point) instead of returning the previous attempt's promise.
  • connectingPromise is null once the attempt has settled, on every path.
  • close() rejects any connect attempt still in flight.

Verification status

Found in the ten-lens production-readiness review of 2026-08-05 (v0.13.0) and re-verified before filing: reproduced by execution. A stub TcpBackend was substituted for the runtime one and driven through exactly the sequence Node's net.Socket emits on a post-handshake reset (onOpen, then onError with ECONNRESET, then onClose):

[WARN] cluster-client - ClusterClient socket error warn: read ECONNRESET code: "ECONNRESET"
ask #1 after 8015ms  -> PENDING
   HELLO_TIMEOUT_MS = 5000, per-ask timeout = 300 — neither fired.
send #2 after 4s        -> PENDING
sockets dialled: 1 (a memoised promise means no re-dial)
connectingPromise still set: true
send #3 after close()   -> rejected: ClusterClient is closed

Two calls, 12 s, neither settled; one socket dialled, so no failover was attempted. The stub was used rather than a live peer because Bun's node:net shim does not surface ECONNRESET as an error event for a locally-reset socket, so the sequence is not reachable from a same-process test server on that runtime — the stub reproduces the documented Node semantics that NodeTcpBackend wires straight through (src/runtime/tcp/NodeTcpBackend.ts:68).

Part of the production-readiness review batch — tracked in #913.

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't workingpriority: highTop priority — high impact, plan nextproduction-goalBlocks or defines the path to production readiness

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions