feat(client): expose blocked autoplay audio state and explicit resume API#2187
feat(client): expose blocked autoplay audio state and explicit resume API#2187
Conversation
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughDynascaleManager now detects audio playback failures caused by browser autoplay policies, exposes an observable Changes
Sequence DiagramsequenceDiagram
participant App as Application
participant DM as DynascaleManager
participant Audio as Audio Element
participant Browser as Browser Autoplay Policy
App->>DM: bindAudioElement(element, source)
DM->>Audio: set srcObject = source
DM->>Audio: play()
Audio->>Browser: request playback
Browser-->>Audio: NotAllowedError (autoplay blocked)
Audio-->>DM: Promise rejection
DM->>DM: addBlockedAudioElement(element)
DM-->>App: autoplayBlocked$ = true
Note over App: after user interaction
App->>DM: resumeAudio()
DM->>Audio: play() for blocked elements
Audio->>Browser: request playback
Browser-->>Audio: success
Audio-->>DM: Promise resolved
DM->>DM: removeBlockedAudioElement(element)
DM-->>App: autoplayBlocked$ = false
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
There was a problem hiding this comment.
🧹 Nitpick comments (2)
packages/client/src/helpers/DynascaleManager.ts (1)
682-707: Consider handling errors more granularly inresumeAudio().The current implementation catches all errors and adds the element back to the blocked set. However, errors other than
NotAllowedError(e.g.,AbortErrorfrom rapid play/pause sequences) may not indicate the element is truly blocked by autoplay policy.♻️ Suggested refinement to only re-block on NotAllowedError
resumeAudio = async () => { const blocked = new Set<HTMLAudioElement>(); await Promise.all( Array.from( getCurrentValue(this.blockedAudioElementsSubject), async (el) => { try { if (el.srcObject) { await el.play(); } - } catch { - this.logger.warn(`Can't resume audio for element: `, el); - blocked.add(el); + } catch (e) { + this.logger.warn(`Can't resume audio for element: `, el, e); + // Only re-add if still blocked by autoplay policy + if (e instanceof DOMException && e.name === 'NotAllowedError') { + blocked.add(el); + } } }, ), ); setCurrentValue(this.blockedAudioElementsSubject, blocked); };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/client/src/helpers/DynascaleManager.ts` around lines 682 - 707, In resumeAudio(), don't treat all exceptions as autoplay-blocked: capture the caught error (e.g., const err = e) inside the async loop in resumeAudio and only add the element back into blockedAudioElementsSubject when the error indicates an autoplay block (e.g., err.name === 'NotAllowedError'); for other errors (like 'AbortError') log at debug/trace with this.logger.debug/trace and do not re-block the element; update references to blockedAudioElementsSubject, resumeAudio, and this.logger.warn accordingly so only true NotAllowedError cases are retained in the blocked set.packages/react-bindings/src/hooks/callStateHooks.ts (1)
499-509: Consider adding defensive handling for missing call context.The hook uses
useCall() as Callwithout checking ifcallis defined. While other hooks in this file follow the same pattern, calling this hook outside a<StreamCall>provider will throw when accessingcall.dynascaleManager. Consider returning a safe default value whencallis undefined, similar to howuseCallState()returns an emptyCallState.♻️ Suggested defensive null check
export const useIsAutoplayBlocked = (): boolean => { - const call = useCall() as Call; - return useObservableValue(call.dynascaleManager.autoplayBlocked$); + const call = useCall(); + const autoplayBlocked$ = useMemo( + () => call?.dynascaleManager.autoplayBlocked$ ?? of(false), + [call], + ); + return useObservableValue(autoplayBlocked$); };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/react-bindings/src/hooks/callStateHooks.ts` around lines 499 - 509, The hook useIsAutoplayBlocked assumes useCall() returns a Call and directly accesses call.dynascaleManager.autoplayBlocked$, which will throw if called outside the provider; update useIsAutoplayBlocked to defensively check for a missing call (useCall() result) and return a safe default (e.g., false) when call is undefined, otherwise return useObservableValue(call.dynascaleManager.autoplayBlocked$); keep the function signature the same and reference useIsAutoplayBlocked and call.dynascaleManager.autoplayBlocked$ when making the change.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@packages/client/src/helpers/DynascaleManager.ts`:
- Around line 682-707: In resumeAudio(), don't treat all exceptions as
autoplay-blocked: capture the caught error (e.g., const err = e) inside the
async loop in resumeAudio and only add the element back into
blockedAudioElementsSubject when the error indicates an autoplay block (e.g.,
err.name === 'NotAllowedError'); for other errors (like 'AbortError') log at
debug/trace with this.logger.debug/trace and do not re-block the element; update
references to blockedAudioElementsSubject, resumeAudio, and this.logger.warn
accordingly so only true NotAllowedError cases are retained in the blocked set.
In `@packages/react-bindings/src/hooks/callStateHooks.ts`:
- Around line 499-509: The hook useIsAutoplayBlocked assumes useCall() returns a
Call and directly accesses call.dynascaleManager.autoplayBlocked$, which will
throw if called outside the provider; update useIsAutoplayBlocked to defensively
check for a missing call (useCall() result) and return a safe default (e.g.,
false) when call is undefined, otherwise return
useObservableValue(call.dynascaleManager.autoplayBlocked$); keep the function
signature the same and reference useIsAutoplayBlocked and
call.dynascaleManager.autoplayBlocked$ when making the change.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 213e9f00-f25c-4d6c-89f7-cab06fd9f878
📒 Files selected for processing (4)
packages/client/src/Call.tspackages/client/src/helpers/DynascaleManager.tspackages/client/src/helpers/__tests__/DynascaleManager.test.tspackages/react-bindings/src/hooks/callStateHooks.ts
💡 Overview
This PR introduces a way to recover when incoming audio is blocked by browser autoplay rules. It tracks blocked audio, exposes that state to consumers, and adds
call.resumeAudio()so they can retry playback after a user interaction. It also includes a test for the case where the audio stream is removed before playback resumes.📝 Implementation notes
call.resumeAudio()for explicit retry from a user gestureuseIsAutoplayBlocked()from call state hooks to detect blocked incoming audio🎫 Ticket: https://linear.app/stream/issue/REACT-941/audio-autoplay-policy-observable
📑 Docs: https://github.com/GetStream/docs-content/pull/1155
Summary by CodeRabbit
New Features
Tests