From 49398094e8927889dc0fa9f39b8a933bbf095dc6 Mon Sep 17 00:00:00 2001 From: oxoxDev <164490987+oxoxDev@users.noreply.github.com> Date: Mon, 11 May 2026 21:59:47 +0530 Subject: [PATCH] fix(human/mascot): consume orphan audio-stopped rejections (#1472) (#1494) --- .../human/useHumanMascot.lipsync.test.ts | 11 ++- app/src/features/human/useHumanMascot.test.ts | 87 ++++++++++++++++++- app/src/features/human/useHumanMascot.ts | 30 +++++-- .../features/human/voice/audioPlayer.test.ts | 63 +++++++++++++- app/src/features/human/voice/audioPlayer.ts | 31 ++++++- 5 files changed, 208 insertions(+), 14 deletions(-) diff --git a/app/src/features/human/useHumanMascot.lipsync.test.ts b/app/src/features/human/useHumanMascot.lipsync.test.ts index f820a100f..6d012553a 100644 --- a/app/src/features/human/useHumanMascot.lipsync.test.ts +++ b/app/src/features/human/useHumanMascot.lipsync.test.ts @@ -33,7 +33,16 @@ vi.mock('./voice/ttsClient', async () => { return { ...actual, synthesizeSpeech: vi.fn() }; }); -vi.mock('./voice/audioPlayer', () => ({ playBase64Audio: vi.fn() })); +vi.mock('./voice/audioPlayer', () => ({ + playBase64Audio: vi.fn(), + // Hook's orphan `.catch(swallowAudioStop)` wiring runs in cleanup paths + // exercised here — mirror the real helper so stop sentinels are silenced. + swallowAudioStop: (err: unknown) => { + if (typeof err === 'object' && err !== null && (err as { stopped?: unknown }).stopped === true) + return; + throw err; + }, +})); let capturedListeners: ChatEventListeners | null = null; diff --git a/app/src/features/human/useHumanMascot.test.ts b/app/src/features/human/useHumanMascot.test.ts index 5936b9d13..fed6fd4ab 100644 --- a/app/src/features/human/useHumanMascot.test.ts +++ b/app/src/features/human/useHumanMascot.test.ts @@ -4,7 +4,7 @@ import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; import type { ChatEventListeners } from '../../services/chatService'; import { VISEMES } from './Mascot/visemes'; import { ACK_FACE_HOLD_MS, pickViseme, useHumanMascot } from './useHumanMascot'; -import { playBase64Audio } from './voice/audioPlayer'; +import { type PlaybackHandle, playBase64Audio } from './voice/audioPlayer'; import { synthesizeSpeech } from './voice/ttsClient'; vi.mock('../../services/chatService', () => ({ @@ -30,7 +30,25 @@ vi.mock('./voice/ttsClient', () => ({ proceduralVisemes: (text: string, durationMs: number) => proceduralVisemesMock(text, durationMs), })); -vi.mock('./voice/audioPlayer', () => ({ playBase64Audio: vi.fn() })); +class FakeAudioStoppedError extends Error { + readonly stopped = true; + constructor() { + super('stopped'); + this.name = 'AudioStoppedError'; + } +} + +vi.mock('./voice/audioPlayer', () => ({ + playBase64Audio: vi.fn(), + // Mirror the real helper so the hook's orphan `.catch(swallowAudioStop)` + // wiring actually executes — otherwise stop sentinels would slip through + // as unhandledrejections under test and the regression coverage is moot. + swallowAudioStop: (err: unknown) => { + if (typeof err === 'object' && err !== null && (err as { stopped?: unknown }).stopped === true) + return; + throw err; + }, +})); function makeFakePlayback(durationMs = 100) { let stopped = false; @@ -47,7 +65,7 @@ function makeFakePlayback(durationMs = 100) { metadataReady: Promise.resolve(), stop: () => { stopped = true; - rejectEnded(new Error('stopped')); + rejectEnded(new FakeAudioStoppedError()); }, ended, }, @@ -402,6 +420,69 @@ describe('useHumanMascot TTS playback', () => { }); }); + it('does not surface an unhandledrejection when a newer turn cancels in-flight playback (#1472)', async () => { + // Two back-to-back turns: the first reaches the `await playBase64Audio` + // point and then a second onDone bumps the playback seq. When the first + // play() finally resolves, the hook takes the `!isStillCurrent()` branch + // and calls `handle.stop()` + early-returns. Before the fix, that left + // the resulting `handle.ended` rejection un-attached → unhandledrejection + // → Sentry. The fix attaches `.catch(swallowAudioStop)` at each such site. + vi.useRealTimers(); + const fake1 = makeFakePlayback(); + const fake2 = makeFakePlayback(); + let resolveFirstPlay!: (h: PlaybackHandle) => void; + const firstPlay = new Promise(r => { + resolveFirstPlay = r; + }); + (synthesizeSpeech as ReturnType) + .mockResolvedValueOnce({ + audio_base64: 'AAA=', + audio_mime: 'audio/mpeg', + visemes: [{ viseme: 'aa', start_ms: 0, end_ms: 100 }], + }) + .mockResolvedValueOnce({ + audio_base64: 'BBB=', + audio_mime: 'audio/mpeg', + visemes: [{ viseme: 'aa', start_ms: 0, end_ms: 100 }], + }); + (playBase64Audio as ReturnType) + .mockImplementationOnce(() => firstPlay) + .mockResolvedValueOnce(fake2.handle); + + const unhandled: PromiseRejectionEvent[] = []; + const handler = (e: PromiseRejectionEvent) => unhandled.push(e); + window.addEventListener('unhandledrejection', handler); + try { + renderHook(() => useHumanMascot({ speakReplies: true })); + // Turn 1 enters startTtsPlayback and blocks on playBase64Audio. + await act(async () => { + capturedListeners?.onDone?.(fakeDone('first')); + await Promise.resolve(); + await Promise.resolve(); + }); + // Turn 2 fires, bumps playbackSeqRef, awaits its own (resolved) play. + await act(async () => { + capturedListeners?.onDone?.(fakeDone('second')); + await Promise.resolve(); + await Promise.resolve(); + await Promise.resolve(); + }); + // Now resolve turn-1's play: its handle is stale → hook stops + bails. + await act(async () => { + resolveFirstPlay(fake1.handle as unknown as PlaybackHandle); + await Promise.resolve(); + await Promise.resolve(); + await Promise.resolve(); + // Macrotask hop so jsdom can dispatch any pending unhandledrejection. + await new Promise(r => setTimeout(r, 0)); + }); + expect(unhandled).toHaveLength(0); + } finally { + window.removeEventListener('unhandledrejection', handler); + vi.useFakeTimers(); + } + }); + it('shows concerned (not happy) when synthesizeSpeech rejects', async () => { (synthesizeSpeech as ReturnType).mockRejectedValueOnce(new Error('voice down')); diff --git a/app/src/features/human/useHumanMascot.ts b/app/src/features/human/useHumanMascot.ts index a11f9f579..96b0e420c 100644 --- a/app/src/features/human/useHumanMascot.ts +++ b/app/src/features/human/useHumanMascot.ts @@ -4,7 +4,7 @@ import { useEffect, useRef, useState } from 'react'; import { subscribeChatEvents } from '../../services/chatService'; import type { MascotFace } from './Mascot'; import { lerpViseme, VISEMES, type VisemeShape } from './Mascot/visemes'; -import { type PlaybackHandle, playBase64Audio } from './voice/audioPlayer'; +import { type PlaybackHandle, playBase64Audio, swallowAudioStop } from './voice/audioPlayer'; import { proceduralVisemes, synthesizeSpeech, @@ -187,8 +187,15 @@ export function useHumanMascot(options: UseHumanMascotOptions = {}): UseHumanMas onError: () => { // Bump seq to invalidate any in-flight startTtsPlayback awaiters. playbackSeqRef.current++; - playbackRef.current?.stop(); + const orphan = playbackRef.current; playbackRef.current = null; + if (orphan) { + orphan.stop(); + // We're early-returning instead of awaiting `orphan.ended`, so the + // stop()-sentinel rejection has no handler — attach one explicitly + // or it surfaces as an unhandledrejection in Sentry (#1472). + orphan.ended.catch(swallowAudioStop); + } visemeFramesRef.current = []; holdThenIdle('concerned'); }, @@ -198,16 +205,24 @@ export function useHumanMascot(options: UseHumanMascotOptions = {}): UseHumanMas clearAckTimer(); // Same — invalidate in-flight callbacks before tearing down. playbackSeqRef.current++; - playbackRef.current?.stop(); + const orphan = playbackRef.current; playbackRef.current = null; + if (orphan) { + orphan.stop(); + orphan.ended.catch(swallowAudioStop); + } }; }, []); async function startTtsPlayback(text: string): Promise { // Cancel any in-flight playback so its handle.ended callback can't reset // state belonging to the new run. - playbackRef.current?.stop(); + const prev = playbackRef.current; playbackRef.current = null; + if (prev) { + prev.stop(); + prev.ended.catch(swallowAudioStop); + } visemeFramesRef.current = []; visemeCursorRef.current = 0; clearAckTimer(); @@ -252,6 +267,7 @@ export function useHumanMascot(options: UseHumanMascotOptions = {}): UseHumanMas const handle = await playBase64Audio(tts.audio_base64, tts.audio_mime ?? 'audio/mpeg'); if (!isStillCurrent()) { handle.stop(); + handle.ended.catch(swallowAudioStop); return; } if (frames.length === 0) { @@ -277,8 +293,10 @@ export function useHumanMascot(options: UseHumanMascotOptions = {}): UseHumanMas ); try { await handle.ended; - } catch { - // Promise rejects when stop() is called — fall through to cleanup. + } catch (err) { + // Stop sentinel is expected when a newer turn cancels playback — + // rethrow anything else so real decoder errors aren't masked. + swallowAudioStop(err); } } finally { if (isStillCurrent()) { diff --git a/app/src/features/human/voice/audioPlayer.test.ts b/app/src/features/human/voice/audioPlayer.test.ts index f9cb8fb1b..576a8fa90 100644 --- a/app/src/features/human/voice/audioPlayer.test.ts +++ b/app/src/features/human/voice/audioPlayer.test.ts @@ -1,6 +1,11 @@ import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; -import { playBase64Audio } from './audioPlayer'; +import { + AudioStoppedError, + isAudioStopped, + playBase64Audio, + swallowAudioStop, +} from './audioPlayer'; /** * Minimal HTMLAudioElement stand-in so we can drive metadata loading and @@ -125,7 +130,7 @@ describe('playBase64Audio', () => { expect(playedSynchronously).toBe(true); }); - it('stop() pauses, cleans up the blob URL, and rejects ended', async () => { + it('stop() pauses, cleans up the blob URL, and rejects ended with AudioStoppedError', async () => { installAudio(url => { const a = new FakeAudio(url); a.duration = 1; @@ -135,8 +140,60 @@ describe('playBase64Audio', () => { handle.stop(); expect(URL.revokeObjectURL).toHaveBeenCalledWith('blob:mock'); expect(handle.currentMs()).toBe(-1); - await expect(handle.ended).rejects.toThrow('stopped'); + // Sentry was firing on these orphan rejections because callers couldn't + // distinguish "we intentionally stopped" from a real decoder error. The + // typed sentinel + `.stopped` brand makes the cancellation case trivially + // detectable by `isAudioStopped` / `swallowAudioStop` consumers (#1472). + const err = await handle.ended.catch(e => e); + expect(err).toBeInstanceOf(AudioStoppedError); + expect(err).toMatchObject({ stopped: true, name: 'AudioStoppedError' }); // Idempotent — second stop() is a no-op. handle.stop(); }); }); + +describe('isAudioStopped / swallowAudioStop', () => { + it('isAudioStopped recognizes AudioStoppedError instances', () => { + expect(isAudioStopped(new AudioStoppedError())).toBe(true); + }); + + it('isAudioStopped accepts the structural `.stopped === true` brand', () => { + // Defends against bundle-duplication: a second copy of audioPlayer.ts + // would produce a different class identity but the brand still matches. + expect(isAudioStopped({ stopped: true })).toBe(true); + expect(isAudioStopped(Object.assign(new Error('x'), { stopped: true }))).toBe(true); + }); + + it('isAudioStopped rejects plain errors', () => { + expect(isAudioStopped(new Error('audio playback error'))).toBe(false); + expect(isAudioStopped(null)).toBe(false); + expect(isAudioStopped(undefined)).toBe(false); + expect(isAudioStopped('stopped')).toBe(false); + }); + + it('swallowAudioStop returns void on AudioStoppedError', () => { + expect(() => swallowAudioStop(new AudioStoppedError())).not.toThrow(); + }); + + it('swallowAudioStop rethrows non-stop errors so real failures stay visible', () => { + const real = new Error('audio playback error'); + expect(() => swallowAudioStop(real)).toThrow(real); + }); + + it('attached as .catch(swallowAudioStop), it consumes orphan stop rejections without unhandledrejection', async () => { + const unhandled: PromiseRejectionEvent[] = []; + const handler = (e: PromiseRejectionEvent) => unhandled.push(e); + window.addEventListener('unhandledrejection', handler); + try { + installAudio(url => new FakeAudio(url)); + const handle = await playBase64Audio('AAA='); + handle.stop(); + handle.ended.catch(swallowAudioStop); + // Let microtasks + a macrotask flush so any unhandledrejection would fire. + await new Promise(r => setTimeout(r, 0)); + expect(unhandled).toHaveLength(0); + } finally { + window.removeEventListener('unhandledrejection', handler); + } + }); +}); diff --git a/app/src/features/human/voice/audioPlayer.ts b/app/src/features/human/voice/audioPlayer.ts index cace6e119..2f73d22fb 100644 --- a/app/src/features/human/voice/audioPlayer.ts +++ b/app/src/features/human/voice/audioPlayer.ts @@ -2,6 +2,35 @@ * Lightweight base64 → playable HTMLAudio wrapper. We don't need WebAudio * graph here; the viseme scheduler reads `currentTime` directly. */ + +/** + * Sentinel thrown by the `ended` promise when `stop()` is called. Callers that + * intentionally cancel playback (e.g. swapping to a new utterance) can detect + * this and silence the rejection without masking real audio decoder errors. + */ +export class AudioStoppedError extends Error { + readonly stopped = true; + constructor() { + super('stopped'); + this.name = 'AudioStoppedError'; + } +} + +export function isAudioStopped(err: unknown): err is AudioStoppedError { + if (err instanceof AudioStoppedError) return true; + return typeof err === 'object' && err !== null && (err as { stopped?: unknown }).stopped === true; +} + +/** + * Use as `.catch(swallowAudioStop)` on an orphan `handle.ended` promise, or + * `catch (err) { swallowAudioStop(err); }` around an awaited one. Swallows the + * stop sentinel; rethrows anything else so real decoder errors stay visible. + */ +export function swallowAudioStop(err: unknown): void { + if (isAudioStopped(err)) return; + throw err; +} + export interface PlaybackHandle { /** ms elapsed since audio started. Returns -1 after playback ends. */ currentMs(): number; @@ -92,7 +121,7 @@ export async function playBase64Audio( stopped = true; audio.pause(); cleanup(); - rejectEnded(new Error('stopped')); + rejectEnded(new AudioStoppedError()); }, ended, };