mirror of
https://github.com/coder/coder.git
synced 2026-09-22 05:05:20 +08:00
fix(site): fix speech recognition race condition and silent errors (#23043)
## Problem
Clicking the microphone button on `/agents` briefly activates recording
then immediately stops, focusing the text input.
## Root causes
**1. Race condition in `start()`** — When aborting a previous
recognition instance, the old instance's `onend` fires
**asynchronously** in real browsers (unlike our mock which fires it
synchronously). The stale `onend` callback then sets `isRecording=false`
and nullifies `recognitionRef.current`, killing the new recording
session. The unit tests didn't catch this because the mock fires `onend`
synchronously inside `abort()`.
**2. Silent `onerror` handler** — The `onerror` callback completely
discarded the error event. If the browser denied mic permission or the
speech service was unreachable (Chrome sends audio to Google servers),
recording silently died with no feedback.
**3. No cleanup on unmount** — The hook leaked a running recognition
instance if the component unmounted while recording.
## Fixes
- Guard `onend`/`onerror` callbacks with `recognitionRef.current !==
recognition` so stale instances are ignored
- Expose `error: string | null` state from the hook; surface it in the
UI ("Mic access denied" / "Voice input failed")
- Add a cleanup `useEffect` that aborts recognition on unmount
- Added 4 new tests covering the race condition, error exposure, and
error clearing
This commit is contained in:
@@ -233,4 +233,106 @@ describe("useSpeechRecognition", () => {
|
||||
expect(first?.abort).toHaveBeenCalled();
|
||||
expect(lastInstance).not.toBe(first);
|
||||
});
|
||||
|
||||
it("start() ignores onend from a previously aborted instance", () => {
|
||||
installMock();
|
||||
const { result } = renderHook(() => useSpeechRecognition());
|
||||
|
||||
// Start recording — creates the first instance.
|
||||
act(() => {
|
||||
result.current.start();
|
||||
});
|
||||
const first = lastInstance!;
|
||||
|
||||
// Override abort so it does NOT fire onend synchronously,
|
||||
// simulating the async browser behaviour.
|
||||
first.abort = vi.fn();
|
||||
|
||||
// Start recording again — creates a second instance and aborts
|
||||
// the first.
|
||||
act(() => {
|
||||
result.current.start();
|
||||
});
|
||||
const second = lastInstance!;
|
||||
|
||||
expect(first.abort).toHaveBeenCalled();
|
||||
expect(result.current.isRecording).toBe(true);
|
||||
|
||||
// Simulate the OLD instance's async onend firing late.
|
||||
act(() => {
|
||||
first.onend?.();
|
||||
});
|
||||
|
||||
// The old onend must be ignored — recording is still active.
|
||||
expect(result.current.isRecording).toBe(true);
|
||||
expect(lastInstance).toBe(second);
|
||||
});
|
||||
|
||||
it("exposes error from onerror event", () => {
|
||||
installMock();
|
||||
const { result } = renderHook(() => useSpeechRecognition());
|
||||
|
||||
expect(result.current.error).toBeNull();
|
||||
|
||||
act(() => {
|
||||
result.current.start();
|
||||
});
|
||||
|
||||
act(() => {
|
||||
lastInstance?.onerror?.({
|
||||
error: "not-allowed",
|
||||
message: "Permission denied",
|
||||
});
|
||||
});
|
||||
|
||||
expect(result.current.error).toBe("not-allowed");
|
||||
expect(result.current.isRecording).toBe(false);
|
||||
});
|
||||
|
||||
it("start() clears previous error", () => {
|
||||
installMock();
|
||||
const { result } = renderHook(() => useSpeechRecognition());
|
||||
|
||||
act(() => {
|
||||
result.current.start();
|
||||
});
|
||||
|
||||
act(() => {
|
||||
lastInstance?.onerror?.({
|
||||
error: "not-allowed",
|
||||
message: "Permission denied",
|
||||
});
|
||||
});
|
||||
expect(result.current.error).toBe("not-allowed");
|
||||
|
||||
act(() => {
|
||||
result.current.start();
|
||||
});
|
||||
|
||||
expect(result.current.error).toBeNull();
|
||||
expect(result.current.isRecording).toBe(true);
|
||||
});
|
||||
|
||||
it("cancel() clears error", () => {
|
||||
installMock();
|
||||
const { result } = renderHook(() => useSpeechRecognition());
|
||||
|
||||
act(() => {
|
||||
result.current.start();
|
||||
});
|
||||
|
||||
act(() => {
|
||||
lastInstance?.onerror?.({
|
||||
error: "not-allowed",
|
||||
message: "Permission denied",
|
||||
});
|
||||
});
|
||||
expect(result.current.error).toBe("not-allowed");
|
||||
|
||||
act(() => {
|
||||
result.current.cancel();
|
||||
});
|
||||
|
||||
expect(result.current.error).toBeNull();
|
||||
});
|
||||
});
|
||||
|
||||
@@ -1,4 +1,4 @@
|
||||
import { useCallback, useRef, useState } from "react";
|
||||
import { useCallback, useEffect, useRef, useState } from "react";
|
||||
|
||||
// Inline type declarations for the Web Speech API, which is not covered
|
||||
// by TypeScript's built-in lib types in all environments.
|
||||
@@ -75,12 +75,14 @@ export function useSpeechRecognition(): {
|
||||
isSupported: boolean;
|
||||
isRecording: boolean;
|
||||
transcript: string;
|
||||
error: string | null;
|
||||
start: () => void;
|
||||
stop: () => void;
|
||||
cancel: () => void;
|
||||
} {
|
||||
const [isRecording, setIsRecording] = useState(false);
|
||||
const [transcript, setTranscript] = useState("");
|
||||
const [error, setError] = useState<string | null>(null);
|
||||
const recognitionRef = useRef<SpeechRecognitionInstance | null>(null);
|
||||
|
||||
// Cache the constructor lookup once per hook instance so we don't hit
|
||||
@@ -103,6 +105,8 @@ export function useSpeechRecognition(): {
|
||||
recognitionRef.current = null;
|
||||
}
|
||||
|
||||
setError(null);
|
||||
|
||||
const recognition = new Ctor();
|
||||
recognition.lang = navigator.language;
|
||||
recognition.continuous = true;
|
||||
@@ -126,12 +130,15 @@ export function useSpeechRecognition(): {
|
||||
setTranscript(finalizedText + interim);
|
||||
};
|
||||
|
||||
recognition.onerror = () => {
|
||||
recognition.onerror = (event: SpeechRecognitionErrorEvent) => {
|
||||
if (recognitionRef.current !== recognition) return;
|
||||
setError(event.error);
|
||||
setIsRecording(false);
|
||||
recognitionRef.current = null;
|
||||
};
|
||||
|
||||
recognition.onend = () => {
|
||||
if (recognitionRef.current !== recognition) return;
|
||||
setIsRecording(false);
|
||||
recognitionRef.current = null;
|
||||
};
|
||||
@@ -160,7 +167,17 @@ export function useSpeechRecognition(): {
|
||||
}
|
||||
setIsRecording(false);
|
||||
setTranscript("");
|
||||
setError(null);
|
||||
}, []);
|
||||
|
||||
return { isSupported, isRecording, transcript, start, stop, cancel };
|
||||
useEffect(() => {
|
||||
return () => {
|
||||
if (recognitionRef.current) {
|
||||
recognitionRef.current.abort();
|
||||
recognitionRef.current = null;
|
||||
}
|
||||
};
|
||||
}, []);
|
||||
|
||||
return { isSupported, isRecording, transcript, error, start, stop, cancel };
|
||||
}
|
||||
|
||||
@@ -695,23 +695,35 @@ export const AgentChatInput = memo<AgentChatInputProps>(
|
||||
</>
|
||||
)}
|
||||
{speech.isSupported && !isStreaming && (
|
||||
<Button
|
||||
type="button"
|
||||
variant="subtle"
|
||||
size="icon"
|
||||
className="size-7 shrink-0 rounded-full [&>svg]:!size-icon-sm [&>svg]:p-0"
|
||||
onClick={
|
||||
speech.isRecording
|
||||
? handleCancelRecording
|
||||
: handleStartRecording
|
||||
}
|
||||
disabled={isDisabled}
|
||||
aria-label={
|
||||
speech.isRecording ? "Cancel voice input" : "Voice input"
|
||||
}
|
||||
>
|
||||
{speech.isRecording ? <XIcon /> : <MicIcon />}
|
||||
</Button>
|
||||
<>
|
||||
<Button
|
||||
type="button"
|
||||
variant="subtle"
|
||||
size="icon"
|
||||
className="size-7 shrink-0 rounded-full [&>svg]:!size-icon-sm [&>svg]:p-0"
|
||||
onClick={
|
||||
speech.isRecording
|
||||
? handleCancelRecording
|
||||
: handleStartRecording
|
||||
}
|
||||
disabled={isDisabled}
|
||||
aria-label={
|
||||
speech.isRecording ? "Cancel voice input" : "Voice input"
|
||||
}
|
||||
>
|
||||
{speech.isRecording ? <XIcon /> : <MicIcon />}
|
||||
</Button>
|
||||
{speech.error && !speech.isRecording && (
|
||||
<span
|
||||
className="text-2xs text-content-destructive"
|
||||
role="alert"
|
||||
>
|
||||
{speech.error === "not-allowed"
|
||||
? "Mic access denied"
|
||||
: "Voice input failed"}
|
||||
</span>
|
||||
)}
|
||||
</>
|
||||
)}
|
||||
{contextUsage !== undefined && (
|
||||
<ContextUsageIndicator usage={contextUsage} />
|
||||
|
||||
Reference in New Issue
Block a user