fix: make getAvailableVoices always settle, and stop rewriting rate 0 to 1 (#1139) - #1144
Open
MOHITKOURAV01 wants to merge 1 commit into
Open
fix: make getAvailableVoices always settle, and stop rewriting rate 0 to 1 (#1139)#1144MOHITKOURAV01 wants to merge 1 commit into
MOHITKOURAV01 wants to merge 1 commit into
Conversation
… to 1 (Aditya8369#1139) getVoices() returning [] on the first call is the normal case in Chrome and Edge -- the list loads asynchronously -- so the else-branch was the branch that ran, and it waited on `voiceschanged` alone. That event is not guaranteed to fire: not in Firefox when the list is already final, not in headless Chrome with no speech engine, not in jsdom. The promise then stayed pending forever, so useVoiceSynthesis's `.then(setVoices)` never resolved and the voice picker sat permanently empty with no error and nothing to retry -- indistinguishable from a device with no voices. It also assigned `speechSynthesis.onvoiceschanged`, which is one slot rather than a listener list: two concurrent callers and the first one's resolve was overwritten and never called. Nothing cleared it afterwards either, so the handler outlived the page's own and held its closure alive. getAvailableVoices now resolves from whichever comes first -- an already populated list, a voiceschanged carrying one, a poll, or a bounded timeout -- registers through addEventListener so callers do not overwrite each other and the page's handler is untouched, and tears down its listener and timers on the way out. It never rejects; an empty list is a usable answer, never answering is not. A voiceschanged that arrives before the list is ready no longer resolves empty. Separately, `config.rate || 1` and `config.pitch || 1` rewrote 0 -- a value the API accepts, and the one the settings slider's minimum produces -- to normal speed and pitch, while volume next to them had the correct guard. The same || let an out-of-range value through, and speak() throws SyntaxError on those, surfacing as an unhandled rejection. clampToRange holds all three to the ranges already documented in types/speech.ts. Absent is not zero: null and '' take the fallback rather than clamping to 0. speakText also rejects an empty utterance up front, rather than queueing one that some engines never fire `end` for -- which would wedge the caller's queue on an item that can never finish. Tests: 30, the first for this module. Twelve fail against the code before this change, two of them by timing out at 5s -- which is the hang.
|
@MOHITKOURAV01 is attempting to deploy a commit to the Aditya Mahajan's projects Team on Vercel. A member of the Team first needs to authorize it. |
Thank You for Your Contribution! 🎉Hi @MOHITKOURAV01, Thank you for opening this Pull Request and contributing to our project. We truly appreciate your efforts.
The maintainer @Aditya8369 will review your PR shortly! Happy Contributing! 🚀 |
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.
Description
getAvailableVoices()returned a promise that, on the most common browser path, couldnever settle.
speechSynthesis.getVoices()returning[]on the first call is the normal case inChrome and Edge — the voice list is populated asynchronously — so the else-branch was the
branch that ran, and it waited on
voiceschangedalone:That event is not guaranteed to fire. Not in Firefox when the list is already final, not in
headless Chrome with no speech engine installed, not in jsdom. When it doesn't, the promise
stays pending,
useVoiceSynthesis'sgetAvailableVoices().then(setVoices)never resolves,and the voice dropdown in
VoiceAlertManagersits permanently empty — no error, nofallback, nothing to retry, and indistinguishable from "this device has no voices".
Two more problems in the same six lines:
onvoiceschangedis a single assignment slot, nota listener list, so two concurrent callers means the first one's
resolveis overwrittenand never called; and nothing ever cleared the handler, so it outlived the page's own and
held its closure alive.
Separately,
rate: 0andpitch: 0were silently rewritten to1.Related Issue
Closes #1139
Type of Change
Changes Made
getAvailableVoices(timeoutMs = 2000)— always settlesResolves from whichever comes first: an already-populated list, a
voiceschangedcarryingone, a 100 ms poll, or the timeout. The poll matters because some engines populate the list
without ever firing the event; the timeout is the backstop that makes "always settles"
true. It never rejects — an empty list is a usable answer, never answering is not.
addEventListenerinstead of theonvoiceschangedslot, so concurrentcallers don't overwrite each other and the page's own handler is left exactly as it was
found. There's a test asserting that specifically.
voiceschangedthat arrives before the list is ready no longer resolves[]. Someengines fire it more than once as lists load, and the old code took the first one.
getVoices()that throws is handled rather than escaping.clampToRange—rate: 0andpitch: 0are deliberate valuesvolumehad the right guard; the two beside it didn't.0is a value the Web Speech APIaccepts and the value the settings slider's minimum produces, so dragging rate to the
bottom silently gave normal speed. The same
||also let an out-of-range value straightthrough, and
speak()throws aSyntaxErroron those, which surfaced as an unhandledrejection.
clampToRangeholds all three to the ranges already documented insrc/types/speech.ts(rate 0.1–10, pitch 0–2, volume 0–1). Absent is not zero:
nulland''take the fallbackrather than clamping to
0, sinceNumber(null)is0and that sits inside two of thethree ranges. Exported along with
SPEECH_RANGESso the settings UI can show the bounds itwill be held to.
speakText— rejects an empty utterance up front rather than queueing one that someengines never fire
endfor, which would wedge the caller's queue on an item that can neverfinish. Tolerates a missing
config, and agetVoices()that isn't an array.Testing
src/services/speechService.test.jsis new — this module had no tests. 30 of them, againsta fake
speechSynthesisthat reproduces the real loading behaviour (getVoices()emptyuntil the engine is ready, with a
voiceschangedthat may or may not fire).Against the code before this change, 12 fail — and two of them fail by timing out at 5s,
which is the hang itself:
Scope
Only the service.
useVoiceSynthesisandVoiceAlertManagerhave their own defect(#1136) and their own PR; this one is deliberately kept to the module the hang lives in, so
the two can merge in either order.
Note on CI
Lint, Build and Playwright are red on
mainand on every open PR (the 3 parse errors of#1129). Nothing here touches those files.