VAPI-4011 fix(signaling): rebuild peer connections and re-publish media on signaling reconnect - #18
Merged
Conversation
…aling reconnect The websocket client auto-reconnects on drops (rpc-websockets, unlimited reconnect), and every reconnect re-fires "open" - re-running setMediaPreferences and re-emitting "init". BandwidthRtc.init() reacted to that by building brand-new RTCPeerConnections every time, without closing the stale ones or re-adding any already-published MediaStream tracks. A signaling reconnect therefore silently dropped all media and orphaned the old peer connections, even though the underlying connection is meant to resume the same session. signaling.ts now tracks whether an "open" is the first one or a reconnect and passes that through on the "init" event. bandwidthRtc.ts's init() closes the stale peer connections and resets subscribe-side bookkeeping on a reconnect, then re-adds every currently published stream to the rebuilt publishing connection and re-offers, so the far end keeps receiving media instead of silence. Also fixes a dead-code bug in setupPeerConnection/setupNewPeerConnection: the "disconnected" connection-state handler was being immediately overwritten by the "failed" handler set right after it, so the disconnected-state log could never fire. Merged into one handler. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
✅ Snyk checks have passed. No issues have been found so far.
💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse. |
Every handler in setupNewPeerConnection (and the merged onconnectionstatechange handler in setupPeerConnection) caught errors but only logged them when globalThis.window was set, silently discarding them in any non-browser environment. logger.warn has no browser dependency (just console + EventEmitter), so there was nothing this guard was protecting against - it just meant every error here vanished with zero trace outside a browser. Log unconditionally. Also switched retryOffer in retryIceOnFailed to an async/await function with braces instead of an implicit-return arrow expression, matching the project's style. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
requestOutboundConnection, hangupConnection, acceptStream, declineStream, offerSdp, and answerSdp only logged the outgoing call. If the gateway rejected the RPC, the rejection propagated with no trace in this SDK's own logs - silent unless the calling application happened to catch and log it itself. Log a warning on rejection and rethrow, so callers still see the same rejected promise. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
smoghe-bw
changed the title
… methods requestOutboundConnection, hangupConnection, acceptStream, declineStream, offerSdp, and answerSdp were wrapped in async/try-catch+rethrow, out of scope for this PR (signaling reconnect + ICE restart retry) and a regression: wrapping a plain `this.ws?.call(...)` passthrough in `async` means `await undefined` (when `this.ws` is null) resolves silently instead of leaving the caller with a non-Promise value that blows up immediately on `.then()`. Reverts to the direct passthrough. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
smoghe-bw
changed the title
Main's VAPI-3929 (republish-on-reconnect) landed in the meantime with its own republishStreams() - a more complete version of this branch's manual re-publish logic (it also retains codecPreferences and reacquires ended tracks). Reconciled by keeping this branch's explicit isReconnect signal (passed from signaling.ts's reconnect detection) and using it to gate main's republishStreams() instead of the old size-based heuristic, then dropped this branch's now-redundant manual re-publish block. Updated the reconnect-replay tests accordingly to pass isReconnect=true, since main's tests were written against the "any re-init with published streams" heuristic rather than an explicit reconnect signal. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
3 tasks
smoghe-bw
pushed a commit
that referenced
this pull request
Sep 23, 2026
Keeps the test valid once init() republishes only on a reconnect (#18). Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
smoghe-bw
changed the title
… gateway - Disable the client ICE-restart retry again: the gateway rejects client offers until its peer is connected, so a retry on a failed peer can never succeed, and the gateway runs its own ICE restart. - Apply "publish" sdpOffer notifications (the gateway's ICE-restart offer) to the publishing connection under publishMutex and answer them as "publish". Before this every sdpOffer went to the subscribing connection. A missing peerType still means subscribe. - Track a separate publish SDP revision and reset it on every init(). - Stop a stale retry loop once init() has replaced the publishing connection. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
stampercasey
approved these changes
Sep 23, 2026
smoghe-bw
added a commit
that referenced
this pull request
Sep 23, 2026
…isconnect (#21) * fix(v1): harden unpublish against races, unknown ids, and disconnect - Run unpublish's transceiver cleanup and renegotiation under one publishMutex section; do the same for publish's attach + negotiate. - unpublish with ids that match no published stream is now a no-op instead of unpublishing every stream. - Stop local tracks first, wait for the publish peer to be connected, and reject with a clear error if renegotiation still fails. - unpublish after disconnect stops tracks locally instead of throwing. - republishStreams skips streams unpublished mid-reconnect and stops any tracks it reacquired for them. - AudioLevelDetector gains stop(); unpublish releases its AudioContext and sampling interval. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> * test(v1): pass isReconnect in the unpublish-during-reacquire test Keeps the test valid once init() republishes only on a reconnect (#18). Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> * fix(v1): release publishMutex while unpublish waits for the publish peer The wait can last 10 s and would otherwise block publish() and gateway ICE-restart offers. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: smoghe-bw <smoghe-bw> Co-authored-by: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Part of VAPI-4011. Makes the SDK survive a signaling reconnect and a publish-peer ICE failure.
Reconnect
rpc-websocketsreconnects with a new WebSocket and firesopenagain.signaling.tsnow passesisReconnecton theinitevent and registersbeforeunloadonly once.init()resets the subscribe SDP revision and the pending track metadata, rebuilds both peer connections, and republishes every published stream. Without the revision reset, the new gateway handler's first subscribe offers (revision 1, 2, …) were dropped as "outdated".onconnectionstatechangehandlers are merged. Thedisconnectedlog was dead code because the second assignment overwrote the first.ICE restart: the gateway owns it
RETRY_ICE_ON_FAILEDstaysfalse. The gateway rejects client offers until its peer isconnected, so a client retry on a failed peer cannot succeed. The retry code is kept, runs on the publish peer only, catches offer errors, and stops onceinit()has replaced the connection.sdpOffernotification withpeerType: "publish". Before this PR the SDK applied everysdpOfferto the subscribing connection. NowhandleSdpOffersendspublishoffers to the publishing connection, underpublishMutex, and answers withanswerSdp(sdp, "publish"). A missingpeerTypestill means subscribe, for older gateways.init(), because every new gateway connection starts its counter at 0.Test plan
npx tsc --noEmitcleannpm test: 113 tests pass (123 when merged with VAPI-4011 fix(v1): harden unpublish against races, unknown ids, and disconnect #21)🤖 Generated with Claude Code