Skip to content

fix(net)!: stop one silent dial from stalling the reconnect loop - #6608

Merged
kixelated merged 5 commits into
mainfrom
net-attempt-deadline
Oct 24, 2026
Merged

kixelated merged 5 commits into
mainfrom
net-attempt-deadline

Conversation

@kixelated

@kixelated kixelated commented Oct 24, 2026 •

Copy link
Copy Markdown
Collaborator

Replaces moq-dev/moq#6540 by Matt Sheppard, whose design and code this is. That branch lives on an organization-owned fork, which GitHub does not let maintainers push to, so this branch carries Matt's commit unchanged, a merge of main, and one test fix. History is intact (no rebase), and Matt keeps authorship of the original commit.

Changes since #6540

  • Merged main; the only conflict was the MIGRATE.md Unreleased list.
  • @moq/qmux on main (moq-dev/moq#6528) now refuses an empty negotiated subprotocol instead of falling back to legacy framing. The new tests' WebSocketStream stubs negotiated "", so they now negotiate qmux-01. The comment listing the bare ALPNs the WebSocket fallback advertises dropped the removed webtransport entry.
  • Merged main again. @moq/clock's install() now returns an Installed handle (moq-dev/moq#6510), so the new tests close it instead of calling it. The js/net/MIGRATE.md entry moved into Impact below, since PRs no longer write MIGRATE.md.

Problem

connect() has no deadline, and Reload hands it only its teardown signal. So a dial the network never answers holds the reconnect loop until the browser or OS gives up. delay.timeout is only checked after an attempt fails, so it doesn't help.

This happens when a client reconnects through a layer-4 load balancer.

  • We opened fresh TCP and QUIC flows every 200 ms to each address of an AWS NLB while one of its two targets was force-killed. The NLB had a TCP_UDP listener, IP targets, a health check every 10 s, a 300 s deregistration delay, and connection termination off.
  • For about 12 s after the target started draining, about one new flow in two landed on the dead target.
  • Every packet of a flow goes to the target it was placed on, so retransmits never escape. Only a new flow does.

In Chrome 152 on macOS, against an address that never answers:

  • WebTransport gives up after 4 s on its own (quiche's 5 s idle-before-handshake timer, less 1 s on the client).
  • A WebSocket waits for the OS TCP connect timeout: 75 s on macOS.

Promise.any waits until both fail, so an attempt whose dials all land on the dead target waits 75 s. Where WebSocket is the only transport (Safari, Firefox before 153, Node, Bun, Deno), one dead dial is enough. A browser page in this situation took 80 s to reconnect.

Approach

Four parts, all in connect(), so Reload, the pool's shared and private loops, GOAWAY migration and a one-shot connect() all get them.

  • A timeout per attempt. ConnectProps.timeout and ConnectionProps.timeout (default 30 s; Milli(0) for none) cover the dial and the MoQ handshake.
    • Past it, the attempt rejects with the new Error.ConnectTimeout, which Reload retries like any other failure.
    • This mirrors Rust's connect.timeout and Error::ConnectTimeout (moq-dev/moq#2622). The pair timeout / delay.timeout mirrors connect.timeout / connect.backoff.timeout.
  • A second WebTransport dial at 500 ms, if the first hasn't opened by then.
    • 500 ms is the default WebSocket head start, so a fresh WebTransport flow races the fallback instead of losing to it. Nothing upgrades a WebSocket session back.
    • Chrome opens each WebTransport on its own UDP socket (allowPooling: false), so each dial is a new flow.
    • Neither dial is cut on a timer of its own. The browser's handshake timer can tell a silent dial from a slow one; script can't. And Chrome penalises every failed or abandoned handshake in a per-page throttle.
    • A first dial that fails outright isn't hedged.
    • If both dials fail, the error names only the first dial's reason, so no transport connected: … keeps one reason per transport. A browser words every handshake failure the same way.
  • A WebSocket redial at 10 s, doubling.
    • A browser allows one connecting WebSocket per address (RFC 6455 section 4.1), so a second dial queues behind a silent one. Closing the silent one frees the address.
    • Checked in Chrome 152, for both WebSocketStream and WebSocket:
      • Against a local forwarder that accepts the TCP connection and never answers, a second WebSocket opened no connection until the first was closed, then connected at once.
      • Against an address that drops SYNs (WebSocket), a second WebSocket dialed 1.8 s in failed 75.0 s after the first was closed (at 5.8 s), not 75 s after its own dial. So it had been queued too.
    • Safari and Firefox are not checked.
  • Losers are closed. A contender that opened in the same turn as the winner had already returned its transport, so cancelling couldn't close it. That leak already existed between WebTransport and WebSocket. Every contender that wasn't chosen is now closed.

Consumers: <moq-watch> and <moq-publish> use the shared pool, so they get all of this through the defaults. The timeout options are the escape hatch described under Impact.

Impact

  • New ConnectProps.timeout and ConnectionProps.timeout. share: true refuses timeout, as it does delay.
  • New public Error.ConnectTimeout, with timeout in milliseconds.
  • Public API: new ConnectProps.timeout, ConnectionProps.timeout, and the exported Error.ConnectTimeout. The hedge and redial delays stay private constants.
  • Breaking: connect() and each Connection attempt reject with Error.ConnectTimeout after 30 s. A deadline set through signal that is longer than that no longer takes effect.
    • Migrate: connect(), and each attempt of a Connection, now rejects with Error.ConnectTimeout once the dial and the MoQ handshake together take longer than 30 s, closing what it had opened. Before, an attempt nothing answered waited until the browser or the OS gave up, which for a WebSocket can be minutes, and a supplied transport whose handshake was never answered waited forever. Pass timeout to change the bound, or timeout: Milli(0) for no deadline. A deadline set through signal, for example AbortSignal.timeout(60_000), now only takes effect when it is shorter than timeout.
  • An invalid timeout (negative, NaN, infinite) is a RangeError: connect() rejects, and a Connection throws when it is built.
  • Servers may see a second WebTransport session with the same URL and token, closed straight away.
  • Slow WebSocket opens: with the 30 s default, the slowest WebSocket open an attempt accepts is 20 s (10 s, then a 20 s redial), or 19.5 s after WebTransport's head start. Firefox already fails WebSocket opens at 20 s, so in practice this affects Safari, and Chrome on networks that block UDP.
  • Chrome's WebTransport throttle: each failed or abandoned handshake costs a 5-minute entry.
    • A client whose opens regularly take over 500 ms (round trips above about 250 ms) hedges on most connects.
    • In a long outage where every dial goes unanswered, hedging reaches the throttle's cap in about half as many attempts. At the cap, each new handshake is delayed 30 to 90 s while 14 or more entries are live.
    • The WebSocket fallback still recovers.
  • No wire change.

Alternatives

  • Racing whole connect() calls inside Reload: a near-tie wires both sessions into the origins and announces on both, an auth rejection gets lost in the AggregateError, and the WebSocket gains nothing because of the per-address limit.
  • A cancelling deadline on WebTransport, fixed or escalating: it either cuts slow handshakes that would have worked or waits on dead ones, and it cancels dials the browser judges better. The WebSocket redial is such a deadline, because the WebSocket can't be hedged.
  • Hedging at 1 s: fewer extra handshakes on slow links, but the WebSocket, already dialing at 500 ms, usually wins first and the session stays on WebSocket.
  • Deriving the hedge delay from a previous session's RTT, which RFC 8305 allows: a first connect has no RTT, and the outage is dominated by detection rather than by the 500 ms.
  • Skipping the hedge for a URL after an attempt in which every dial failed, remembered the way websocketWon is: it would halve the throttle cost in a long outage, but slow recovery when both of an attempt's dials happen to land on dead targets.

Follow-ups

  • accept() has no SETUP deadline on the server side either. That's a different problem (what a silent peer can make a server hold); to be planned as a quest.
  • moq-tokio has the 30 s timeout but no hedge or redial. Native clients have no per-address WebSocket limit, so a parallel redial would work there; to be planned as a quest.

Decisions (settled)

  • The 30 s default: kept, to match Rust's connect.timeout.
    • ✅ 30 s, matching Rust
    • Something shorter for browsers
  • The 500 ms second-dial delay and the 10 s WebSocket redial: kept as private constants. Nothing needs to tune them yet; if something does, they go on ConnectProps next to timeout.
    • ✅ Private constants
    • Public options now
  • Chrome's WebTransport throttle cost: accepted as documented above. The per-URL memory under Alternatives stays the option if recovery with WebSocket disabled matters.
    • ✅ Accept the cost
    • Skip the second dial after an attempt where every dial failed

Checks

  • just check passes, and the js/net suite passed 3 runs in a row (2732 tests).
  • quest check --base origin/main passes. No wire, moq-ffi, or gateway change, so no interop run.
  • Wire: none.

🤖 Generated with Claude Code

(Written by Claude Opus 5.5)

mattshep and others added 3 commits October 23, 2026 05:26
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
Main's @moq/qmux (#6528) refuses an empty negotiated subprotocol instead of
falling back to the legacy framing, so the stubs name qmux-01. The comment on
the advertised bare ALPNs drops the removed webtransport entry.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Oct 24, 2026 •

Copy link
Copy Markdown
Contributor

Warning

Review limit reached

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Next included review available in 14 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used all 4 included reviews currently available.

Learn how review limits work.

Review configuration:

Run configuration
  • Source mode: Auto
  • Repository: Unavailable
  • Branch: Unavailable
  • File: No configuration file from this repository applied
  • Commit: Unavailable
  • Layers: Organization UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 5ed45452-cfe6-4943-bf67-b2a2f516b185
Commits

Reviewing files that changed from the base of the PR and between d917a1f and 12034df.

Files selected for processing: 8
  • js/net/src/connection/connect.test.ts
  • js/net/src/connection/connect.ts
  • js/net/src/connection/pool.test.ts
  • js/net/src/connection/pool.ts
  • js/net/src/connection/reload.test.ts
  • js/net/src/connection/reload.ts
  • js/net/src/error.ts
  • js/net/src/errors.ts
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

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

@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Oct 24, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-10-24T10:37:01.040739Z 12034df New commits
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@kixelated
kixelated added this pull request to the merge queue Oct 24, 2026
@github-merge-queue
github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Oct 24, 2026
kixelated and others added 2 commits October 24, 2026 03:31
…ove the migration note to the PR body

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@kixelated
kixelated added this pull request to the merge queue Oct 24, 2026
Merged via the queue into main with commit babb189 Oct 24, 2026
10 checks passed
@kixelated
kixelated deleted the net-attempt-deadline branch October 24, 2026 13:34
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.

2 participants