feat(moq-ffi): sessions reconnect automatically - #2618
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 252a01ad40
ℹ️ 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".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b5e9e99d33
ℹ️ 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".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2f52538a1b
ℹ️ 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".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 61764de228
ℹ️ 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".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| pub fn close(&self) { | ||
| if let Some(session) = self.session() { | ||
| session.abort(moq_net::Error::Cancel); | ||
| } | ||
| self.abort.0.abort(); |
There was a problem hiding this comment.
When close() races with a redial completing, self.session() can return None and the background task can then publish a newly connected session before its abort takes effect, since aborting a Tokio task does not interrupt synchronous work before its next yield. The closed producer's final State consequently retains that un-aborted session through surviving Connection clones, so closed() reports that the loop stopped while the transport remains open. The fresh evidence after the earlier retained-session finding is this read-before-abort window in the final implementation; record closure in the shared state so publishing and removing the live session are synchronized, and cover the race with a regression test. (Written by GPT-5.6 Sol)
AGENTS.md reference: AGENTS.md:L99-L102
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in eabb522 — confirmed the read-before-abort window is real. abort read the live session out of the shared state and then aborted the task, and since aborting a tokio task does not interrupt it before its next yield, a dial completing in that window handed Shared::connected a session abort had already looked for and missed. It was parked in the final state and closed later by the refcount drop with a bare Cancel, so the code the caller passed never reached the peer.
Shared::connected and Connection::abort now take a shared CloseGuard around their state access, in the same order, so the session is either published before the close is recorded or refused (and aborted with the caller's error) after it.
On the regression test you asked for: I added abort_carries_its_code_to_the_peer, but it only covers the ordinary path where a session is live when abort runs, and I verified that by neutering the fix and watching it still pass. The race turns on when tokio cancels the loop task, which I could not reach deterministically from an integration test, so I have not claimed coverage I do not have rather than shipping a test that passes either way.
🤖 Addressed by Claude Code
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 16ca708609
ℹ️ 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".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
moq/rs/moq-native/src/connection.rs
Line 24 in bfb6eb5
Preserve TOML backoff overrides
When a relay/clustering config is loaded from TOML, Config::parse_and_merge deserializes the file and then re-applies CLI args with update_from; because this is a bare Duration with a clap default, an absent CLI flag writes 1s over any TOML backoff.initial value. The same pattern on multiplier, max, and timeout means TOML-only retry tuning is silently reset to defaults unless every knob is repeated on the command line, so make these CLI fields optional and resolve defaults through accessors like the other TOML-overridable settings.
moq/rs/moq-native/src/connection.rs
Line 693 in bfb6eb5
Stop stats readers from retaining closed transports
If a caller keeps a ConnectionStatsReader after dropping the last Connection clone, this cloned Consumer<State> keeps the final State allocation alive; when the abort-on-drop fires, the loop is aborted but state.session is never cleared or aborted, so the stats reader's retained state can keep the transport open indefinitely. This breaks the documented last-clone teardown path for native callers that cache the stats reader for polling; make drop/abort clear or abort the retained session, or avoid storing a strong session clone in the stats reader state.
ℹ️ 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".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 580c3c78d4
ℹ️ 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".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c3539d246b
ℹ️ 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".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 93727c00c7
ℹ️ 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".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| while await asyncio.wait_for(session.status(), timeout=10) != moq.ConnectionStatus.CONNECTED: | ||
| pass |
There was a problem hiding this comment.
After status() was narrowed to a snapshot API, this test can still miss the DISCONNECTED state: if the event loop does not reach this await until the 50 ms backoff has elapsed and the un-gated accept loop has already accepted the redial, the session's last reported value is still CONNECTED, the current value is also CONNECTED, and status() waits for a future transition until the 10s timeout. Gate the second accept, like the Rust test does, or assert via the post-reconnect announcement instead of depending on observing this coalescible edge.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in eabb522 — right on both counts, and it was a live flake rather than a theoretical one: with the accept loop un-gated the redial could land before we asked, leaving CONNECTED -> CONNECTED, which coalesces to no change and blocks until the 10s timeout.
Gated the second accept the way the Rust test does, and since the redial now cannot land early, the test asserts DISCONNECTED explicitly instead of looping past it.
🤖 Addressed by Claude Code
fa5484a to
38404f2
Compare
93727c0 to
f761c71
Compare
38404f2 to
59f9de2
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f761c71366
ℹ️ 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".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| // out a relay restart instead of tearing down. The linger rides the clone handed | ||
| // to the session; the caller-facing handle keeps the origin's own window. | ||
| let ingest = match reconnect { | ||
| true => subscribe.clone().with_linger(linger), |
There was a problem hiding this comment.
When reconnecting is enabled, a local shutdown() or cancel() aborts the session and drops its SourceGuard as an ungraceful loss, so every remote broadcast created through this linger-configured producer remains announced after the connection has permanently stopped. With the default backoff this leaves stale broadcasts for about five minutes, and with the documented timeout_ms = 0 setting Backoff::linger() returns Duration::MAX, leaving them in the origin forever. Tie linger to the reconnect loop's actual lifetime and finish the sources when that loop terminates, rather than encoding its policy solely as a fixed origin timer.
AGENTS.md reference: AGENTS.md:L102-L106
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Real, and filed as #2695 rather than fixed here. The with_linger(backoff.linger()) this describes is not introduced by this PR: the same pattern is already merged on dev in moq_native::Client::consume (via #2614), and this PR mirrors it into the FFI. Your timeout_ms = 0 -> Duration::MAX case is the sharpest form of it and I confirmed it reads that way.
Fixing it properly means tying linger to the reconnect loop's lifetime and finishing the sources when the loop terminates, which is a change to origin/linger lifetime semantics in already-merged moq-native code and deserves its own PR and tests rather than riding along on the FFI mirror.
🤖 Addressed by Claude Code
f761c71 to
04fdb64
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 04fdb642e7
ℹ️ 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".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| // Status blocks until the next connection status change. A client session | ||
| // reports StatusConnected first (the connect it was built from), then follows | ||
| // the reconnect loop: StatusDisconnected while redialing, StatusMigrating | ||
| // during a GOAWAY handover. It returns an error once the connection stops for |
There was a problem hiding this comment.
When a disconnect and reconnect both occur before the next Status call, Connection::poll_status compares only the last-reported value with the current value, so a Connected -> Disconnected -> Connected flap is coalesced and this call waits for a later change rather than reporting either edge. The fresh evidence after the earlier documentation fix is that the newly added Go documentation still promises the "next" change, while the Swift and Python wrappers similarly promise each/next transition; update these public API docs to match the narrowed current-state semantics already described on the Kotlin page, otherwise monitors can unexpectedly block after a fast reconnect. (Written by GPT-5.6 Sol)
AGENTS.md reference: AGENTS.md:L83-L88
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in eabb522 — you are right, and the earlier documentation pass was less complete than my reply to it claimed: it corrected the guides but left the code docs promising "the next change" in moq-ffi itself, Go, Python and Swift. Only the Kotlin page described the narrowed semantics.
All four now say status() waits for the status to differ from the one that handle last reported, that a drop which reconnects first is coalesced away, and that it is not for counting outages. Corrected at the source (rs/moq-ffi) as well, since that is where the wrappers take their wording from.
🤖 Addressed by Claude Code
04fdb64 to
ab3d9ac
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ab3d9ac915
ℹ️ 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".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| pub fn draining(&self) -> Option<moq_net::goaway::Consumer> { | ||
| self.state.read().session.as_ref().map(moq_net::Session::draining) |
There was a problem hiding this comment.
Replacing Connection::session() with draining() removes a public moq-native method that exists in the parent commit, so downstream callers using the accessor stop compiling when they update. Keep the old accessor working as a hidden deprecated compatibility shim, or land this removal only as a breaking release rather than removing it from the current API.
AGENTS.md reference: AGENTS.md:L91-L95
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Not applying this one. This PR targets dev, which CLAUDE.md reserves for exactly one thing: "a semver break in a published API: a renamed, removed, or signature-changed pub/exported item". So your own remedy, landing the removal as a breaking release rather than in the current API, is what targeting dev already means here; main is where the additive work goes.
A hidden deprecated shim would also cut against the repo's deprecation policy, which exists to keep a dead name off published surfaces, on the branch whose whole job is to carry breaks.
On the substance of keeping it: handing out a raw session clone is what the refcount lifecycle is there to prevent, since the clone keeps that transport open after the loop has moved on. The one real caller wanted the GOAWAY notice, which Connection::draining() now exposes directly.
🤖 Addressed by Claude Code
MoqClient.connect now returns a session backed by moq-native's Connection: when the transport drops (a relay restart, a laptop waking from sleep), a background loop redials with backoff and broadcasts consumed through the session linger across the gap, so an `announced()` iterator resumes instead of stalling silently forever. New knobs: set_reconnect(bool) and set_backoff(MoqBackoff), plus MoqSession.status() reporting Connected/Disconnected/Migrating transitions; closed() now means the connection stopped for good. Server-accepted sessions are unchanged (one transport; status() waits for the terminal close). To support this, moq-native's Connection is now Clone (the loop stops when the last clone drops) with an explicit close(). Also fixes a latent moq-ffi bug the new default exposed: Task::cancel used watch::Sender::send, which refuses to store the value while no receiver exists, so MoqClient.cancel() before the first connect() silently no-oped. It was masked because a bad dial used to fail fast on its own; with reconnecting on it hung the kt/swift fail-fast smoke tests. send_replace always stores. Wrappers: go (WithReconnect/WithBackoff/Session.Status), python (reconnect=/backoff= kwargs, Session.status), swift (setReconnect/setBackoff/status()), kotlin (connect params). Regression tests at the ffi and python layers kill the #2609 scenario: the server severs the first session and a broadcast published only after the redial must still reach the client. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
… FFI
Go's zero value and the native encoding disagree: the reconnect loop reads a
zero timeout as "retry forever" and a zero initial delay as no pacing, so
moq.Backoff{} (the most natural literal a Go caller writes) crossed the FFI
boundary as an unthrottled dial loop on moq-ffi's shared single-threaded
runtime. Negative durations were worse: uint64(d.Milliseconds()) wrapped to
~1.8e19 ms. Sub-millisecond values truncated back to an unpaced zero.
Each field now resolves to its documented default when unset, matching the
uniffi defaults the Python/Swift/Kotlin bindings already get for free, so a
partial Backoff overrides only what it sets. Unlimited retries move to an
explicit moq.RetryForever, which is the only way to reach the native zero.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
… up on Dropping a JoinHandle detaches its task rather than stopping it, so a caller that gave up (an asyncio.wait_for timeout, a cancelled Swift/Kotlin task) left the spawned closure running with the state lock held. It then consumed the next event into its own cursor while the retry blocked behind it, so the retry missed the edge it was waiting for. Every repeatable read on the bindings sits on this path (status, next, read_frame, recv_datagram); the new repeatable status() is just the easiest to drive, and its regression test fails without the fix. Also close the live session in Connection::close. Aborting the task drops the loop's producer, but the final state stays readable through every surviving handle, and the session clone parked in it held the transport open until the last handle went away, so the documented "stop now" didn't. And drop a match that only bound one arm, which -D warnings rejected in CI. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
`closed()` and `status()` ran the connection's terminal error through `map_connect_error`, which stringifies everything non-auth into `MoqError::Connect`. That claims the dial failed when it had succeeded, drops the `moq_net::Error` variant a caller can match on, and disagreed with the server-accepted path, which reports the same event as `MoqError::Protocol`. `map_closed_error` keeps the auth rejections in their dedicated variants, since that is what `is_auth` reads in every binding, and preserves the underlying error otherwise. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The new connect parameter took the raw `uniffi.moq.MoqBackoff`, so a Kotlin caller importing `dev.moq.*` like the docs say could not name or construct it without reaching into the bindings. Swift, Go, and Python all got their aliases in the same pass; Kotlin was missed. Extended the alias-coverage test, which is a hand-maintained list and so did not catch this, and fixed the guide, which named the raw type too. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
A caller blocked in `status()` while another handle shuts the connection down got `MoqError::Connect`, so every Python/Go/Swift/Kotlin status watcher read an expected teardown as a broken connection, and `is_shutdown` did not recognize it. `closed()` already returned `Ok` for the same stop. `moq_native::Error::Stopped` now names that case, and it maps to `Closed`, which is what `is_shutdown` reads. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
`Connection::abort` read the live session out of the shared state and then aborted the loop task. Aborting a tokio task doesn't interrupt it before its next yield, so a redial completing in that window handed `Shared::connected` a session the abort had already looked for and missed. It was parked in the final state and closed later by the refcount drop, with a bare `Cancel`, so the code the caller passed never reached the peer. Both sides now take a shared `CloseGuard` around their `state` access, so the session is either published before the close is recorded or refused after it. Also corrects `status()`'s documented contract, which promised "the next change" while the implementation reports the current status, coalescing a flap that heals before the next call. The Kotlin guide already said so; this brings moq-ffi, Go, Python and Swift in line. The Python reconnect test depended on observing that coalescible edge, so it now gates the redial like the Rust one. Addresses unresolved review findings on this PR. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1211623452
ℹ️ 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".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| reconnect: Boolean? = null, | ||
| backoff: Backoff? = null, |
There was a problem hiding this comment.
Existing callers that pass publish or subscribe positionally now bind those arguments to reconnect and backoff, producing type errors after upgrading. Keep the existing parameter order by appending these new defaulted options after subscribe, or provide a compatibility overload, so this additive feature does not become a Kotlin source break. (Written by GPT-5.6 Sol)
AGENTS.md reference: AGENTS.md:L180-L180
Useful? React with 👍 / 👎.
1211623 to
eabb522
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: eabb522da3
ℹ️ 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".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| #[uniffi(default = 30000)] | ||
| pub max_ms: u64, | ||
| /// Time spent retrying before giving up, in milliseconds. 0 retries forever. | ||
| #[uniffi(default = 300000)] |
There was a problem hiding this comment.
When a binding passes an otherwise-default MoqBackoff, or a Go caller supplies a partial Backoff, these 30s/5m values replace moq_native::Backoff's actual 5s/10s defaults. Consequently, changing only one knob can unexpectedly extend a failed connection from roughly 10 seconds to five minutes. Align these defaults with the native configuration, or initialize every client with the longer binding defaults before applying overrides. (Written by GPT-5.6 Sol)
AGENTS.md reference: AGENTS.md:L100-L102
Useful? React with 👍 / 👎.
1 participant
Closes #2609. #2614 has merged, so this now targets
devdirectly.What
MoqClient.connectnow returns a session backed by moq-native'sConnection: when the transport drops (a relay restart, a laptop waking from sleep severing the QUIC path), a background loop redials with backoff, and broadcasts consumed through the session linger across the gap (with_linger(backoff.linger()), mirroringmoq_native::Client::consume). The silent stall from the issue, anannounced()iterator that never yields, returns, or raises after a drop, becomes a gap the session splices over.New surface, mirrored into every wrapper:
MoqClient.set_reconnect(bool)(default on) andset_backoff(MoqBackoff { initial_ms, multiplier, max_ms, timeout_ms }).MoqSession.status(): waits for the status to differ from the one that handle last reported (Connected/Disconnected/Migrating); errors once the connection stops for good. This is the issue's "closed-style handle that distinguishes still-retrying from gave-up". It reports the current status rather than queueing every edge, so a flap that heals before you ask again is coalesced away.MoqSession.closed()now resolves only when the connection is over: the terminal error on give-up,Okafter a local shutdown. Server-accepted sessions are unchanged (one transport; theirstatus()waits for the terminal close).Internally
MoqSessionwraps anInnerenum (clientConnectionvs serverSession) with separateclosed/statustask locks, since a parkedclosed()holds its lock forever.moq_native::ConnectionbecomesClone(loop stops when the last clone drops;AbortOnDropin anArc) with an explicitclose(), which is what lets the FFI hand clones to those tasks.Connection::session()is removed as part of that change. Handing out a raw session clone keeps that transport open after the loop has moved on, which is exactly the leak the refcount lifecycle exists to prevent. The one caller that needed it wanted the GOAWAY notice, soConnection::draining()exposes that directly.Bug found along the way (root cause + fix)
Task::cancelinrs/moq-ffi/src/ffi.rsusedwatch::Sender::send, which refuses to store a value while no receiver exists. SoMoqClient.cancel()before the firstconnect()silently no-oped. It was masked because a bad dial used to fail fast on its own; with reconnecting on, the kt/swift "cancel then connect fails fast" smoke tests hung in the retry loop (the swift suite quietly burned 5 minutes even while passing). Fixed withsend_replace, regression-tested bycancel_before_connect_fails_fast.Semantics note: rejections
devclassifies the wire's UNAUTHORIZED as terminal, so an auth rejection stops the loop instead of retrying the same credentials until the give-up timeout (reconnect_stops_on_a_session_level_rejection). Any other MoQ-layer rejection (Request::closewith some other code, after the transport is accepted) still reaches the client as an untyped transport close, so the loop cannot tell it from a network blip and retries until the timeout. Classifying the rest needs the transport trait to expose close codes, the same gap as #2508 on the JS side; noted in a code comment on the loop. Tests that want to observe those rejections directly use one-shot mode (set_reconnect(false)).Tests
client_reconnects_and_resumes_announcements(Rust FFI) andtest_client_reconnects_and_resumes_announcements(Python): the server severs the first session; a broadcast published only after the automatic redial must reach the client, withstatus()observing Connected -> Disconnected -> Connected. This encodes the exact moq-ffi: expose moq-native's reconnect — bindings get a one-shot session that stalls silently #2609 stall.one_shot_client_close_surfaces_through_closed,one_shot_surfaces_a_session_level_rejection,rejected_session_surfaces_through_closed,cancel_before_connect_fails_fast.Cross-package sync
WithReconnect/WithBackoff(durations) /Session.Status+ConnectionStatusconsts; py:reconnect=/backoff=kwargs +Session.status+moq.Backoff/moq.ConnectionStatusre-exports; swift:setReconnect/setBackoff/status()+ aliases; kotlin:connect(reconnect =, backoff =)(the rawsessionalready exposesstatus()).doc/lib/{py,go,swift,kt}.Rebase onto merged
devRebasing off the merged #2614 needed three real resolutions, not just context shuffling:
devgained "leave on a GOAWAY instead of ignoring it in one-shot mode".goaway_testdials one-shot, so honoring the notice now ends the connection, and readingconnection.draining()after signalling the drain raced that teardown and panicked onNone(5goaway_*tests). The handle is now taken while the session is still up, which is what the test meant all along.devmovedsession_close_surfaces_a_rejection_codeandreconnect_stops_on_a_session_level_rejectionbelowconnect_onceand rewrote them; the 3-way merge duplicated both. Kept one copy each, adapted toconnect_oncereturning aConnection.dev's UNAUTHORIZED handling and contradicted the code above it. Corrected here and in the matching Python test comment.Adversarial review follow-up
A Codex adversarial pass flagged the Go
Backoffconversion, and it was right: Go's zero value and the native encoding disagree. The loop reads a zero timeout as "retry forever" and a zero initial delay as no pacing, somoq.Backoff{}, the most natural literal a Go caller writes, crossed the boundary as an unthrottled dial loop on the shared single-threaded FFI runtime, and negative durations wrapped to ~1.8e19 ms viauint64(d.Milliseconds()). Fixed in the second commit: every field resolves to its documented default when unset (matching the uniffi defaults Python/Swift/Kotlin already get for free), sub-millisecond values floor at 1ms, and unlimited retries move to an explicitmoq.RetryForever. Table test ingo/wrapper/moq/backoff_internal_test.gocovers each case.The same review raised two more points that don't apply here. It flagged the GOAWAY redirect guard (
is_localclassifying hosts by name rather than resolved address), which is real but pre-existing:git blameputs it in #2542, already ondev. Filed separately as #2624. It also reported a breaking public API change againstmain, an artifact of diffing against the wrong base, since this targetsdevper the branch-targeting rules.Review follow-up (this round)
abortcould miss a redial landing in its own teardown window. It read the live session out of the shared state and then aborted the loop task, but aborting a tokio task doesn't interrupt it before its next yield, so a dial completing in that window handedShared::connecteda sessionaborthad already looked for and missed. It was parked in the final state and closed later by the refcount drop with a bareCancel, so the code the caller passed never reached the peer. Both sides now take a sharedCloseGuardaround theirstateaccess, so the session is either published before the close is recorded or refused after it.status()'s documented contract was wrong. It promised "the next change" while the implementation reports the current status, coalescing a flap that heals before the next call. The Kotlin guide already said so; moq-ffi, Go, Python and Swift now match. The Python reconnect test depended on observing that coalescible edge, so it gates the redial like the Rust one instead of racing it.Two findings were not applied:
Connection::session()as a deprecated shim. This targetsdev, which CLAUDE.md reserves for exactly this ("a renamed, removed, or signature-changedpubitem"), so the reviewer's own remedy (land it as a breaking release) is satisfied. A hidden shim would contradict the deprecation policy on the branch that exists to carry breaks.with_linger(backoff.linger())it describes is already merged ondevinmoq_native::Client::consume; this PR only mirrors it into the FFI. A fix belongs at the moq-native/moq-net layer with its own tests, so it is filed as moq-native: linger keeps broadcasts announced after a permanent local stop #2695.On test coverage for the
abortrace:abort_carries_its_code_to_the_peercovers the ordinary path, where a session is live whenabortruns. It does not cover the race, and I verified that by neutering the fix and watching it still pass. The window turns on when tokio cancels the loop task, which isn't reachable from an integration test, so the fix ships without a regression test for the race itself rather than with one that passes either way.Validation
Against
dev@613bf5269:just fixclean,just checkgreen,cargo nextest run --workspace --exclude moq-gst2774/2774 (just checkalone never compilesmoq-ffiorlibmoq, which are commented out ofdefault-members, so the workspace run is what actually covers this PR).just go check(incl.go test -race),just py check+just py test(52),just swift check,just kt checkall green; rustdoc-D warningsclean.just test smoke-fullis 21/21 green. It was failing 9 browser pairs earlier in this PR's life withFailed to construct 'WebTransportError'; I confirmed at the time that a baseline run on plaindevproduced a byte-identical matrix, and #2698 has since fixed it ondev.(written by Opus 5)