Repository navigation
fix(media): keep a publication Cloudflare lists as active through propagation lag - #792
Conversation
…pagation lag Listener pulls answer "track not found" for several seconds after a track turns active. The liveness sweep treated 2 s of such failures as a stuck track and closed the publication, so every listener lost the speaker until the speaker toggled the mic. Failures on an active track now count only after 10 s. Dead or inactive tracks still close at once. Each skipped closure is counted (chalk.media.publication_closure_skipped) and logged. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
realhasanshoaib
added
risk:medium
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
| func TestConfirmRemoteTrackFailuresKeepsActiveTrackThroughPropagationLag(t *testing.T) { | ||
| now := time.Date(2026, 8, 25, 12, 0, 0, 0, time.UTC) | ||
| repository := &reconciliationRepositoryStub{} | ||
| service := newService(repository, func() time.Time { return now }) | ||
| tenantID, episodeID, publisherID, identity := publishRemoteObservationFixture(t, service, repository) | ||
| listeners := []utilities.ID{ | ||
| reconciliationTestID(t, "44444444-4444-4444-8444-444444444444"), | ||
| reconciliationTestID(t, "55555555-5555-4555-8555-555555555555"), | ||
| reconciliationTestID(t, "66666666-6666-4666-8666-666666666666"), | ||
| } | ||
|
|
||
| // Every listener retries its failed pull about every 1.2 s for 6 s. | ||
| for elapsed := time.Duration(0); elapsed <= 6*time.Second; elapsed += 1200 * time.Millisecond { | ||
| now = time.Date(2026, 8, 25, 12, 0, 0, 0, time.UTC).Add(elapsed) | ||
| for _, listenerID := range listeners { | ||
| if result := confirmActiveTrackFailure(t, service, tenantID, episodeID, listenerID, identity); result.Closed != 0 { |
There was a problem hiding this comment.
🟡 Production regression lacks observed failure fixture
The new TestConfirmRemoteTrackFailuresKeepsActiveTrackThroughPropagationLag uses invented camera failures and retry timing, not the observed microphone failure. The required production regression must derive from a sanitized request, trace, or bundle.
Learn more
The repository's review standard requires a production-failure fix to have a regression built from an observed request, retained trace, or sanitized bundle. This test creates a synthetic camera publication and assumes three listeners retry at fixed 1.2-second intervals. It does not encode the reported microphone failure from the rig, so it cannot establish that the observed sequence stays open.
Example: The reported failure closed a new microphone publication after roughly three seconds with seven listeners; this fixture uses a camera publication and three listeners for six seconds.
Recommended fix: Use a sanitized fixture derived from the rig's logged request or retained trace, including the observed track source, failure sequence, and timestamps. Assert that the observed sequence keeps the publication enabled, alongside the existing synthetic coverage.
Was this helpful? React with 👍 or 👎 to provide feedback.
| publicationClosuresSkipped.Add(ctx, 1) | ||
| slog.InfoContext(ctx, "kept media publication that Cloudflare lists as active", | ||
| "event", "media_publication.publisher_track_close_skipped", | ||
| "listener_failing_ms", failingFor.Milliseconds(), |
There was a problem hiding this comment.
📝 Info: Skipped-closure count mixes distinct decisions
publicationClosuresSkipped counts every retained active track, including reports from departed listeners and tracks successfully served to others. It cannot isolate propagation-lag cases without a reason breakdown.
Was this helpful? React with 👍 or 👎 to provide feedback.
In a rig with an impaired network, a speaker's new microphone track was closed by the API about 3 s after it was published, so all 7 listeners never heard that speaker until the speaker toggled the mic by hand. This was the only closure in 262 rooms. The server does it, so every SDK version is hit, including 5.2.6.
The API checks Cloudflare for the publisher track before closing, and Cloudflare did list it as active. The close came from a second rule: if listeners' pulls of an active track keep failing for 2 s, treat the track as stuck and withdraw it so the speaker republishes. But Cloudflare answers pulls with
not_found_track_errorfor several seconds (4.5 s in the rig) after a track turns active, so a healthy track hit that rule.Fix
apps/api/internal/mediapublications/liveness.go, listener failures on an active track count only after 10 s (publicationActiveTrackGrace), up from 2 s. A track that is still stuck after 10 s is withdrawn as before.chalk.media.publication_closure_skippedand logsmedia_publication.publisher_track_close_skippedwithlistener_failing_ms, at most once per 2 s per publication.I chose a longer failure window over a minimum publication age because the failure timer already starts at the first failed pull, which follows the publish within milliseconds. It also needs no new data and still lets a late-joining listener trigger healing on an old stuck track.
Proof
TestConfirmRemoteTrackFailuresKeepsActiveTrackThroughPropagationLagfeeds three listeners failing every 1.2 s for 6 s against an active track. It fails on master (closed at 2.4 s) and passes now.TestConfirmRemoteTrackFailuresClosesOnlyGoneOrInactiveTracksstill passes: missing, inactive and session-not-found tracks close on the first check.TestConfirmRemoteTrackFailuresWithdrawsActiveTrackListenersStillFailand the withdrawal table test now wait for the 10 s window and still withdraw a stuck track.Needs you
publicationActiveTrackGraceto tune it.Undo: revert this commit; it changes one constant's use and adds a counter.
Created with Claude Sonnet 5.5 in Claude Code.