-
Notifications
You must be signed in to change notification settings - Fork 5.3k
Pr 2365 #2379
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. Weβll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Pr 2365 #2379
Conversation
Reviewer's guide (collapsed on small PRs)Reviewer's GuideAdjusts WhatsApp Baileys connection-close handling to avoid reconnect loops during initial QR setup and adds clearer logging around reconnection decisions. Sequence diagram for WhatsApp Baileys connection close and reconnection logicsequenceDiagram
participant BaileysStartupService
participant WhatsAppSocket
participant Logger
WhatsAppSocket->>BaileysStartupService: connection.update(connection = close, lastDisconnect)
BaileysStartupService->>BaileysStartupService: read statusCode from lastDisconnect
BaileysStartupService->>BaileysStartupService: isInitialConnection = !instance.wuid && (instance.qrcode.count == 0)
alt Initial connection (waiting for QR)
BaileysStartupService->>Logger: info(Initial connection closed, waiting for QR code generation...)
BaileysStartupService-->>WhatsAppSocket: do not reconnect
else Non initial connection
BaileysStartupService->>BaileysStartupService: shouldReconnect = !codesToNotReconnect.includes(statusCode)
alt shouldReconnect
BaileysStartupService->>Logger: warn(Connection lost, reconnecting...)
BaileysStartupService->>BaileysStartupService: connectToWhatsapp(phoneNumber)
else shouldNotReconnect
BaileysStartupService->>Logger: info(Skipping reconnection for statusCode in codesToNotReconnect)
BaileysStartupService->>BaileysStartupService: sendDataWebhook(Events.STATUS_INSTANCE, payload)
end
end
Updated class diagram for BaileysStartupService connection close handlingclassDiagram
class ChannelStartupService {
}
class BaileysStartupService {
- any instance
- any phoneNumber
- any logger
+ handleConnectionClose(connection, lastDisconnect) void
+ connectToWhatsapp(phoneNumber) Promise
+ sendDataWebhook(event, payload) void
}
BaileysStartupService --|> ChannelStartupService
class Instance {
+ string name
+ string wuid
+ Qrcode qrcode
}
class Qrcode {
+ number count
}
class Events {
<<enumeration>>
STATUS_INSTANCE
}
BaileysStartupService o-- Instance : instance
BaileysStartupService ..> Events : uses
BaileysStartupService ..> Qrcode : reads count
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey - I've found 1 issue, and left some high level feedback:
- The
isInitialConnectiondetection mixes!this.instance.wuidwith aqrcode?.countcheck inline; consider encapsulating this in a well-named helper (e.g.isWaitingForInitialQr()) or using an explicit state flag so the semantics are clearer and less fragile if the instance properties change in the future.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The `isInitialConnection` detection mixes `!this.instance.wuid` with a `qrcode?.count` check inline; consider encapsulating this in a well-named helper (e.g. `isWaitingForInitialQr()`) or using an explicit state flag so the semantics are clearer and less fragile if the instance properties change in the future.
## Individual Comments
### Comment 1
<location> `src/api/integrations/channel/whatsapp/whatsapp.baileys.service.ts:432-436` </location>
<code_context>
+
+ // FIX: Do not reconnect if it's the initial connection (waiting for QR code)
+ // This prevents infinite loop that blocks QR code generation
+ const isInitialConnection = !this.instance.wuid && (this.instance.qrcode?.count ?? 0) === 0;
+
+ if (isInitialConnection) {
+ this.logger.info('Initial connection closed, waiting for QR code generation...');
+ return;
+ }
+
</code_context>
<issue_to_address>
**issue (bug_risk):** Early return on initial connection may skip other important close-handling logic (e.g., status notifications).
Because this returns early when `isInitialConnection` is true, none of the later close-handling logic runs (including the `STATUS_INSTANCE` webhook in the non-reconnect path). If callers expect that status even on initial connection close, this can break monitoring/state tracking. You might instead still emit the appropriate status event here, or refactor so only the reconnect loop is skipped while other close-handling remains consistent.
</issue_to_address>Help me be more useful! Please click π or π on each comment and I'll use the feedback to improve your reviews.
| const isInitialConnection = !this.instance.wuid && (this.instance.qrcode?.count ?? 0) === 0; | ||
|
|
||
| if (isInitialConnection) { | ||
| this.logger.info('Initial connection closed, waiting for QR code generation...'); | ||
| return; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
issue (bug_risk): Early return on initial connection may skip other important close-handling logic (e.g., status notifications).
Because this returns early when isInitialConnection is true, none of the later close-handling logic runs (including the STATUS_INSTANCE webhook in the non-reconnect path). If callers expect that status even on initial connection close, this can break monitoring/state tracking. You might instead still emit the appropriate status event here, or refactor so only the reconnect loop is skipped while other close-handling remains consistent.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR attempts to fix an infinite reconnection loop that blocks QR code generation during initial WhatsApp connection. It adds a check to prevent automatic reconnection when a connection closes during the initial connection phase (before a user is authenticated). The PR also adds logging for better visibility into connection state changes and reconnection decisions.
Changes:
- Added
isInitialConnectioncheck to prevent reconnection during initial connection phase - Added logging statements for reconnection attempts, skipped reconnections, and initial connection closures
π‘ Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
| // FIX: Do not reconnect if it's the initial connection (waiting for QR code) | ||
| // This prevents infinite loop that blocks QR code generation | ||
| const isInitialConnection = !this.instance.wuid && (this.instance.qrcode?.count ?? 0) === 0; |
Copilot
AI
Jan 23, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The condition (this.instance.qrcode?.count ?? 0) === 0 only prevents reconnection when the connection closes before the first QR code is generated. However, based on the comment "Do not reconnect if it's the initial connection (waiting for QR code)", it appears the intent is to prevent reconnection during the entire QR code phase (count can be 1 to LIMIT while waiting for the user to scan). After the first QR code is generated (count becomes 1), if the connection closes, this check will fail and reconnection will still trigger, potentially causing the infinite loop mentioned in the comment. Consider using !this.instance.wuid alone (which indicates no authenticated user) or checking if count is below the LIMIT to cover all initial connection scenarios.
| const isInitialConnection = !this.instance.wuid && (this.instance.qrcode?.count ?? 0) === 0; | |
| const isInitialConnection = !this.instance.wuid; |
| if (connection === 'close') { | ||
| const statusCode = (lastDisconnect?.error as Boom)?.output?.statusCode; | ||
| const codesToNotReconnect = [DisconnectReason.loggedOut, DisconnectReason.forbidden, 402, 406]; | ||
|
|
||
| // FIX: Do not reconnect if it's the initial connection (waiting for QR code) | ||
| // This prevents infinite loop that blocks QR code generation | ||
| const isInitialConnection = !this.instance.wuid && (this.instance.qrcode?.count ?? 0) === 0; | ||
|
|
||
| if (isInitialConnection) { | ||
| this.logger.info('Initial connection closed, waiting for QR code generation...'); | ||
| return; | ||
| } | ||
|
|
||
| const shouldReconnect = !codesToNotReconnect.includes(statusCode); | ||
| if (shouldReconnect) { | ||
| this.logger.warn(`Connection lost (status: ${statusCode}), reconnecting...`); | ||
| await this.connectToWhatsapp(this.phoneNumber); | ||
| } else { | ||
| this.logger.info( | ||
| `Skipping reconnection for status code ${statusCode} (code is in codesToNotReconnect list)`, | ||
| ); | ||
| this.sendDataWebhook(Events.STATUS_INSTANCE, { |
Copilot
AI
Jan 23, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The PR description is incomplete - it only contains the template without filling in the actual details. Please provide a description of what issue this PR is addressing, what the root cause was, and how this fix resolves it. This will help reviewers understand the context and validate that the solution is correct.
π Description
π Related Issue
Closes #(issue_number)
π§ͺ Type of Change
π§ͺ Testing
πΈ Screenshots (if applicable)
β Checklist
π Additional Notes
Summary by Sourcery
Handle WhatsApp Baileys connection closures more safely to avoid blocking initial QR code generation and improve reconnection logging.
Bug Fixes:
Enhancements: