Repository navigation
Added Error Logs in Realtime Service (Frontend) - #245
sanskaraggarwal2025 wants to merge 1 commit into
Conversation
WalkthroughError handling and logging improvements added to connection and disconnection flows in the AudioRealtimeModal component. The handleConnection error path now logs errors and updates status messaging, while handleDisconnect wraps cleanup operations in try-catch to guard against teardown failures. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ 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 |
Summary of ChangesHello @sanskaraggarwal2025, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request enhances the robustness and debuggability of the frontend's real-time audio service by introducing comprehensive error logging. It ensures that any failures during connection or disconnection attempts are properly reported, aiding in quicker identification and resolution of issues related to API keys or WebRTC operations. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request adds error logging to the handleConnection and handleDisconnect functions in the RealtimeAudioModal component, which is a good improvement for debugging. I've identified a potential issue in the new handleDisconnect implementation. If an error occurs while closing the connection or stopping media tracks, the component's state will not be reset, leading to an inconsistent UI. I've suggested using a finally block to ensure that state cleanup always executes, regardless of errors during the disconnection process.
| try { | ||
| pcRef.current?.close(); | ||
| streamRef.current?.getTracks().forEach(track => track.stop()); | ||
|
|
||
| pcRef.current = null; | ||
| streamRef.current = null; | ||
|
|
||
| $isConnected.set(false); | ||
| setIsConnecting(false); | ||
| setStatusMessage('Select provider and click Connect'); | ||
| setInputTokens(0); | ||
| setOutputTokens(0); | ||
| $isConnected.set(false); | ||
| setIsConnecting(false); | ||
| setStatusMessage('Select provider and click Connect'); | ||
| setInputTokens(0); | ||
| setOutputTokens(0); | ||
| } | ||
| catch (error) { | ||
| console.error("handleDisconnect error", error); | ||
| } |
There was a problem hiding this comment.
While adding a try...catch block is a good step for robustness, placing the state cleanup logic inside the try block can lead to an inconsistent UI state if an error occurs. For instance, if pcRef.current?.close() throws an error, the subsequent state updates (like $isConnected.set(false)) will be skipped, potentially leaving the UI in a 'connected' state.
To ensure the component state is always reset correctly, it's better to move the state update and resource nullification logic into a finally block. This guarantees that the cleanup code runs regardless of whether an error was thrown or not.
try {
pcRef.current?.close();
streamRef.current?.getTracks().forEach(track => track.stop());
} catch (error) {
console.error("handleDisconnect error", error);
} finally {
pcRef.current = null;
streamRef.current = null;
$isConnected.set(false);
setIsConnecting(false);
setStatusMessage('Select provider and click Connect');
setInputTokens(0);
setOutputTokens(0);
}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
frontend/src/components/AudioRealtimeModal.tsx(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
frontend/**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/project-description.mdc)
Use React with Tailwind CSS for frontend development
Files:
frontend/src/components/AudioRealtimeModal.tsx
🧬 Code graph analysis (1)
frontend/src/components/AudioRealtimeModal.tsx (1)
frontend/src/store/realtime.ts (1)
$isConnected(13-13)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: Build Go backend (windows, amd64, ubuntu-latest)
- GitHub Check: Build Go backend (darwin, arm64, macos-latest)
- GitHub Check: Build Go backend (linux, amd64, ubuntu-latest)
- GitHub Check: Build Go backend (linux, arm64, ubuntu-latest)
🔇 Additional comments (1)
frontend/src/components/AudioRealtimeModal.tsx (1)
126-131: Good addition of error logging.The error logging improves debuggability for connection failures, and the existing error handling flow correctly resets state and provides user feedback.
| const handleDisconnect = () => { | ||
| pcRef.current?.close(); | ||
| streamRef.current?.getTracks().forEach(track => track.stop()); | ||
|
|
||
| pcRef.current = null; | ||
| streamRef.current = null; | ||
| try { | ||
| pcRef.current?.close(); | ||
| streamRef.current?.getTracks().forEach(track => track.stop()); | ||
|
|
||
| pcRef.current = null; | ||
| streamRef.current = null; | ||
|
|
||
| $isConnected.set(false); | ||
| setIsConnecting(false); | ||
| setStatusMessage('Select provider and click Connect'); | ||
| setInputTokens(0); | ||
| setOutputTokens(0); | ||
| $isConnected.set(false); | ||
| setIsConnecting(false); | ||
| setStatusMessage('Select provider and click Connect'); | ||
| setInputTokens(0); | ||
| setOutputTokens(0); | ||
| } | ||
| catch (error) { | ||
| console.error("handleDisconnect error", error); | ||
| } | ||
| }; |
There was a problem hiding this comment.
State resets must execute even if cleanup fails.
If the cleanup operations (close peer connection or stop tracks) throw an error, the state resets on lines 142-146 won't execute, leaving the UI in an inconsistent state. This could prevent users from reconnecting.
Apply this diff to ensure state is always reset using a finally block:
const handleDisconnect = () => {
try {
pcRef.current?.close();
streamRef.current?.getTracks().forEach(track => track.stop());
pcRef.current = null;
streamRef.current = null;
-
- $isConnected.set(false);
- setIsConnecting(false);
- setStatusMessage('Select provider and click Connect');
- setInputTokens(0);
- setOutputTokens(0);
- }
- catch (error) {
+ } catch (error) {
console.error("handleDisconnect error", error);
+ } finally {
+ $isConnected.set(false);
+ setIsConnecting(false);
+ setStatusMessage('Select provider and click Connect');
+ setInputTokens(0);
+ setOutputTokens(0);
}
};📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const handleDisconnect = () => { | |
| pcRef.current?.close(); | |
| streamRef.current?.getTracks().forEach(track => track.stop()); | |
| pcRef.current = null; | |
| streamRef.current = null; | |
| try { | |
| pcRef.current?.close(); | |
| streamRef.current?.getTracks().forEach(track => track.stop()); | |
| pcRef.current = null; | |
| streamRef.current = null; | |
| $isConnected.set(false); | |
| setIsConnecting(false); | |
| setStatusMessage('Select provider and click Connect'); | |
| setInputTokens(0); | |
| setOutputTokens(0); | |
| $isConnected.set(false); | |
| setIsConnecting(false); | |
| setStatusMessage('Select provider and click Connect'); | |
| setInputTokens(0); | |
| setOutputTokens(0); | |
| } | |
| catch (error) { | |
| console.error("handleDisconnect error", error); | |
| } | |
| }; | |
| const handleDisconnect = () => { | |
| try { | |
| pcRef.current?.close(); | |
| streamRef.current?.getTracks().forEach(track => track.stop()); | |
| pcRef.current = null; | |
| streamRef.current = null; | |
| } catch (error) { | |
| console.error("handleDisconnect error", error); | |
| } finally { | |
| $isConnected.set(false); | |
| setIsConnecting(false); | |
| setStatusMessage('Select provider and click Connect'); | |
| setInputTokens(0); | |
| setOutputTokens(0); | |
| } | |
| }; |
🤖 Prompt for AI Agents
In frontend/src/components/AudioRealtimeModal.tsx around lines 134 to 151, the
current try/catch wraps both cleanup operations and state resets so if closing
the peer connection or stopping tracks throws, the subsequent state resets
(clearing refs, $isConnected, setIsConnecting, setStatusMessage, setInputTokens,
setOutputTokens) won't run; wrap the potentially-throwing cleanup calls
(pcRef.current?.close() and streamRef.current?.getTracks().forEach(track =>
track.stop())) in a try/catch, then move all state-reset lines into a finally
block so they always execute regardless of errors, logging any cleanup error in
the catch.
Summary by CodeRabbit
Bug Fixes
✏️ Tip: You can customize this high-level summary in your review settings.