fix: wait for WA connection before returning socket - #16
Conversation
startWhatsAppConnection was resolving immediately after createWASocket, before the WebSocket handshake with WhatsApp completed (~700ms). Any tool call that arrived during that window failed with "Connection Closed". Add a connectionReady promise that resolves on connection === "open" (with a 20s timeout). The MCP server now only starts accepting requests once the socket is fully authenticated and ready to send messages. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
📝 WalkthroughWalkthrough
ChangesConnection readiness synchronization
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint skipped: no ESLint configuration detected in root package.json. To enable, add 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 |
![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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/whatsapp.ts (1)
153-155:⚠️ Potential issue | 🟠 Major | ⚡ Quick winNon-logout close leaves the promise pending until timeout.
When connection closes for a non-logout reason, the code starts a recursive reconnection but does not clear the timeout or reject
connectionReady. The original caller will wait the full 20s before receiving a timeout error, even though the socket is already closed. Additionally, the recursive call creates a new socket that is never returned to the original caller (orphaned).Consider rejecting immediately on any close to give fast feedback:
Proposed fix
if (statusCode !== DisconnectReason.loggedOut) { + clearTimeout(connectionTimeout); + rejectConnection?.( + new Error(`Connection closed: ${DisconnectReason[statusCode as number] || "Unknown"}`) + ); + rejectConnection = null; + resolveConnection = null; logger.info("Reconnecting..."); startWhatsAppConnection(logger);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/whatsapp.ts` around lines 153 - 155, The connection close handler currently starts a new reconnection via startWhatsAppConnection when statusCode !== DisconnectReason.loggedOut but does not reject/resolve the existing connectionReady promise or clear its timeout, leaving the original caller waiting and creating an orphaned socket; modify the close branch so that on any non-logout close you immediately reject (or resolve with a failure) the existing connectionReady promise and clear its associated timeout before triggering startWhatsAppConnection, and ensure any socket reference created for this attempt is cleaned up or returned to the caller to avoid orphaned sockets (referencing connectionReady, startWhatsAppConnection, statusCode, DisconnectReason and the timeout variable used to guard the 20s wait).
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/whatsapp.ts`:
- Around line 168-170: The logout/timeout handling nulls both promise handlers
but the success path only nulls resolveConnection; update the success branch
where connectionTimeout is cleared and resolveConnection is set to null to also
set rejectConnection = null for consistency and to avoid leaving a stale
reference to the reject handler (referencing resolveConnection,
rejectConnection, and connectionTimeout).
---
Outside diff comments:
In `@src/whatsapp.ts`:
- Around line 153-155: The connection close handler currently starts a new
reconnection via startWhatsAppConnection when statusCode !==
DisconnectReason.loggedOut but does not reject/resolve the existing
connectionReady promise or clear its timeout, leaving the original caller
waiting and creating an orphaned socket; modify the close branch so that on any
non-logout close you immediately reject (or resolve with a failure) the existing
connectionReady promise and clear its associated timeout before triggering
startWhatsAppConnection, and ensure any socket reference created for this
attempt is cleaned up or returned to the caller to avoid orphaned sockets
(referencing connectionReady, startWhatsAppConnection, statusCode,
DisconnectReason and the timeout variable used to guard the 20s wait).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: a92156a4-9bf3-4e40-adcf-16261fdf6fa1
📒 Files selected for processing (1)
src/whatsapp.ts
| clearTimeout(connectionTimeout); | ||
| resolveConnection?.(); | ||
| resolveConnection = null; |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial | 💤 Low value
Consider nulling rejectConnection for consistency.
On logout (lines 159-160) and timeout (lines 127-128), both resolveConnection and rejectConnection are nulled. Here only resolveConnection is nulled. While functionally harmless (the promise is settled), nulling both would be consistent.
Proposed fix
clearTimeout(connectionTimeout);
resolveConnection?.();
resolveConnection = null;
+ rejectConnection = null;📝 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.
| clearTimeout(connectionTimeout); | |
| resolveConnection?.(); | |
| resolveConnection = null; | |
| clearTimeout(connectionTimeout); | |
| resolveConnection?.(); | |
| resolveConnection = null; | |
| rejectConnection = null; |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/whatsapp.ts` around lines 168 - 170, The logout/timeout handling nulls
both promise handlers but the success path only nulls resolveConnection; update
the success branch where connectionTimeout is cleared and resolveConnection is
set to null to also set rejectConnection = null for consistency and to avoid
leaving a stale reference to the reject handler (referencing resolveConnection,
rejectConnection, and connectionTimeout).
1 participant
Summary
startWhatsAppConnectionresolved immediately aftermakeWASocket, before the WebSocket handshake with WhatsApp completed (~700ms on average).Connection Closederror and failed silently.connectionReadypromise that resolves onconnection === 'open'(with a 20s timeout). The function now awaits this promise before returning the socket, so the MCP server only accepts requests once the socket is fully authenticated and ready.Behaviour change
Before:
send_messagecalled immediately after server start would fail withError: Connection Closed.After: server startup blocks until WA confirms the session is open, then accepts tool calls.
Test plan
send_messageimmediately — message should succeed.wa-logs.txtshowsConnection openedbeforeMessage sent successfully.auth_info/and restart — QR flow should still work (promise resolves on the nextopenafter scan).Generated with Claude Code
Summary by CodeRabbit
Release Notes