diff --git a/js/publish/src/source/camera.ts b/js/publish/src/source/camera.ts index cd0e8601f3..bdccdb6dfb 100644 --- a/js/publish/src/source/camera.ts +++ b/js/publish/src/source/camera.ts @@ -124,8 +124,8 @@ export class Camera { stream = await effect.race(media); } catch (error) { if (effect.abort.aborted) return; + if (this.#retry.rejected(error)) return; this.#out.error.set(error instanceof Error ? error : new Error(String(error))); - this.#retry.terminal(); return; } @@ -138,7 +138,10 @@ export class Camera { effect.cleanup(this.device.capture(source?.getSettings().deviceId)); // A track that arrives dead already fired "ended", so nothing would ever rerun us. - if (!source || source.readyState === "ended") return this.#retry.failed(); + if (!source || source.readyState === "ended") { + this.#retry.failed(); + return; + } this.#retry.succeeded(effect, source); effect.set(this.#out.source, { video: source }); diff --git a/js/publish/src/source/microphone.ts b/js/publish/src/source/microphone.ts index 45258f294c..0cce86ed1d 100644 --- a/js/publish/src/source/microphone.ts +++ b/js/publish/src/source/microphone.ts @@ -110,8 +110,8 @@ export class Microphone { stream = await effect.race(media); } catch (error) { if (effect.abort.aborted) return; + if (this.#retry.rejected(error)) return; this.#out.error.set(error instanceof Error ? error : new Error(String(error))); - this.#retry.terminal(); return; } @@ -125,7 +125,10 @@ export class Microphone { effect.cleanup(this.device.capture(settings?.deviceId)); // A track that arrives dead already fired "ended", so nothing would ever rerun us. - if (!track || track.readyState === "ended") return this.#retry.failed(); + if (!track || track.readyState === "ended") { + this.#retry.failed(); + return; + } this.#retry.succeeded(effect, track); effect.set(this.#out.source, { audio: { track, kind: "voice" } }); diff --git a/js/publish/src/source/retry.test.ts b/js/publish/src/source/retry.test.ts index 1eae4564b8..7f30c766c8 100644 --- a/js/publish/src/source/retry.test.ts +++ b/js/publish/src/source/retry.test.ts @@ -1,4 +1,4 @@ -import { expect, spyOn, test } from "bun:test"; +import { expect, jest, spyOn, test } from "bun:test"; import { Signal } from "@moq/signals"; import { Camera } from "./camera"; import { Microphone } from "./microphone"; @@ -30,6 +30,7 @@ class FakeTrack extends EventTarget { class FakeMediaDevices extends EventTarget { tracks: FakeTrack[] = []; + errors: Error[] = []; // getUserMedia rejects, the way it does once the last device is gone. missing = false; @@ -51,6 +52,8 @@ class FakeMediaDevices extends EventTarget { async getUserMedia(): Promise { this.attempts += 1; + const error = this.errors.shift(); + if (error) throw error; if (this.denied) throw new DOMException("Permission denied", "NotAllowedError"); if (this.missing) throw new Error("NotFoundError"); @@ -177,10 +180,168 @@ function published(media: Media | undefined): unknown { return "track" in source ? source.track : source; } +function fakeTime(): Disposable { + jest.useFakeTimers(); + return { [Symbol.dispose]: () => jest.useRealTimers() }; +} + for (const [kind, create] of [ ["camera", (enabled: Signal) => new Camera({ enabled })], ["microphone", (enabled: Signal) => new Microphone({ enabled })], ] as const) { + for (const name of ["NotReadableError", "AbortError"]) { + test(`${kind} retries ${name} without reporting a terminal error`, async () => { + using _time = fakeTime(); + using media = install(new FakeMediaDevices()); + const enabled = new Signal(false); + const source = create(enabled); + try { + // Settle discovery before starting so it cannot refund this attempt's budget. + await settle(); + media.errors = [new DOMException("temporarily unavailable", name)]; + enabled.set(true); + await settle(); + expect(media.attempts).toBe(1); + expect(source.out.error.peek()).toBeUndefined(); + expect(source.out.source.peek()).toBeUndefined(); + + jest.advanceTimersByTime(Retry.DELAY.initial); + await settle(); + expect(media.attempts).toBe(2); + expect(published(source.out.source.peek())).toBe(media.latest()); + expect(source.out.error.peek()).toBeUndefined(); + } finally { + source.close(); + } + }); + + test(`${kind} retains ${name} after exhausting the retry budget`, async () => { + using _time = fakeTime(); + using media = install(new FakeMediaDevices()); + const enabled = new Signal(false); + const source = create(enabled); + try { + await settle(); + const errors = Array.from( + { length: Retry.LIMIT + 1 }, + (_, i) => new DOMException(`failure ${i}`, name), + ); + media.errors = [...errors]; + enabled.set(true); + await settle(); + for (let i = 0; i < Retry.LIMIT; i++) { + expect(media.attempts).toBe(i + 1); + expect(source.out.error.peek()).toBeUndefined(); + jest.advanceTimersToNextTimer(); + await settle(); + } + expect(media.attempts).toBe(Retry.LIMIT + 1); + expect(source.out.source.peek()).toBeUndefined(); + expect(source.out.error.peek()).toBe(errors.at(-1)); + jest.advanceTimersByTime(Retry.SETTLED); + await settle(); + expect(media.attempts).toBe(Retry.LIMIT + 1); + expect(source.out.error.peek()).toBe(errors.at(-1)); + + enabled.set(false); + await settle(); + expect(source.out.error.peek()).toBeUndefined(); + enabled.set(true); + await settle(); + expect(media.attempts).toBe(Retry.LIMIT + 2); + expect(published(source.out.source.peek())).toBe(media.latest()); + } finally { + source.close(); + } + }); + } + + for (const error of [ + new DOMException("denied", "NotAllowedError"), + new TypeError("bad constraints"), + new Error("unknown"), + ]) { + test(`${kind} does not retry ${error.name}`, async () => { + using _time = fakeTime(); + using media = install(new FakeMediaDevices()); + const enabled = new Signal(false); + const source = create(enabled); + try { + await settle(); + media.errors = [error]; + enabled.set(true); + await settle(); + expect(source.out.error.peek()).toBe(error); + jest.advanceTimersByTime(Retry.SETTLED); + await settle(); + expect(media.attempts).toBe(1); + expect(source.out.source.peek()).toBeUndefined(); + expect(source.out.error.peek()).toBe(error); + } finally { + source.close(); + } + }); + } + + for (const close of [false, true]) { + test(`${kind} cancels a pending retry when ${close ? "closed" : "disabled"}`, async () => { + using _time = fakeTime(); + using media = install(new FakeMediaDevices()); + const enabled = new Signal(false); + const source = create(enabled); + try { + await settle(); + media.errors = [new DOMException("busy", "NotReadableError")]; + enabled.set(true); + await settle(); + expect(media.attempts).toBe(1); + expect(source.out.error.peek()).toBeUndefined(); + if (close) source.close(); + else enabled.set(false); + await settle(); + jest.advanceTimersByTime(Retry.SETTLED); + await settle(); + expect(media.attempts).toBe(1); + expect(source.out.source.peek()).toBeUndefined(); + expect(source.out.error.peek()).toBeUndefined(); + } finally { + source.close(); + } + }); + } + + for (const reject of [false, true]) { + test(`${kind} ignores a ${reject ? "rejection" : "stream"} arriving after cancellation`, async () => { + using _time = fakeTime(); + using media = install(new FakeMediaDevices()); + const pending = Promise.withResolvers(); + const open = spyOn(media, "getUserMedia").mockImplementationOnce(() => pending.promise); + const enabled = new Signal(false); + const source = create(enabled); + try { + await settle(); + enabled.set(true); + await settle(); + expect(open).toHaveBeenCalledTimes(1); + enabled.set(false); + await settle(); + const track = new FakeTrack(); + if (reject) pending.reject(new DOMException("aborted", "AbortError")); + else pending.resolve({ getTracks: () => [track] } as unknown as MediaStream); + await settle(); + if (!reject) expect(track.stopped).toBe(true); + jest.advanceTimersByTime(Retry.SETTLED); + await settle(); + expect(open).toHaveBeenCalledTimes(1); + expect(source.out.error.peek()).toBeUndefined(); + expect(source.out.source.peek()).toBeUndefined(); + } finally { + source.close(); + open.mockRestore(); + } + }); + } + test(`${kind} reports a refusal and stops retrying until capture is reset`, async () => { using media = install(new FakeMediaDevices()); media.denied = true; diff --git a/js/publish/src/source/retry.ts b/js/publish/src/source/retry.ts index 50cc045973..7475afd85f 100644 --- a/js/publish/src/source/retry.ts +++ b/js/publish/src/source/retry.ts @@ -4,8 +4,8 @@ import { type Effect, Signal } from "@moq/signals"; * A budget for re-opening a `getUserMedia` capture that failed or died. * * A `MediaStreamTrack` ends when the device disappears, the OS revokes it, or another application - * takes an exclusive device, and the reopen that follows can itself fail. Both spend budget, so a - * device that never works stops being asked. + * takes an exclusive device, and the reopen that follows can itself fail. Transient open failures + * and ended tracks spend budget; other open failures are terminal immediately. * * Every outcome reruns the owning effect, including running out of budget. That rerun is what clears * `out.source` and stops the stream, via the cleanup the previous run registered, so no caller has to @@ -69,14 +69,29 @@ export class Retry { return true; } - /** The attempt produced no usable track. Spends budget and reruns the effect. */ - failed(): void { + /** Spends budget and reruns the effect, returning whether another attempt is allowed. */ + failed(): boolean { this.#failures += 1; - // Unlimited budget, so there is always a next delay. + const retry = this.#failures <= Retry.LIMIT; // Equal jitter, so a page with several captures doesn't reopen them all on the same tick. - this.#wait = this.#delay * (0.5 + Math.random() / 2); + this.#wait = retry ? this.#delay * (0.5 + Math.random() / 2) : undefined; this.#delay = Math.min(this.#delay * Retry.DELAY.multiplier, Retry.DELAY.max); this.#rerun.update((rerun) => rerun + 1); + return retry; + } + + /** + * Classify a `getUserMedia` rejection, returning whether another attempt is allowed. + * + * Only a busy device (`NotReadableError`) or an aborted request (`AbortError`) spends budget; + * anything else, including bad constraints, is terminal at once. + */ + rejected(error: unknown): boolean { + if (error instanceof Error && (error.name === "NotReadableError" || error.name === "AbortError")) { + return this.failed(); + } + this.terminal(); + return false; } /** Stop attempting this capture until its settings, device list, or permission changes. */ diff --git a/quest/m1/README.md b/quest/m1/README.md index 1a17f595ea..ed4231c04c 100644 --- a/quest/m1/README.md +++ b/quest/m1/README.md @@ -49,7 +49,6 @@ QUIC studies there on that rule. - [Audio graph lifetime](/quest/m1/watch-audio-graph.md) - the watch audio graph survives a rendition leaving the catalog, so a publisher mute keeps the tail and the next start - [Audio publish hygiene](/quest/m1/audio-publish-hygiene.md) - JS audio uses the shared container producer, so any stop ends its epoch, and volume ramps over a configurable `fade` - [LIFO cleanups](/quest/m1/signals-lifo.md) - signals effects clean up last-in, first-out, so nested teardown runs first -- [Transient capture errors](/quest/m1/capture-transient-errors.md) - a busy or aborted getUserMedia spends the retry budget; every other rejection stays terminal - [Still screen share](/quest/m1/capture-still-frame.md) - video capture keeps its latest frame and replays it, re-stamped, to late subscribers - [SUBSCRIBE_DROP](/quest/m1/subscribe-drop.md) - every stream group in a lite subscription arrives or is dropped by name, and lite-07 drops its stream count for it - [Subscribe ranges](/quest/m1/subscribe-ranges/README.md) - a lite-07 SUBSCRIBE asks for past and live ranges in either order and replaces FETCH; relays fill misses by range, including over moq-transport diff --git a/quest/m1/capture-transient-errors.md b/quest/m1/capture-transient-errors.md deleted file mode 100644 index 05094d55d8..0000000000 --- a/quest/m1/capture-transient-errors.md +++ /dev/null @@ -1,28 +0,0 @@ -# [S] A busy or aborted getUserMedia spends the retry budget - -## Goal - -A camera or microphone that fails to open because the device is busy -(`NotReadableError`) or the request was aborted (`AbortError`) retries within -the existing attempt budget. Every other rejection is terminal at once. - -## Plan - -Since #3934, `js/publish/src/source/camera.ts` and -`js/publish/src/source/microphone.ts` call `retry.terminal()` for every -rejection, so a device-switch race is never retried, and the doc in -`js/publish/src/source/retry.ts` is stale. - -Decided (2026-10-04): an allowlist, not a denylist. Only `NotReadableError` -and `AbortError` call `retry.failed()`; anything else, including `TypeError` -from bad constraints, stays terminal (fail loud). `out.error` stays unset -while retrying and is set once the budget is spent. This spends the budget -that already exists and adds no new retry or timeout. Rust's capture makes -the same split (`Failure::retry` and `fatal`). Fix the `retry.ts` doc. - -Tests: a mocked `NotReadableError` followed by success opens the device; a -`NotAllowedError` or `TypeError` is terminal on the first attempt. - -## Closes - -- [#4789](https://github.com/moq-dev/moq/issues/4789) - close this issue when the quest finishes