fix(chat): ignore a stale voice recognizer's async callbacks after restart - #6494
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Bug found while reading recently-touched
apps/web/src/hooks/use-voice-input.ts(churned by #6451).Failure scenario:
cancelRecording()callsrecognitionRef.current?.abort()but does not null out the ref, andstartRecording()right after creates a newSpeechRecognitioninstance, reassigningrecognitionRef.current. Browsers (Chrome) resolveabort()asynchronously as anerrorevent witherror: "aborted"— so the old, already-cancelled recognizer'sonerrorcan fire after the new one is already recording. Its handler unconditionally doesisRecordingRef.current = false; recognitionRef.current = null; setStatus(...), which kills the ref to the live recognizer and flips the UI status back toidle/permission-deniedmid-recording. The same stale-closure issue affectsonend, which could restart a dead recognizer instance that no longer holds the mic session.Fix: each handler now checks
recognitionRef.current !== recognitionand bails if the callback belongs to a superseded instance — the same pattern already used to distinguish the live session from historic ones.Regression test:
use-voice-input.test.ts— starts recording, cancels, restarts (secondSpeechRecognitioninstance), then fires the first instance'sonerror({error: "aborted"}). Before the fix this drops status toidleeven though the second recognizer is still recording; after the fix status staysrecording. Verified the test fails on the pre-fix code (git stashthe source file, rerun) and passes after.To confirm:
bun test apps/web/src/hooks/use-voice-input.test.tsChecked locally:
bun run fmt,cd apps/web && bunx tsc --noEmit(no new errors),bunx oxlinton both changed files (0 warnings/errors), and the targeted test file above. Full CI covers the rest.Summary by cubic
Prevents stale
SpeechRecognitioncallbacks from affecting the current session after a restart. Previously, a cancelled recognizer’s lateonerror("aborted")oronendcould null out the liverecognitionRef, flip status fromrecording, or restart a dead instance; now those callbacks are ignored if they don’t belong to the current recognizer.use-voice-input.tsearly-return whenrecognitionRef.current !== recognition.onerror("aborted"), asserting status remainsrecording.Written for commit 52cec91. Summary will update on new commits.