fix: prevent unhandled rejections and stale audio URL leaks in useTTS.js - #1166
Conversation
|
@hrshjswniii is attempting to deploy a commit to the itzzavdhesh's projects Team on Vercel. A member of the Team first needs to authorize it. |
✍️ DCO Sign-off NeededHey @hrshjswniii! 👋 One or more commits in this PR are missing a Warning
How to fix: For the latest commit: git commit --amend --signoff
git push --force-with-leaseFor multiple commits, replace git rebase --signoff HEAD~N
git push --force-with-leaseThis comment will update automatically after you push. 🤖 VoiceForge Automation · Updates automatically on edits |
|
Warning Review limit reached
Next review available in: 59 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
github-actions
Bot
added
assignee-match
bug
🎉 PR Ready for Mentor ReviewHey @hrshjswniii! 👋 Your PR passed all checks and is now in the GSSoC review queue. Note 🔗 Closing: #1133 · 📐 60 lines across 2 file(s) · 📬 Already requested or no eligible reviewer found @sabeenaviklar @Anushreebasics @itsdakshjain @snehkris @Mrigakshi-Rathore @Itzzavdheshh @Nitya-003 @4f4d @lovestaco, this PR is ready for your review — please confirm scope, check behavior and tests, then approve or request changes. Important This is not an approval. Please wait for mentor feedback before expecting a merge. If changes are requested, push them to this same branch and keep the PR focused on the linked issue. 🤖 VoiceForge Automation · Updates automatically on edits |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
2 issues found across 2 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="client/src/hooks/useTTS.js">
<violation number="1" location="client/src/hooks/useTTS.js:124">
P2: Rapid replacement requests can still leave an earlier browser-fallback utterance audible. `controller.abort()` does not cancel `speechSynthesis`; wire the signal to `speechSynthesis.cancel()` (including unmount) so this cancelled path stops playback immediately.</violation>
</file>
<file name="client/src/hooks/useTTS.test.js">
<violation number="1" location="client/src/hooks/useTTS.test.js:1">
P2: The test coverage for this PR's core fixes (abort handling and blob URL cleanup) is minimal. The single test only verifies that the default export is a function, which is a module-level assertion that doesn't exercise the runtime behavior of the hook. The PR description lists "Unit Tests" as a key deliverable, and the existing test in the codebase (`MessageCard.test.js`) sets a precedent of testing edge cases, mock timers, and behavioral assertions. The `vitest` environment is `"node"` and `@testing-library/react` is not in the dependencies, but the test could still be extended to verify the hook's return shape, state transitions, or—after installing `jsdom` and `@testing-library/react`—runtime coverage of abort handling and URL revocation.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| const payload = await response.json(); | ||
| const nextAudioUrl = payload.audioUrl; | ||
|
|
||
| if (controller.signal.aborted) { |
There was a problem hiding this comment.
P2: Rapid replacement requests can still leave an earlier browser-fallback utterance audible. controller.abort() does not cancel speechSynthesis; wire the signal to speechSynthesis.cancel() (including unmount) so this cancelled path stops playback immediately.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At client/src/hooks/useTTS.js, line 124:
<comment>Rapid replacement requests can still leave an earlier browser-fallback utterance audible. `controller.abort()` does not cancel `speechSynthesis`; wire the signal to `speechSynthesis.cancel()` (including unmount) so this cancelled path stops playback immediately.</comment>
<file context>
@@ -90,8 +121,12 @@ export default function useTTS() {
const payload = await response.json();
const nextAudioUrl = payload.audioUrl;
+ if (controller.signal.aborted) {
+ return;
+ }
</file context>
| @@ -0,0 +1,8 @@ | |||
| import { describe, it, expect } from "vitest"; | |||
There was a problem hiding this comment.
P2: The test coverage for this PR's core fixes (abort handling and blob URL cleanup) is minimal. The single test only verifies that the default export is a function, which is a module-level assertion that doesn't exercise the runtime behavior of the hook. The PR description lists "Unit Tests" as a key deliverable, and the existing test in the codebase (MessageCard.test.js) sets a precedent of testing edge cases, mock timers, and behavioral assertions. The vitest environment is "node" and @testing-library/react is not in the dependencies, but the test could still be extended to verify the hook's return shape, state transitions, or—after installing jsdom and @testing-library/react—runtime coverage of abort handling and URL revocation.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At client/src/hooks/useTTS.test.js, line 1:
<comment>The test coverage for this PR's core fixes (abort handling and blob URL cleanup) is minimal. The single test only verifies that the default export is a function, which is a module-level assertion that doesn't exercise the runtime behavior of the hook. The PR description lists "Unit Tests" as a key deliverable, and the existing test in the codebase (`MessageCard.test.js`) sets a precedent of testing edge cases, mock timers, and behavioral assertions. The `vitest` environment is `"node"` and `@testing-library/react` is not in the dependencies, but the test could still be extended to verify the hook's return shape, state transitions, or—after installing `jsdom` and `@testing-library/react`—runtime coverage of abort handling and URL revocation.</comment>
<file context>
@@ -0,0 +1,8 @@
+import { describe, it, expect } from "vitest";
+import useTTS from "./useTTS";
+
</file context>
Nitya-003
left a comment
Nitya-003
left a comment
There was a problem hiding this comment.
@hrshjswniii Resolve the conflicts and comments.
itsdakshjain
left a comment
itsdakshjain
left a comment
There was a problem hiding this comment.
All good
Resolve the branch merge conflicts
itzzavdhesh
added
gssoc:approved
github-actions
Bot
added
mentor:Nitya-003
🔄 Changes RequestedHey @hrshjswniii! 👋 A mentor has reviewed your PR and requested some changes. Warning Please review the feedback above, update this same branch, and keep the PR focused on the linked issue. Once you push your updates, the review flow will continue automatically on this same PR. 🤖 VoiceForge Automation · Updates automatically on edits |
🎊 PR Merged SuccessfullyHey @hrshjswniii! 👋 Congratulations and thank you for your contribution to VoiceForge! Note 🔗 Linked issue(s): #1133 · ✅ Marked as merged and complete Maintainers may still handle final cleanup, release notes, or follow-up tracking after the merge. 🤖 VoiceForge Automation · Updates automatically on edits |
itzzavdhesh
added
SSoC26
4 participants
🚀 Program
GSSoC
📝 Description
This PR resolves unhandled
AbortErrorpromise rejections and stale Blob audio URL memory leaks inuseTTS.jsduring rapid speech synthesis requests.Key fixes:
updateAudioUrlhelper and an unmount cleanupuseEffecthandler to revoke staleblob:URLs viaURL.revokeObjectURL(prevUrl).controller.signal.abortedandAbortErrorin both outer synthesis and fallbackcatchblocks. Aborted requests now resolve cleanly without throwing unhandled promise rejections to caller components (Call.jsx,TextToSpeech.jsx).useTTS.test.js): Added Vitest test assertions foruseTTS.🔗 Related Issue
Closes #1133
🔄 Type of Change
🧪 How to Test
http://localhost:5173in a browser.AbortErrorunhandled rejections occur.npm run test --workspace clientand verify all tests pass.✅ Checklist
fix: prevent unhandled rejections and stale audio URL leaks in useTTS.js)Summary by cubic
Fixes unhandled
AbortErrorrejections and stops staleblob:audio URL leaks inuseTTS. Rapid speak calls no longer log errors, and audio URLs are cleaned up on update and unmount.audioUrldirectly and route updates throughupdateAudioUrl; revoke previousblob:URLs, abort in-flight requests, and clean up on unmount.controller.signal.aborted, catchAbortErrorin both primary and fallback paths, and keep engine/status updates consistent.Written for commit 945f3e2. Summary will update on new commits.