fix(human/mascot): consume orphan audio-stopped rejections (#1472) (#1494)

This commit is contained in:
oxoxDev
2026-05-11 09:29:47 -07:00
committed by GitHub
parent 5138a726dd
commit 49398094e8
5 changed files with 208 additions and 14 deletions
@@ -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;
+84 -3
View File
@@ -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<PlaybackHandle>(r => {
resolveFirstPlay = r;
});
(synthesizeSpeech as ReturnType<typeof vi.fn>)
.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<typeof vi.fn>)
.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<typeof vi.fn>).mockRejectedValueOnce(new Error('voice down'));
+24 -6
View File
@@ -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<void> {
// 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()) {
@@ -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);
}
});
});
+30 -1
View File
@@ -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,
};