refactor: Enhance error handling, logging, and remote error capture a…#332
refactor: Enhance error handling, logging, and remote error capture a…#332
Conversation
…cross remote channel operations, and remove the subscribe method.
|
CodeAnt AI is reviewing your PR. Thanks for using CodeAnt! 🎉We're free for open-source projects. if you're enjoying it, help us grow by sharing. Share on X · |
📝 WalkthroughWalkthroughThis change adds comprehensive error handling, logging, and reliability improvements throughout the remote device channel lifecycle, including session setup, user retrieval, device management, channel subscription, and call tracking, with enhanced reconnect logic during device initialization. Changes
Sequence Diagram(s)sequenceDiagram
participant Client as Remote Client
participant Session as Session Manager
participant Device as Device Registry
participant Channel as Channel
participant Call as Call Tracker
participant Heartbeat as Heartbeat Monitor
Client->>Session: setSession()
alt Session Setup Fails
Session-->>Client: Log error, capture event, return error
else Session Setup Success
Session->>Device: User retrieval
alt User Not Found
Device-->>Session: Log failure, capture event, throw
else User Found
Session->>Device: findDevice()
alt Device Not Found
Device-->>Session: Log, capture event, proceed to create
else Device Found
Device->>Device: Update status/last_seen
Device->>Channel: Create channel with reconnect guard
alt Channel Init Fails
Channel-->>Device: Catch & retry on reconnect (silent)
else Channel Init Success
Channel->>Client: Subscribe & log health events
Client->>Call: markCallExecuting()
Call->>Heartbeat: updateHeartbeat()
Heartbeat->>Device: setOnlineStatus()
end
end
end
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
codeant-ai
bot
added
the
size:M
Nitpicks 🔍
|
|
CodeAnt AI finished reviewing your PR. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/remote-device/remote-channel.ts (2)
220-230:⚠️ Potential issue | 🟠 MajorFire-and-forget
setOnlineStatuscalls risk unhandled promise rejections.Lines 222 and 227 call
this.setOnlineStatus(...)withoutawaitor.catch(). If the underlying Supabase call orcaptureRemoterejects, it produces an unhandled promise rejection — which in Node.js can crash the process depending on the--unhandled-rejectionsflag.Add a
.catch()to match the pattern already used on line 215–217.🐛 Proposed fix
} else if (status === 'CHANNEL_ERROR') { // console.error('❌ Channel subscription failed:', err); - this.setOnlineStatus(this.deviceId!, 'offline'); + this.setOnlineStatus(this.deviceId!, 'offline').catch(e => { + console.error('Failed to set offline status:', e.message); + }); captureRemote('remote_channel_subscription_error', { error: err || 'Channel error' }).catch(() => { }); reject(err || new Error('Failed to initialize tool call channel subscription')); } else if (status === 'TIMED_OUT') { console.error('⏱️ Channel subscription timed out, Reconnecting...'); - this.setOnlineStatus(this.deviceId!, 'offline'); + this.setOnlineStatus(this.deviceId!, 'offline').catch(e => { + console.error('Failed to set offline status:', e.message); + }); captureRemote('remote_channel_subscription_timeout', {}).catch(() => { }); reject(new Error('Tool call channel subscription timed out'));
253-258:⚠️ Potential issue | 🟠 MajorAwaiting
captureRemotein synchronouscheckConnectionHealth— floating promise.Line 254 calls
captureRemote(...)(which returns a Promise) withoutawaitor.catch(). SincecheckConnectionHealthis not async, the rejection is unhandled. Add.catch(() => {})to match the pattern used elsewhere (e.g., lines 223, 228).🐛 Proposed fix
if (state !== 'joined') { - captureRemote('remote_channel_state_health', { state }); + captureRemote('remote_channel_state_health', { state }).catch(() => { }); console.debug(`[DEBUG] ⚠️ Channel in unhealthy state '${state}' - recreating...`);
🤖 Fix all issues with AI agents
In `@src/remote-device/remote-channel.ts`:
- Around line 167-170: Fix the typo in the inline comment near the channel
initialization retry: change "Inialization" to "Initialization" in the comment
that precedes the await this.createChannel().catch(...) call in
remote-channel.ts so codespell passes; ensure the comment reads something like
"// ! Ignore silently in Initialization to reconnect after" and keep the rest of
the catch block and logging unchanged.
- Around line 316-326: The update call using
this.client.from('mcp_remote_calls').update(updateData).eq('id', callId') does
not chain .select(), so Supabase always returns data as null; either chain
.select() on that query to return the updated row(s) and log the returned data
(matching the pattern used in updateDevice), or stop destructuring data and only
check error (e.g., remove data from the const and change the success log to a
simple "Call result updated successfully" without printing data); update the
code around the update call and the subsequent console.debug to reflect the
chosen approach.
🧹 Nitpick comments (3)
src/remote-device/remote-channel.ts (3)
53-58:console.errorwith[DEBUG]prefix is semantically misleading.Throughout these changes,
console.error('[DEBUG] Failed to ...')mixes error-level severity with a debug tag. This confuses log aggregation tools that key on severity. For operational clarity, useconsole.debugfor diagnostic context and reserveconsole.errorfor actionable errors without the[DEBUG]prefix.This pattern recurs at lines 55, 63, 70, 96, 112, 129, 298, 322, 338, 387.
76-76: User email logged in debug output — potential PII concern.
user.emailis logged to stdout. Even at debug level, this could persist in log aggregation systems. Consider whether this aligns with your data-handling policy, or redact it (e.g., mask all but the domain).
290-303: Inconsistent throw-vs-swallow pattern across methods.
markCallExecutingswallows errors (logs + captures but doesn't throw), whilefindDeviceandcreateDevicethrow. This inconsistency makes it harder for callers to reason about failure modes. Consider documenting the intended contract (throw on critical failures, swallow on best-effort updates) or normalizing the pattern.
| // ! Ignore silently in Inialization to reconnect after | ||
| await this.createChannel().catch((error) => { | ||
| console.debug('[DEBUG] Failed to create channel, will retry after socket reconnect', error); | ||
| }); |
There was a problem hiding this comment.
Fix typo: "Inialization" → "Initialization" (pipeline failure).
The codespell check is failing on this line.
✏️ Proposed fix
- // ! Ignore silently in Inialization to reconnect after
+ // ! Ignore silently in Initialization to reconnect after📝 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.
| // ! Ignore silently in Inialization to reconnect after | |
| await this.createChannel().catch((error) => { | |
| console.debug('[DEBUG] Failed to create channel, will retry after socket reconnect', error); | |
| }); | |
| // ! Ignore silently in Initialization to reconnect after | |
| await this.createChannel().catch((error) => { | |
| console.debug('[DEBUG] Failed to create channel, will retry after socket reconnect', error); | |
| }); |
🧰 Tools
🪛 GitHub Actions: Codespell
[error] 167-167: Misspelling detected by codespell: 'Inialization' should be 'Initialization'.
🪛 GitHub Check: Check for spelling errors
[failure] 167-167:
Inialization ==> Initialization
🤖 Prompt for AI Agents
In `@src/remote-device/remote-channel.ts` around lines 167 - 170, Fix the typo in
the inline comment near the channel initialization retry: change "Inialization"
to "Initialization" in the comment that precedes the await
this.createChannel().catch(...) call in remote-channel.ts so codespell passes;
ensure the comment reads something like "// ! Ignore silently in Initialization
to reconnect after" and keep the rest of the catch block and logging unchanged.
| const { data, error } = await this.client | ||
| .from('mcp_remote_calls') | ||
| .update(updateData) | ||
| .eq('id', callId); | ||
|
|
||
| if (error) { | ||
| console.error('[DEBUG] Failed to update call result:', error.message); | ||
| await captureRemote('remote_channel_update_call_result_error', { error }); | ||
| } else { | ||
| console.debug('[DEBUG] Call result updated successfully:', data); | ||
| } |
There was a problem hiding this comment.
data will always be null — .select() is not chained on this update.
Unlike updateDevice (line 109), this query doesn't chain .select(), so Supabase returns null for data. The debug log on line 325 will always print null, which is misleading.
Either chain .select() if you need the returned row, or remove the data capture and simplify the log.
✏️ Proposed fix (option A: add .select())
const { data, error } = await this.client
.from('mcp_remote_calls')
.update(updateData)
- .eq('id', callId);
+ .eq('id', callId)
+ .select();📝 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 { data, error } = await this.client | |
| .from('mcp_remote_calls') | |
| .update(updateData) | |
| .eq('id', callId); | |
| if (error) { | |
| console.error('[DEBUG] Failed to update call result:', error.message); | |
| await captureRemote('remote_channel_update_call_result_error', { error }); | |
| } else { | |
| console.debug('[DEBUG] Call result updated successfully:', data); | |
| } | |
| const { data, error } = await this.client | |
| .from('mcp_remote_calls') | |
| .update(updateData) | |
| .eq('id', callId) | |
| .select(); | |
| if (error) { | |
| console.error('[DEBUG] Failed to update call result:', error.message); | |
| await captureRemote('remote_channel_update_call_result_error', { error }); | |
| } else { | |
| console.debug('[DEBUG] Call result updated successfully:', data); | |
| } |
🤖 Prompt for AI Agents
In `@src/remote-device/remote-channel.ts` around lines 316 - 326, The update call
using this.client.from('mcp_remote_calls').update(updateData).eq('id', callId')
does not chain .select(), so Supabase always returns data as null; either chain
.select() on that query to return the updated row(s) and log the returned data
(matching the pattern used in updateDevice), or stop destructuring data and only
check error (e.g., remove data from the const and change the success log to a
simple "Call result updated successfully" without printing data); update the
code around the update call and the subsequent console.debug to reflect the
chosen approach.
1 participant
User description
This pull request improves error handling and debugging for the
RemoteChannelclass insrc/remote-device/remote-channel.ts. The main focus is on adding detailed error logging, capturing remote errors for monitoring, and providing more informative debug output throughout device and call management operations.Most important changes:
Error handling and logging improvements:
Debug output enhancements:
Device and channel management robustness:
registerDeviceto handle failures gracefully by logging and allowing for retry after socket reconnection, instead of failing outright.subscribemethod, consolidating channel subscription logic and reducing code duplication.Minor improvements:
CodeAnt-AI Description
Prevent initialization exit on channel creation failure and add clearer remote error reporting
What Changed
Impact
✅ Clearer remote error reporting during session and device operations✅ Fewer initialization exits when channel creation fails✅ Easier diagnosis of failed call, heartbeat, and status updates💡 Usage Guide
Checking Your Pull Request
Every time you make a pull request, our system automatically looks through it. We check for security issues, mistakes in how you're setting up your infrastructure, and common code problems. We do this to make sure your changes are solid and won't cause any trouble later.
Talking to CodeAnt AI
Got a question or need a hand with something in your pull request? You can easily get in touch with CodeAnt AI right here. Just type the following in a comment on your pull request, and replace "Your question here" with whatever you want to ask:
This lets you have a chat with CodeAnt AI about your pull request, making it easier to understand and improve your code.
Example
Preserve Org Learnings with CodeAnt
You can record team preferences so CodeAnt AI applies them in future reviews. Reply directly to the specific CodeAnt AI suggestion (in the same thread) and replace "Your feedback here" with your input:
This helps CodeAnt AI learn and adapt to your team's coding style and standards.
Example
Retrigger review
Ask CodeAnt AI to review the PR again, by typing:
Check Your Repository Health
To analyze the health of your code repository, visit our dashboard at https://app.codeant.ai. This tool helps you identify potential issues and areas for improvement in your codebase, ensuring your repository maintains high standards of code health.
Summary by CodeRabbit
Release Notes
Bug Fixes
Refactor