fix(publish): retry transient capture failures within the existing budget

[shermerL]

Problem

Camera and microphone sources treat every getUserMedia rejection as terminal, so a temporary device-opening failure cannot recover within the retry budget already used for ended tracks.

Approach

Allow only NotReadableError and AbortError to spend the existing retry budget, classified once in Retry.rejected and shared by both sources. Keep out.error unset while retrying and retain the last error when the budget is exhausted. Preserve immediate failure for all other errors and the existing cancellation and late-stream cleanup paths.

Add fake-timer regressions for both sources covering recovery, exhaustion, reset, terminal errors, cancellation during backoff, and late results.

Impact

• Public API shape and wire format: unchanged.
• Transient opening failures now retry within the existing limit and backoff before reporting a terminal error.
• Screen capture and native capture are unchanged.

Validation

• Both NotReadableError recovery regressions fail without the fix.
• All 192 publish tests pass, including 22 new cases; JS type checks, lint and builds pass with flake-pinned tools.
• Root just check stops at pre-existing TOML formatting errors in two ignored local scratch projects.
• No real-browser or physical-device validation was performed.

Alternatives

Keep the existing Retry budget instead of adding an application retry loop, timeout, or relaxed device constraints. Permission and malformed-input failures remain terminal.

Follow-ups

• Completes and deletes quest/m1/capture-transient-errors.md.
• Pre-existing: an ended track that exhausts the budget still sets no out.error, so the failure is silent.

Fixes #4789.

[kixelated]

Automated review by review (OpenAI)

Reviewed commit: ee99d41

No actionable introduced bugs found in the four-file diff and surrounding capture, device, and signal lifecycle code.

Direction: sound and focused. The allowlist in js/publish/src/source/camera.ts:125–134 and microphone.ts:111–120 preserves terminal handling for other failures; retry.ts:73–82 spends the existing shared budget and retains the final transient error on exhaustion. The new fake-timer cases in retry.test.ts:193–343 cover recovery, exhaustion/reset, terminal failures, backoff cancellation, and late results. No public API or wire-format change identified.

Verification limits: static review only; I did not run tests, type checks, builds, or browser/physical-device checks, and did not independently verify the PR's reported validation results.

(Written by OpenAI)

[coderabbitai]

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

• Configuration used: Organization UI
• Review profile: CHILL
• Plan: Advanced
• Run ID: d0ccfeb9-dcfb-4986-aef3-3ef414573f2b

📥 Commits

Reviewing files that changed from the base of the PR and between 2704e10 and ee99d41.

📒 Files selected for processing (4)

• js/publish/src/source/camera.ts
• js/publish/src/source/microphone.ts
• js/publish/src/source/retry.test.ts
• js/publish/src/source/retry.ts

Included review availability: This review used your included allowance. Your plan provides up to 4 included reviews per hour; 3 remain after this review.

---

Walkthrough

Retry.failed() now reports whether the retry limit allows another attempt. Camera and microphone sources use the retry budget for NotReadableError, AbortError, and missing or ended tracks. Other capture errors remain terminal. Tests cover retry limits, re-enabling, terminal errors, and cancellation of pending or unresolved captures.

Priority: ➖ Normal

Severity of issue fixed: Medium

Merge Risk: ⚪ Minimal · up to ee99d

Camera and microphone capture retry transient failures within the configured budget without an identified merge-blocking issue.

Security Architecture Review

Security architecture risk: ⚪ Minimal · up to ee99d

The additional capture attempts remain bounded and use the existing device selection and browser permission checks. Permission failures remain terminal, and cancellation retains cleanup for late-arriving streams. No material security regression was identified in the changed behavior.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

• inferred — The changed exposure is additional browser-local camera or microphone acquisition attempts while the existing source remains enabled. Caller-controlled settings still reach the same getUserMedia operation and existing source output; the change does not introduce an additional privileged sink or cross-service path in these capture blocks.

Trust Boundaries and Controls

• inferred — Browser-mediated capture remains the authority boundary. The new branch retries only two named transient errors and does not relax device constraints or convert permission denial into automatic retry. NotAllowedError and malformed-input failures continue through terminal handling for the current budget cycle.

Resilience and Maintainability Implications

• observed — The existing teardown mechanism protects media ownership during retries: reruns abort and clean the previous scope before opening another, timers are canceled on teardown, and late streams are stopped without publication. Added tests assert recovery, final-error retention, terminal failures, cancellation during backoff, and late resolve/reject behavior for both source types. These are inspected assertions, not independently executed validation.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 2 functions across 4 files.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Linked Issues check	✅ Passed	[#4789] Camera and Microphone now send NotReadableError and AbortError through Retry.failed(). They leave out.error unset while retries remain and record the last error when the retry budget is exhaus…
Out of Scope Changes check	✅ Passed	The changes are limited to Camera, Microphone, Retry, and Retry tests. The Retry.failed() budget handling and tests support [#4789] by bounding retries and verifying exhaustion, reset, and cancellatio…
Title check	✅ Passed	The title clearly and concisely describes the main change: retrying transient capture failures within the existing budget.
Description check	✅ Passed	The description explains the problem, retry behavior, scope, tests, and validation. It is directly related to the changeset.

• Fix all pre-merge checks with AI

✨ Finishing Touches

✨ Simplify code

• Create a new PR

• Autopilot · 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

• X
• Mastodon
• Reddit
• LinkedIn

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

[kixelated]

Automated review: MERGE on head ee99d41b255597bd2c187d47323bd7001ea031d1

This matches the quest decision exactly: only NotReadableError and AbortError spend the existing Retry budget, everything else stays terminal, out.error stays unset while retrying, and the last error is kept once the budget runs out. The ordering works. failed() reruns the effect, begin() returns false once #failures > LIMIT, and the error set after it isn't cleared. The cancellation and late-stream tests cover the paths that matter. CI is green.

Non-blocking

1. The quest isn't completed in-tree. quest/m1/capture-transient-errors.md still exists, and quest/m1/README.md:52 still lists it. Other completion PRs (e.g. fix(hang)!: CMAF decoders time samples from the frame timestamp #4826) delete the quest file and its README line. The quest: claim commit (d2e57d5) adds no file changes, so after merge the quest still looks open while publish: since #3934 every getUserMedia rejection is terminal, so a transient NotReadableError (device busy) is never retried, although Retry still documents that a failed reopen spends budget #4789 auto-closes.
2. When an ended track exhausts the budget, it still sets no error. That's the succeeded → ended → failed() path, and the !source || readyState === "ended" branch at camera.ts:145. It predates this PR, but now a busy device that never opens reports an error while a device that opens and keeps dying fails silently. Worth setting out.error there too, or noting it as a follow-up.
3. The transient-name predicate is copied in camera.ts:127 and microphone.ts:113. A Retry.transient(error) helper would keep the two allowlists from drifting apart.

Verdict: MERGE on head ee99d41.

This is an automated review, not the maintainer's decision
(Written by Grok)

[kixelated]

Merging. Summary of what landed on top of @shermerL's fix:

• Moved the NotReadableError/AbortError allowlist into Retry.rejected(error) so camera and microphone share one classifier (behavior unchanged; all 192 publish tests pass).
• Completed the quest: deleted quest/m1/capture-transient-errors.md and its quest/m1/README.md entry.

Decisions: the follow-up commit is a behavior-preserving refactor plus quest cleanup, so the earlier review of ee99d41 stands.

Follow-up, pre-existing and out of scope: an ended track that exhausts the budget still leaves out.error unset, so the failure is silent.

(Written by Claude Opus 5.5)