feat: implement comprehensive Baileys WhatsApp message retry mechanism #73
+149
−0
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
- Add BaileysMessageRetryJob with exponential backoff retry logic (30s, 2m, 5m) - Implement state persistence for failed messages after all retries using Redis - Add BaileysFailedMessageRecoveryJob for periodic retry attempts every 5 minutes - Enhance error handling in WhatsappBaileysService with detailed error messages - Add BaileysRetryMonitorService for monitoring retry statistics and failed messages - Prevent Thundering Herd problem with proper job scheduling, delays, and Redis locking - Integrate with existing BaileysHelper locking mechanism for concurrent message processing - Modify SendReplyJob to delegate Baileys WhatsApp messages to specialized retry job - Add comprehensive logging and monitoring for retry attempts and failures - Store failed messages in Redis lists per channel for future redelivery when API becomes available Tested successfully with message to +5587988565072 - retry mechanism working correctly with proper tracking attributes (baileys_retry_count, baileys_last_attempt_at) added to messages. Fixes linting issues and follows Ruby style guidelines. Co-Authored-By: cayo@fazer.ai <cayoproliveira@gmail.com>
Member
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (2)
🧰 Additional context used🧠 Learnings (3)📓 Common learningsspec/services/whatsapp/send_on_whatsapp_service_spec.rb (6)spec/models/channel/whatsapp_spec.rb (3)🧬 Code Graph Analysis (1)spec/models/channel/whatsapp_spec.rb (1)
🔇 Additional comments (2)
✨ Finishing Touches
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Pull Request Overview
Enhance WhatsApp messaging via the Baileys protocol by adding robust retry, persistence, and recovery logic.
- Introduces a retry job with exponential backoff and Redis-backed persistence for failed messages.
- Adds a scheduled recovery job (every 5 minutes) to re-enqueue failed messages once the API is available.
- Improves error handling in the HTTP service and adds a monitoring service for retry statistics.
Reviewed Changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| config/schedule.yml | Schedules the recovery job every 5 minutes |
| app/services/whatsapp/providers/whatsapp_baileys_service.rb | Adds detailed error logging and rescues connection/timeouts |
| app/services/whatsapp/baileys_retry_monitor_service.rb | New service for gathering retry and failure metrics |
| app/jobs/send_reply_job.rb | Routes WhatsApp messages through the specialized retry handler |
| app/jobs/channels/whatsapp/baileys_message_retry_job.rb | Implements retry logic, persistence, and backoff calculations |
| app/jobs/channels/whatsapp/baileys_failed_message_recovery_job.rb | Processes Redis-stored failures for redelivery |
Comments suppressed due to low confidence (1)
app/services/whatsapp/baileys_retry_monitor_service.rb:1
- Add unit tests for both
failed_message_statsandretry_statistics(including JSON parsing) to verify monitoring logic and edge cases behave correctly.
class Whatsapp::BaileysRetryMonitorService
| rescue HTTParty::Error, Net::TimeoutError => e | ||
| Rails.logger.error "Baileys API connection error: #{e.message}" | ||
| raise MessageNotSentError, "Baileys API connection failed: #{e.message}" | ||
| rescue StandardError => e |
There was a problem hiding this comment.
Rescuing StandardError may mask unexpected bugs; narrow the rescue clause to only the exception types you intend to handle so genuine errors bubble up.
Suggested change
| rescue StandardError => e | |
| rescue RuntimeError, ArgumentError => e |
|
|
||
| queue_as :high | ||
|
|
||
| retry_on StandardError, wait: :exponentially_longer, attempts: 3 |
There was a problem hiding this comment.
The retry_on StandardError is redundant because your perform method already rescues all StandardError instances; consider removing this to avoid confusion.
Suggested change
| retry_on StandardError, wait: :exponentially_longer, attempts: 3 |
Comment on lines
58
to
59
| base_delay = 30.seconds | ||
| [base_delay * (2 ** (attempt_count - 1)), 5.minutes].min |
There was a problem hiding this comment.
[nitpick] Extract the base delay (30.seconds) and max delay (5.minutes) into named constants or configuration to make retry timing easier to adjust and document.
Suggested change
| base_delay = 30.seconds | |
| [base_delay * (2 ** (attempt_count - 1)), 5.minutes].min | |
| [BASE_DELAY * (2 ** (attempt_count - 1)), MAX_DELAY].min |
app/jobs/send_reply_job.rb
Outdated
| ::Facebook::SendOnFacebookService.new(message: message).perform | ||
| end | ||
| end | ||
|
|
There was a problem hiding this comment.
[nitpick] Consider marking handle_whatsapp_message as a private method to encapsulate routing logic and avoid unintended external calls.
Suggested change
| private |
![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
🧹 Nitpick comments (3)
app/jobs/channels/whatsapp/baileys_failed_message_recovery_job.rb (1)
28-29: Consider preserving retry count history for better tracking.The recovery job resets the attempt count to 1, which loses the history of previous retry attempts. Consider tracking both the original retry count and recovery attempt count for better monitoring.
- Channels::Whatsapp::BaileysMessageRetryJob.set(wait: delay).perform_later(message.id, 1) + original_retry_count = message_data['retry_count'] || 0 + Channels::Whatsapp::BaileysMessageRetryJob.set(wait: delay).perform_later(message.id, original_retry_count + 1)app/services/whatsapp/baileys_retry_monitor_service.rb (2)
1-19: LGTM with a performance consideration.The
failed_message_statsmethod correctly usesfind_eachfor memory efficiency and handles the Redis operations appropriately. The logic for collecting failed message statistics is sound.Consider adding Redis connection error handling for robustness in production environments.
def self.failed_message_stats stats = {} Channel::Whatsapp.where(provider: 'baileys').find_each do |channel| failed_message_key = "baileys:failed_messages:#{channel.id}" - failed_count = Redis::Alfred.llen(failed_message_key) + begin + failed_count = Redis::Alfred.llen(failed_message_key) + rescue Redis::BaseError => e + Rails.logger.error "Redis error for channel #{channel.id}: #{e.message}" + next + end if failed_count.positive? stats[channel.id] = { channel_phone: channel.phone_number, failed_messages_count: failed_count, oldest_failed_message: get_oldest_failed_message(failed_message_key) } end end stats end
21-34: Consider query optimization for better performance.The complex joins and
averagecalculation could be expensive on large datasets. The logic is correct, but performance could be improved by adding database indexes or restructuring the query.Consider adding a database index on
messages.additional_attributesusing a GIN index for PostgreSQL:CREATE INDEX CONCURRENTLY idx_messages_baileys_retry_count ON messages USING GIN ((additional_attributes->'baileys_retry_count'));Also consider caching this data or computing it asynchronously if it's called frequently, as the joins across multiple tables could be slow on large datasets.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
app/jobs/channels/whatsapp/baileys_failed_message_recovery_job.rb(1 hunks)app/jobs/channels/whatsapp/baileys_message_retry_job.rb(1 hunks)app/jobs/send_reply_job.rb(2 hunks)app/services/whatsapp/baileys_retry_monitor_service.rb(1 hunks)app/services/whatsapp/providers/whatsapp_baileys_service.rb(1 hunks)config/schedule.yml(1 hunks)
🧰 Additional context used
🧠 Learnings (7)
📓 Common learnings
Learnt from: CayoPOliveira
PR: fazer-ai/chatwoot#53
File: app/services/whatsapp/baileys_handlers/messages_upsert.rb:7-16
Timestamp: 2025-05-26T20:14:33.177Z
Learning: In the Whatsapp Baileys handlers for this codebase, errors should propagate to the WhatsappController level rather than being caught and handled individually within message processing methods. The user prefers centralized error handling at the controller level over individual message-level error handling.
Learnt from: CayoPOliveira
PR: fazer-ai/chatwoot#20
File: app/services/whatsapp/incoming_message_baileys_service.rb:0-0
Timestamp: 2025-04-08T21:26:26.063Z
Learning: Chatwoot message model (app/models/message.rb) only supports four status states: sent (0), delivered (1), read (2), and failed (3). When mapping from Baileys WhatsApp statuses, only ERROR (0), SERVER_ACK (2), DELIVERY_ACK (3), and READ (4) can be mapped to Chatwoot's statuses. Baileys statuses like PENDING (1) and PLAYED (5) don't have Chatwoot equivalents and should be handled as special cases or logged as unsupported.
Learnt from: CayoPOliveira
PR: fazer-ai/chatwoot#53
File: app/services/whatsapp/baileys_handlers/helpers.rb:59-70
Timestamp: 2025-05-26T20:14:33.151Z
Learning: In WhatsApp Baileys message handling, audio, file, sticker, protocol, and unsupported message types don't have extractable text content. Only text messages, image captions, video captions, and reaction text should be handled in message_content extraction methods.
Learnt from: CayoPOliveira
PR: fazer-ai/chatwoot#53
File: app/services/whatsapp/baileys_handlers/messages_update.rb:61-61
Timestamp: 2025-05-26T20:14:34.814Z
Learning: In Whatsapp::BaileysHandlers::MessagesUpdate#handle_edited_content, reassigning @raw_message to the nested edited message data is acceptable because this method executes at the end of the corresponding flow, with no subsequent operations depending on the original @raw_message structure.
Learnt from: CayoPOliveira
PR: fazer-ai/chatwoot#26
File: app/services/whatsapp/incoming_message_baileys_service.rb:198-207
Timestamp: 2025-04-16T11:55:12.231Z
Learning: In the WhatsApp Baileys API integration, only 'image' and 'video' message types support captions. The 'file', 'audio', and 'sticker' message types do not have caption fields.
Learnt from: CayoPOliveira
PR: fazer-ai/chatwoot#26
File: app/services/whatsapp/incoming_message_baileys_service.rb:198-207
Timestamp: 2025-04-16T11:55:12.231Z
Learning: In the WhatsApp Baileys API integration, only 'image' and 'video' message types support captions. The 'file', 'audio', and 'sticker' message types do not have caption fields.
app/jobs/send_reply_job.rb (6)
Learnt from: CayoPOliveira
PR: fazer-ai/chatwoot#53
File: app/services/whatsapp/baileys_handlers/messages_update.rb:61-61
Timestamp: 2025-05-26T20:14:34.814Z
Learning: In Whatsapp::BaileysHandlers::MessagesUpdate#handle_edited_content, reassigning @raw_message to the nested edited message data is acceptable because this method executes at the end of the corresponding flow, with no subsequent operations depending on the original @raw_message structure.
Learnt from: CayoPOliveira
PR: fazer-ai/chatwoot#53
File: app/services/whatsapp/baileys_handlers/messages_upsert.rb:7-16
Timestamp: 2025-05-26T20:14:33.177Z
Learning: In the Whatsapp Baileys handlers for this codebase, errors should propagate to the WhatsappController level rather than being caught and handled individually within message processing methods. The user prefers centralized error handling at the controller level over individual message-level error handling.
Learnt from: CayoPOliveira
PR: fazer-ai/chatwoot#20
File: app/services/whatsapp/incoming_message_baileys_service.rb:0-0
Timestamp: 2025-04-08T21:26:26.063Z
Learning: Chatwoot message model (app/models/message.rb) only supports four status states: sent (0), delivered (1), read (2), and failed (3). When mapping from Baileys WhatsApp statuses, only ERROR (0), SERVER_ACK (2), DELIVERY_ACK (3), and READ (4) can be mapped to Chatwoot's statuses. Baileys statuses like PENDING (1) and PLAYED (5) don't have Chatwoot equivalents and should be handled as special cases or logged as unsupported.
Learnt from: CayoPOliveira
PR: fazer-ai/chatwoot#53
File: app/services/whatsapp/baileys_handlers/helpers.rb:59-70
Timestamp: 2025-05-26T20:14:33.151Z
Learning: In WhatsApp Baileys message handling, audio, file, sticker, protocol, and unsupported message types don't have extractable text content. Only text messages, image captions, video captions, and reaction text should be handled in message_content extraction methods.
Learnt from: CayoPOliveira
PR: fazer-ai/chatwoot#26
File: app/services/whatsapp/incoming_message_baileys_service.rb:198-207
Timestamp: 2025-04-16T11:55:12.231Z
Learning: In the WhatsApp Baileys API integration, only 'image' and 'video' message types support captions. The 'file', 'audio', and 'sticker' message types do not have caption fields.
Learnt from: CayoPOliveira
PR: fazer-ai/chatwoot#26
File: app/services/whatsapp/incoming_message_baileys_service.rb:198-207
Timestamp: 2025-04-16T11:55:12.231Z
Learning: In the WhatsApp Baileys API integration, only 'image' and 'video' message types support captions. The 'file', 'audio', and 'sticker' message types do not have caption fields.
config/schedule.yml (2)
Learnt from: CayoPOliveira
PR: fazer-ai/chatwoot#53
File: app/services/whatsapp/baileys_handlers/messages_upsert.rb:7-16
Timestamp: 2025-05-26T20:14:33.177Z
Learning: In the Whatsapp Baileys handlers for this codebase, errors should propagate to the WhatsappController level rather than being caught and handled individually within message processing methods. The user prefers centralized error handling at the controller level over individual message-level error handling.
Learnt from: CayoPOliveira
PR: fazer-ai/chatwoot#20
File: app/services/whatsapp/incoming_message_baileys_service.rb:0-0
Timestamp: 2025-04-08T21:26:26.063Z
Learning: Chatwoot message model (app/models/message.rb) only supports four status states: sent (0), delivered (1), read (2), and failed (3). When mapping from Baileys WhatsApp statuses, only ERROR (0), SERVER_ACK (2), DELIVERY_ACK (3), and READ (4) can be mapped to Chatwoot's statuses. Baileys statuses like PENDING (1) and PLAYED (5) don't have Chatwoot equivalents and should be handled as special cases or logged as unsupported.
app/services/whatsapp/providers/whatsapp_baileys_service.rb (6)
Learnt from: CayoPOliveira
PR: fazer-ai/chatwoot#53
File: app/services/whatsapp/baileys_handlers/messages_upsert.rb:7-16
Timestamp: 2025-05-26T20:14:33.177Z
Learning: In the Whatsapp Baileys handlers for this codebase, errors should propagate to the WhatsappController level rather than being caught and handled individually within message processing methods. The user prefers centralized error handling at the controller level over individual message-level error handling.
Learnt from: CayoPOliveira
PR: fazer-ai/chatwoot#20
File: app/services/whatsapp/incoming_message_baileys_service.rb:0-0
Timestamp: 2025-04-08T21:26:26.063Z
Learning: Chatwoot message model (app/models/message.rb) only supports four status states: sent (0), delivered (1), read (2), and failed (3). When mapping from Baileys WhatsApp statuses, only ERROR (0), SERVER_ACK (2), DELIVERY_ACK (3), and READ (4) can be mapped to Chatwoot's statuses. Baileys statuses like PENDING (1) and PLAYED (5) don't have Chatwoot equivalents and should be handled as special cases or logged as unsupported.
Learnt from: CayoPOliveira
PR: fazer-ai/chatwoot#53
File: app/services/whatsapp/baileys_handlers/messages_update.rb:61-61
Timestamp: 2025-05-26T20:14:34.814Z
Learning: In Whatsapp::BaileysHandlers::MessagesUpdate#handle_edited_content, reassigning @raw_message to the nested edited message data is acceptable because this method executes at the end of the corresponding flow, with no subsequent operations depending on the original @raw_message structure.
Learnt from: CayoPOliveira
PR: fazer-ai/chatwoot#53
File: app/services/whatsapp/baileys_handlers/helpers.rb:59-70
Timestamp: 2025-05-26T20:14:33.151Z
Learning: In WhatsApp Baileys message handling, audio, file, sticker, protocol, and unsupported message types don't have extractable text content. Only text messages, image captions, video captions, and reaction text should be handled in message_content extraction methods.
Learnt from: CayoPOliveira
PR: fazer-ai/chatwoot#26
File: app/services/whatsapp/incoming_message_baileys_service.rb:198-207
Timestamp: 2025-04-16T11:55:12.231Z
Learning: In the WhatsApp Baileys API integration, only 'image' and 'video' message types support captions. The 'file', 'audio', and 'sticker' message types do not have caption fields.
Learnt from: CayoPOliveira
PR: fazer-ai/chatwoot#26
File: app/services/whatsapp/incoming_message_baileys_service.rb:198-207
Timestamp: 2025-04-16T11:55:12.231Z
Learning: In the WhatsApp Baileys API integration, only 'image' and 'video' message types support captions. The 'file', 'audio', and 'sticker' message types do not have caption fields.
app/services/whatsapp/baileys_retry_monitor_service.rb (4)
Learnt from: CayoPOliveira
PR: fazer-ai/chatwoot#20
File: app/services/whatsapp/incoming_message_baileys_service.rb:0-0
Timestamp: 2025-04-08T21:26:26.063Z
Learning: Chatwoot message model (app/models/message.rb) only supports four status states: sent (0), delivered (1), read (2), and failed (3). When mapping from Baileys WhatsApp statuses, only ERROR (0), SERVER_ACK (2), DELIVERY_ACK (3), and READ (4) can be mapped to Chatwoot's statuses. Baileys statuses like PENDING (1) and PLAYED (5) don't have Chatwoot equivalents and should be handled as special cases or logged as unsupported.
Learnt from: CayoPOliveira
PR: fazer-ai/chatwoot#53
File: app/services/whatsapp/baileys_handlers/messages_upsert.rb:7-16
Timestamp: 2025-05-26T20:14:33.177Z
Learning: In the Whatsapp Baileys handlers for this codebase, errors should propagate to the WhatsappController level rather than being caught and handled individually within message processing methods. The user prefers centralized error handling at the controller level over individual message-level error handling.
Learnt from: CayoPOliveira
PR: fazer-ai/chatwoot#53
File: app/services/whatsapp/baileys_handlers/helpers.rb:59-70
Timestamp: 2025-05-26T20:14:33.151Z
Learning: In WhatsApp Baileys message handling, audio, file, sticker, protocol, and unsupported message types don't have extractable text content. Only text messages, image captions, video captions, and reaction text should be handled in message_content extraction methods.
Learnt from: CayoPOliveira
PR: fazer-ai/chatwoot#53
File: app/services/whatsapp/baileys_handlers/messages_update.rb:61-61
Timestamp: 2025-05-26T20:14:34.814Z
Learning: In Whatsapp::BaileysHandlers::MessagesUpdate#handle_edited_content, reassigning @raw_message to the nested edited message data is acceptable because this method executes at the end of the corresponding flow, with no subsequent operations depending on the original @raw_message structure.
app/jobs/channels/whatsapp/baileys_failed_message_recovery_job.rb (4)
Learnt from: CayoPOliveira
PR: fazer-ai/chatwoot#53
File: app/services/whatsapp/baileys_handlers/messages_upsert.rb:7-16
Timestamp: 2025-05-26T20:14:33.177Z
Learning: In the Whatsapp Baileys handlers for this codebase, errors should propagate to the WhatsappController level rather than being caught and handled individually within message processing methods. The user prefers centralized error handling at the controller level over individual message-level error handling.
Learnt from: CayoPOliveira
PR: fazer-ai/chatwoot#20
File: app/services/whatsapp/incoming_message_baileys_service.rb:0-0
Timestamp: 2025-04-08T21:26:26.063Z
Learning: Chatwoot message model (app/models/message.rb) only supports four status states: sent (0), delivered (1), read (2), and failed (3). When mapping from Baileys WhatsApp statuses, only ERROR (0), SERVER_ACK (2), DELIVERY_ACK (3), and READ (4) can be mapped to Chatwoot's statuses. Baileys statuses like PENDING (1) and PLAYED (5) don't have Chatwoot equivalents and should be handled as special cases or logged as unsupported.
Learnt from: CayoPOliveira
PR: fazer-ai/chatwoot#53
File: app/services/whatsapp/baileys_handlers/messages_update.rb:61-61
Timestamp: 2025-05-26T20:14:34.814Z
Learning: In Whatsapp::BaileysHandlers::MessagesUpdate#handle_edited_content, reassigning @raw_message to the nested edited message data is acceptable because this method executes at the end of the corresponding flow, with no subsequent operations depending on the original @raw_message structure.
Learnt from: CayoPOliveira
PR: fazer-ai/chatwoot#53
File: app/services/whatsapp/baileys_handlers/helpers.rb:59-70
Timestamp: 2025-05-26T20:14:33.151Z
Learning: In WhatsApp Baileys message handling, audio, file, sticker, protocol, and unsupported message types don't have extractable text content. Only text messages, image captions, video captions, and reaction text should be handled in message_content extraction methods.
app/jobs/channels/whatsapp/baileys_message_retry_job.rb (4)
Learnt from: CayoPOliveira
PR: fazer-ai/chatwoot#53
File: app/services/whatsapp/baileys_handlers/messages_upsert.rb:7-16
Timestamp: 2025-05-26T20:14:33.177Z
Learning: In the Whatsapp Baileys handlers for this codebase, errors should propagate to the WhatsappController level rather than being caught and handled individually within message processing methods. The user prefers centralized error handling at the controller level over individual message-level error handling.
Learnt from: CayoPOliveira
PR: fazer-ai/chatwoot#20
File: app/services/whatsapp/incoming_message_baileys_service.rb:0-0
Timestamp: 2025-04-08T21:26:26.063Z
Learning: Chatwoot message model (app/models/message.rb) only supports four status states: sent (0), delivered (1), read (2), and failed (3). When mapping from Baileys WhatsApp statuses, only ERROR (0), SERVER_ACK (2), DELIVERY_ACK (3), and READ (4) can be mapped to Chatwoot's statuses. Baileys statuses like PENDING (1) and PLAYED (5) don't have Chatwoot equivalents and should be handled as special cases or logged as unsupported.
Learnt from: CayoPOliveira
PR: fazer-ai/chatwoot#53
File: app/services/whatsapp/baileys_handlers/messages_update.rb:61-61
Timestamp: 2025-05-26T20:14:34.814Z
Learning: In Whatsapp::BaileysHandlers::MessagesUpdate#handle_edited_content, reassigning @raw_message to the nested edited message data is acceptable because this method executes at the end of the corresponding flow, with no subsequent operations depending on the original @raw_message structure.
Learnt from: CayoPOliveira
PR: fazer-ai/chatwoot#53
File: app/services/whatsapp/baileys_handlers/helpers.rb:59-70
Timestamp: 2025-05-26T20:14:33.151Z
Learning: In WhatsApp Baileys message handling, audio, file, sticker, protocol, and unsupported message types don't have extractable text content. Only text messages, image captions, video captions, and reaction text should be handled in message_content extraction methods.
🧬 Code Graph Analysis (1)
app/jobs/channels/whatsapp/baileys_message_retry_job.rb (5)
app/services/whatsapp/providers/whatsapp_baileys_service.rb (1)
include(1-287)app/jobs/send_reply_job.rb (2)
queue_as(1-47)perform(4-26)app/jobs/channels/whatsapp/baileys_failed_message_recovery_job.rb (2)
queue_as(1-52)perform(4-8)app/jobs/application_job.rb (1)
discard_on(1-8)app/helpers/baileys_helper.rb (1)
with_baileys_channel_lock_on_outgoing_message(17-32)
🪛 ast-grep (0.38.1)
app/services/whatsapp/baileys_retry_monitor_service.rb
[warning] 21-26: Found the use of an hardcoded passphrase for RSA. The passphrase can be easily discovered, and therefore should not be stored in source-code. It is recommended to remove the passphrase from source-code, and use system environment variables or a restricted configuration file.
Context: Message.joins(:conversation)
.joins('JOIN inboxes ON conversations.inbox_id = inboxes.id')
.joins('JOIN channel_whatsapp ON inboxes.channel_id = channel_whatsapp.id')
.where("channel_whatsapp.provider = 'baileys'")
.where("messages.additional_attributes ? 'baileys_retry_count'")
.where('messages.created_at > ?', 24.hours.ago)
Note: [CWE-798]: Use of Hard-coded Credentials [OWASP A07:2021]: Identification and Authentication Failures [REFERENCES]
https://cwe.mitre.org/data/definitions/522.html
(hardcoded-secret-rsa-passphrase-ruby)
🔇 Additional comments (14)
config/schedule.yml (1)
50-55: LGTM! Well-structured scheduled job configuration.The configuration correctly sets up the Baileys failed message recovery job to run every 5 minutes, using appropriate queue assignment and following the established patterns in the file.
app/jobs/send_reply_job.rb (2)
21-22: LGTM! Clean refactoring to support provider-specific routing.The change from direct service mapping to the
handle_whatsapp_messagemethod enables provider-specific message handling, which is essential for the Baileys retry mechanism.
38-46: LGTM! Well-implemented provider routing logic.The method correctly identifies Baileys provider channels and delegates them to the specialized retry job while maintaining backward compatibility for other providers.
app/services/whatsapp/providers/whatsapp_baileys_service.rb (1)
233-242: Excellent error handling improvements!The enhanced error handling provides better diagnostics by including response body in error messages and specifically handling network-related exceptions. The logging additions will be valuable for monitoring and debugging message sending failures.
app/jobs/channels/whatsapp/baileys_failed_message_recovery_job.rb (2)
12-40: LGTM! Well-designed recovery mechanism with good error handling.The batch processing, randomized delays, and graceful error handling are well implemented. The use of
rpopfor FIFO processing is appropriate for message recovery.
42-51: LGTM! Proper API availability check prevents unnecessary work.The API availability check is well-implemented with appropriate exception handling and debug logging when the API is unavailable.
app/jobs/channels/whatsapp/baileys_message_retry_job.rb (6)
13-15: LGTM! Proper use of channel locking prevents concurrent message conflicts.The use of
with_baileys_channel_lock_on_outgoing_messageis essential for preventing concurrent outgoing message processing, which aligns with the existing locking system fromBaileysHelper.
16-21: LGTM! Comprehensive error handling with proper delegation.The error handling correctly catches both specific
MessageNotSentErrorand generalStandardError, delegating both to the failure handler while maintaining appropriate logging.
25-35: LGTM! Clean message sending with proper status tracking.The message sending logic correctly delegates to the existing service and updates the message with retry metadata, providing good visibility into retry attempts.
37-55: Well-implemented failure handling with manual retry logic.The failure handling correctly updates message status, implements retry logic with attempt counting, and delegates to persistence after exhausting retries. The manual retry approach gives better control than ActiveJob's built-in mechanism.
57-60: LGTM! Proper exponential backoff with reasonable cap.The exponential backoff calculation correctly doubles the delay with each attempt while capping at 5 minutes to prevent excessively long delays.
62-73: LGTM! Well-structured Redis persistence for failed messages.The persistence logic correctly serializes message data with relevant metadata and uses the appropriate Redis key structure for channel-based organization.
app/services/whatsapp/baileys_retry_monitor_service.rb (2)
36-43: Excellent error handling for JSON parsing.The method correctly handles both missing data and JSON parsing errors. The Redis operation and error handling are implemented properly.
21-26: Ignore the static analysis false positive.The static analysis tool incorrectly flagged this SQL query as containing "hardcoded RSA passphrase". This is clearly a false positive - these are just database JOIN statements for querying message retry statistics.
|
|
||
| { | ||
| total_messages_with_retries: messages_with_retries.count, | ||
| average_retry_count: messages_with_retries.average("(additional_attributes->>'baileys_retry_count')::int"), |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Handle potential nil average gracefully.
The average method could return nil if no records match, which would cause issues when casting to integer.
-average_retry_count: messages_with_retries.average("(additional_attributes->>'baileys_retry_count')::int"),
+average_retry_count: messages_with_retries.average("(additional_attributes->>'baileys_retry_count')::int")&.to_i || 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.
Suggested change
| average_retry_count: messages_with_retries.average("(additional_attributes->>'baileys_retry_count')::int"), | |
| average_retry_count: messages_with_retries.average("(additional_attributes->>'baileys_retry_count')::int")&.to_i || 0, |
🤖 Prompt for AI Agents
In app/services/whatsapp/baileys_retry_monitor_service.rb at line 31, the use of
the average method on messages_with_retries can return nil if there are no
matching records, which may cause errors when casting to integer. To fix this,
add a safe navigation or a fallback value to handle the nil case gracefully,
ensuring the code does not break when average returns nil.
…mediate initial sends - Restructure BaileysMessageRetryJob to inherit from MutexApplicationJob - Remove delays from initial message sending (delays only apply to retries) - Implement FIFO ordering for failed message retries using rpush/lpop - Add BAILEYS_MESSAGE_MUTEX key following existing Redis key patterns - Ensure proper locking logic consistent with other Chatwoot jobs - Update recovery job to maintain FIFO ordering - Follow existing job patterns from HookJob and InstagramEventsJob - Use keyword arguments for boolean parameters to satisfy RuboCop Co-Authored-By: cayo@fazer.ai <cayoproliveira@gmail.com>
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
app/services/whatsapp/baileys_retry_monitor_service.rb (1)
31-31: Handle potential nil average gracefully.The
averagemethod could returnnilif no records match, which would cause issues when casting to integer.-average_retry_count: messages_with_retries.average("(additional_attributes->>'baileys_retry_count')::int"), +average_retry_count: messages_with_retries.average("(additional_attributes->>'baileys_retry_count')::int")&.to_i || 0,
🧹 Nitpick comments (2)
app/jobs/channels/whatsapp/baileys_message_retry_job.rb (2)
53-53: Extract max retry attempts as a constant.The hardcoded value of 3 for max attempts should be extracted as a constant for better maintainability and consistency.
Add a constant at the class level:
MAX_RETRY_ATTEMPTS = 3Then update the condition:
-if attempt_count >= 3 +if attempt_count >= MAX_RETRY_ATTEMPTS
69-72: Extract delay constants for better maintainability.The base delay and maximum delay values should be extracted as constants to make retry timing easier to adjust and document.
Add constants at the class level:
BASE_RETRY_DELAY = 30.seconds MAX_RETRY_DELAY = 5.minutesThen update the method:
-def calculate_retry_delay(attempt_count) - base_delay = 30.seconds - [base_delay * (2 ** (attempt_count - 1)), 5.minutes].min -end +def calculate_retry_delay(attempt_count) + [BASE_RETRY_DELAY * (2 ** (attempt_count - 1)), MAX_RETRY_DELAY].min +end
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
app/jobs/channels/whatsapp/baileys_failed_message_recovery_job.rb(1 hunks)app/jobs/channels/whatsapp/baileys_message_retry_job.rb(1 hunks)app/jobs/send_reply_job.rb(2 hunks)app/services/whatsapp/baileys_retry_monitor_service.rb(1 hunks)lib/redis/alfred.rb(1 hunks)lib/redis/redis_keys.rb(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- lib/redis/redis_keys.rb
🚧 Files skipped from review as they are similar to previous changes (2)
- app/jobs/send_reply_job.rb
- app/jobs/channels/whatsapp/baileys_failed_message_recovery_job.rb
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
Learnt from: CayoPOliveira
PR: fazer-ai/chatwoot#53
File: app/services/whatsapp/baileys_handlers/messages_upsert.rb:7-16
Timestamp: 2025-05-26T20:14:33.177Z
Learning: In the Whatsapp Baileys handlers for this codebase, errors should propagate to the WhatsappController level rather than being caught and handled individually within message processing methods. The user prefers centralized error handling at the controller level over individual message-level error handling.
Learnt from: CayoPOliveira
PR: fazer-ai/chatwoot#53
File: app/services/whatsapp/baileys_handlers/helpers.rb:59-70
Timestamp: 2025-05-26T20:14:33.151Z
Learning: In WhatsApp Baileys message handling, audio, file, sticker, protocol, and unsupported message types don't have extractable text content. Only text messages, image captions, video captions, and reaction text should be handled in message_content extraction methods.
Learnt from: CayoPOliveira
PR: fazer-ai/chatwoot#20
File: app/services/whatsapp/incoming_message_baileys_service.rb:0-0
Timestamp: 2025-04-08T21:26:26.063Z
Learning: Chatwoot message model (app/models/message.rb) only supports four status states: sent (0), delivered (1), read (2), and failed (3). When mapping from Baileys WhatsApp statuses, only ERROR (0), SERVER_ACK (2), DELIVERY_ACK (3), and READ (4) can be mapped to Chatwoot's statuses. Baileys statuses like PENDING (1) and PLAYED (5) don't have Chatwoot equivalents and should be handled as special cases or logged as unsupported.
Learnt from: CayoPOliveira
PR: fazer-ai/chatwoot#53
File: app/services/whatsapp/baileys_handlers/messages_update.rb:61-61
Timestamp: 2025-05-26T20:14:34.814Z
Learning: In Whatsapp::BaileysHandlers::MessagesUpdate#handle_edited_content, reassigning @raw_message to the nested edited message data is acceptable because this method executes at the end of the corresponding flow, with no subsequent operations depending on the original @raw_message structure.
Learnt from: CayoPOliveira
PR: fazer-ai/chatwoot#26
File: app/services/whatsapp/incoming_message_baileys_service.rb:198-207
Timestamp: 2025-04-16T11:55:12.231Z
Learning: In the WhatsApp Baileys API integration, only 'image' and 'video' message types support captions. The 'file', 'audio', and 'sticker' message types do not have caption fields.
Learnt from: CayoPOliveira
PR: fazer-ai/chatwoot#26
File: app/services/whatsapp/incoming_message_baileys_service.rb:198-207
Timestamp: 2025-04-16T11:55:12.231Z
Learning: In the WhatsApp Baileys API integration, only 'image' and 'video' message types support captions. The 'file', 'audio', and 'sticker' message types do not have caption fields.
app/jobs/channels/whatsapp/baileys_message_retry_job.rb (4)
Learnt from: CayoPOliveira
PR: fazer-ai/chatwoot#53
File: app/services/whatsapp/baileys_handlers/messages_upsert.rb:7-16
Timestamp: 2025-05-26T20:14:33.177Z
Learning: In the Whatsapp Baileys handlers for this codebase, errors should propagate to the WhatsappController level rather than being caught and handled individually within message processing methods. The user prefers centralized error handling at the controller level over individual message-level error handling.
Learnt from: CayoPOliveira
PR: fazer-ai/chatwoot#20
File: app/services/whatsapp/incoming_message_baileys_service.rb:0-0
Timestamp: 2025-04-08T21:26:26.063Z
Learning: Chatwoot message model (app/models/message.rb) only supports four status states: sent (0), delivered (1), read (2), and failed (3). When mapping from Baileys WhatsApp statuses, only ERROR (0), SERVER_ACK (2), DELIVERY_ACK (3), and READ (4) can be mapped to Chatwoot's statuses. Baileys statuses like PENDING (1) and PLAYED (5) don't have Chatwoot equivalents and should be handled as special cases or logged as unsupported.
Learnt from: CayoPOliveira
PR: fazer-ai/chatwoot#53
File: app/services/whatsapp/baileys_handlers/messages_update.rb:61-61
Timestamp: 2025-05-26T20:14:34.814Z
Learning: In Whatsapp::BaileysHandlers::MessagesUpdate#handle_edited_content, reassigning @raw_message to the nested edited message data is acceptable because this method executes at the end of the corresponding flow, with no subsequent operations depending on the original @raw_message structure.
Learnt from: CayoPOliveira
PR: fazer-ai/chatwoot#53
File: app/services/whatsapp/baileys_handlers/helpers.rb:59-70
Timestamp: 2025-05-26T20:14:33.151Z
Learning: In WhatsApp Baileys message handling, audio, file, sticker, protocol, and unsupported message types don't have extractable text content. Only text messages, image captions, video captions, and reaction text should be handled in message_content extraction methods.
app/services/whatsapp/baileys_retry_monitor_service.rb (4)
Learnt from: CayoPOliveira
PR: fazer-ai/chatwoot#20
File: app/services/whatsapp/incoming_message_baileys_service.rb:0-0
Timestamp: 2025-04-08T21:26:26.063Z
Learning: Chatwoot message model (app/models/message.rb) only supports four status states: sent (0), delivered (1), read (2), and failed (3). When mapping from Baileys WhatsApp statuses, only ERROR (0), SERVER_ACK (2), DELIVERY_ACK (3), and READ (4) can be mapped to Chatwoot's statuses. Baileys statuses like PENDING (1) and PLAYED (5) don't have Chatwoot equivalents and should be handled as special cases or logged as unsupported.
Learnt from: CayoPOliveira
PR: fazer-ai/chatwoot#53
File: app/services/whatsapp/baileys_handlers/messages_upsert.rb:7-16
Timestamp: 2025-05-26T20:14:33.177Z
Learning: In the Whatsapp Baileys handlers for this codebase, errors should propagate to the WhatsappController level rather than being caught and handled individually within message processing methods. The user prefers centralized error handling at the controller level over individual message-level error handling.
Learnt from: CayoPOliveira
PR: fazer-ai/chatwoot#53
File: app/services/whatsapp/baileys_handlers/helpers.rb:59-70
Timestamp: 2025-05-26T20:14:33.151Z
Learning: In WhatsApp Baileys message handling, audio, file, sticker, protocol, and unsupported message types don't have extractable text content. Only text messages, image captions, video captions, and reaction text should be handled in message_content extraction methods.
Learnt from: CayoPOliveira
PR: fazer-ai/chatwoot#53
File: app/services/whatsapp/baileys_handlers/messages_update.rb:61-61
Timestamp: 2025-05-26T20:14:34.814Z
Learning: In Whatsapp::BaileysHandlers::MessagesUpdate#handle_edited_content, reassigning @raw_message to the nested edited message data is acceptable because this method executes at the end of the corresponding flow, with no subsequent operations depending on the original @raw_message structure.
🧬 Code Graph Analysis (2)
app/jobs/channels/whatsapp/baileys_message_retry_job.rb (4)
app/jobs/send_reply_job.rb (2)
queue_as(1-47)perform(4-26)app/jobs/channels/whatsapp/baileys_failed_message_recovery_job.rb (2)
queue_as(1-52)perform(4-8)app/jobs/application_job.rb (1)
discard_on(1-8)lib/redis/alfred.rb (2)
set(10-12)rpush(56-58)
app/services/whatsapp/baileys_retry_monitor_service.rb (1)
lib/redis/alfred.rb (2)
llen(40-42)lrange(44-46)
🪛 ast-grep (0.38.1)
app/services/whatsapp/baileys_retry_monitor_service.rb
[warning] 21-26: Found the use of an hardcoded passphrase for RSA. The passphrase can be easily discovered, and therefore should not be stored in source-code. It is recommended to remove the passphrase from source-code, and use system environment variables or a restricted configuration file.
Context: Message.joins(:conversation)
.joins('JOIN inboxes ON conversations.inbox_id = inboxes.id')
.joins('JOIN channel_whatsapp ON inboxes.channel_id = channel_whatsapp.id')
.where("channel_whatsapp.provider = 'baileys'")
.where("messages.additional_attributes ? 'baileys_retry_count'")
.where('messages.created_at > ?', 24.hours.ago)
Note: [CWE-798]: Use of Hard-coded Credentials [OWASP A07:2021]: Identification and Authentication Failures [REFERENCES]
https://cwe.mitre.org/data/definitions/522.html
(hardcoded-secret-rsa-passphrase-ruby)
🔇 Additional comments (6)
lib/redis/alfred.rb (1)
56-58: LGTM!The
rpushmethod correctly implements the Redis right push operation, following the established pattern in this module.app/services/whatsapp/baileys_retry_monitor_service.rb (2)
2-19: LGTM!The
failed_message_statsmethod correctly iterates through Baileys channels and collects failure statistics from Redis.
36-43: LGTM!The
get_oldest_failed_messagehelper method properly handles JSON parsing errors and edge cases.app/jobs/channels/whatsapp/baileys_message_retry_job.rb (3)
1-9: LGTM!The job configuration and initial validation logic are well-structured. Good use of distributed locking and proper validation of the Baileys provider.
26-38: LGTM!The
send_baileys_messagemethod correctly sends the message and updates the status with proper retry tracking metadata.
74-85: LGTM!The
persist_failed_message_for_redeliverymethod correctly stores failed message data in Redis using the newrpushmethod, with all necessary metadata for future recovery.
- Create RetrySendReplyJob to process failed messages after channel reconnection - Process messages in 24-hour window, batches of 10 with 1-minute delays - Change message status to sent before retrying via SendReplyJob - Hook job trigger after successful setup_channel_provider for Baileys - Maintain proper error handling and logging Co-Authored-By: cayo@fazer.ai <cayoproliveira@gmail.com>
…quando-o-Baileys-falha' of https://git-manager.devin.ai/proxy/github.com/fazer-ai/chatwoot into Cayo-Oliveira/CU-86a8qebu7/Tratar-reenvio-de-mensagens-quando-o-Baileys-falha
- Remove baileys_failed_message_recovery_job.rb - Remove baileys_message_retry_job.rb - Remove baileys_retry_monitor_service.rb - Revert changes to send_reply_job.rb - Revert changes to whatsapp_baileys_service.rb - Revert changes to schedule.yml, alfred.rb, redis_keys.rb - Keep new RetrySendReplyJob implementation as per refined requirements Co-Authored-By: cayo@fazer.ai <cayoproliveira@gmail.com>
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
app/jobs/channels/whatsapp/retry_send_reply_job.rb (1)
22-30: Optimize database query performance.The query uses string-based JOIN which could be improved for readability and potential performance optimization.
- def find_failed_messages(channel) - Message.joins(:conversation) - .joins('JOIN inboxes ON conversations.inbox_id = inboxes.id') - .where(inboxes: { channel: channel }) - .where(status: :failed) - .where('messages.created_at > ?', 24.hours.ago) - .where(message_type: :outgoing) - .order(:created_at) - end + def find_failed_messages(channel) + Message.joins(conversation: :inbox) + .where(conversations: { inboxes: { channel: channel } }) + .where(status: :failed) + .where('messages.created_at > ?', 24.hours.ago) + .where(message_type: :outgoing) + .order(:created_at) + endapp/controllers/api/v1/accounts/inboxes_controller.rb (1)
74-79: Improve code readability by extracting conditional logic.The conditional logic for enqueuing the retry job makes line 75 quite long and reduces readability.
if channel.setup_channel_provider - Channels::Whatsapp::RetrySendReplyJob.perform_later(channel.id) if channel.is_a?(Channel::Whatsapp) && channel.provider == 'baileys' + enqueue_baileys_retry_job(channel) head :ok else render json: { error: 'Channel setup failed' }, status: :unprocessable_entity endAdd the extracted method:
def enqueue_baileys_retry_job(channel) return unless channel.is_a?(Channel::Whatsapp) && channel.provider == 'baileys' Channels::Whatsapp::RetrySendReplyJob.perform_later(channel.id) endThis improves readability and makes the intent clearer.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
app/controllers/api/v1/accounts/inboxes_controller.rb(1 hunks)app/jobs/channels/whatsapp/retry_send_reply_job.rb(1 hunks)sage.last.additional_attributes(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- sage.last.additional_attributes
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
Learnt from: CayoPOliveira
PR: fazer-ai/chatwoot#53
File: app/services/whatsapp/baileys_handlers/messages_upsert.rb:7-16
Timestamp: 2025-05-26T20:14:33.177Z
Learning: In the Whatsapp Baileys handlers for this codebase, errors should propagate to the WhatsappController level rather than being caught and handled individually within message processing methods. The user prefers centralized error handling at the controller level over individual message-level error handling.
Learnt from: CayoPOliveira
PR: fazer-ai/chatwoot#53
File: app/services/whatsapp/baileys_handlers/helpers.rb:59-70
Timestamp: 2025-05-26T20:14:33.151Z
Learning: In WhatsApp Baileys message handling, audio, file, sticker, protocol, and unsupported message types don't have extractable text content. Only text messages, image captions, video captions, and reaction text should be handled in message_content extraction methods.
Learnt from: CayoPOliveira
PR: fazer-ai/chatwoot#20
File: app/services/whatsapp/incoming_message_baileys_service.rb:0-0
Timestamp: 2025-04-08T21:26:26.063Z
Learning: Chatwoot message model (app/models/message.rb) only supports four status states: sent (0), delivered (1), read (2), and failed (3). When mapping from Baileys WhatsApp statuses, only ERROR (0), SERVER_ACK (2), DELIVERY_ACK (3), and READ (4) can be mapped to Chatwoot's statuses. Baileys statuses like PENDING (1) and PLAYED (5) don't have Chatwoot equivalents and should be handled as special cases or logged as unsupported.
Learnt from: CayoPOliveira
PR: fazer-ai/chatwoot#53
File: app/services/whatsapp/baileys_handlers/messages_update.rb:61-61
Timestamp: 2025-05-26T20:14:34.814Z
Learning: In Whatsapp::BaileysHandlers::MessagesUpdate#handle_edited_content, reassigning @raw_message to the nested edited message data is acceptable because this method executes at the end of the corresponding flow, with no subsequent operations depending on the original @raw_message structure.
Learnt from: CayoPOliveira
PR: fazer-ai/chatwoot#26
File: app/services/whatsapp/incoming_message_baileys_service.rb:198-207
Timestamp: 2025-04-16T11:55:12.231Z
Learning: In the WhatsApp Baileys API integration, only 'image' and 'video' message types support captions. The 'file', 'audio', and 'sticker' message types do not have caption fields.
Learnt from: CayoPOliveira
PR: fazer-ai/chatwoot#26
File: app/services/whatsapp/incoming_message_baileys_service.rb:198-207
Timestamp: 2025-04-16T11:55:12.231Z
Learning: In the WhatsApp Baileys API integration, only 'image' and 'video' message types support captions. The 'file', 'audio', and 'sticker' message types do not have caption fields.
app/controllers/api/v1/accounts/inboxes_controller.rb (3)
Learnt from: CayoPOliveira
PR: fazer-ai/chatwoot#53
File: app/services/whatsapp/baileys_handlers/messages_upsert.rb:7-16
Timestamp: 2025-05-26T20:14:33.177Z
Learning: In the Whatsapp Baileys handlers for this codebase, errors should propagate to the WhatsappController level rather than being caught and handled individually within message processing methods. The user prefers centralized error handling at the controller level over individual message-level error handling.
Learnt from: CayoPOliveira
PR: fazer-ai/chatwoot#20
File: app/services/whatsapp/incoming_message_baileys_service.rb:0-0
Timestamp: 2025-04-08T21:26:26.063Z
Learning: Chatwoot message model (app/models/message.rb) only supports four status states: sent (0), delivered (1), read (2), and failed (3). When mapping from Baileys WhatsApp statuses, only ERROR (0), SERVER_ACK (2), DELIVERY_ACK (3), and READ (4) can be mapped to Chatwoot's statuses. Baileys statuses like PENDING (1) and PLAYED (5) don't have Chatwoot equivalents and should be handled as special cases or logged as unsupported.
Learnt from: CayoPOliveira
PR: fazer-ai/chatwoot#53
File: app/services/whatsapp/baileys_handlers/messages_update.rb:61-61
Timestamp: 2025-05-26T20:14:34.814Z
Learning: In Whatsapp::BaileysHandlers::MessagesUpdate#handle_edited_content, reassigning @raw_message to the nested edited message data is acceptable because this method executes at the end of the corresponding flow, with no subsequent operations depending on the original @raw_message structure.
app/jobs/channels/whatsapp/retry_send_reply_job.rb (3)
Learnt from: CayoPOliveira
PR: fazer-ai/chatwoot#20
File: app/services/whatsapp/incoming_message_baileys_service.rb:0-0
Timestamp: 2025-04-08T21:26:26.063Z
Learning: Chatwoot message model (app/models/message.rb) only supports four status states: sent (0), delivered (1), read (2), and failed (3). When mapping from Baileys WhatsApp statuses, only ERROR (0), SERVER_ACK (2), DELIVERY_ACK (3), and READ (4) can be mapped to Chatwoot's statuses. Baileys statuses like PENDING (1) and PLAYED (5) don't have Chatwoot equivalents and should be handled as special cases or logged as unsupported.
Learnt from: CayoPOliveira
PR: fazer-ai/chatwoot#53
File: app/services/whatsapp/baileys_handlers/messages_upsert.rb:7-16
Timestamp: 2025-05-26T20:14:33.177Z
Learning: In the Whatsapp Baileys handlers for this codebase, errors should propagate to the WhatsappController level rather than being caught and handled individually within message processing methods. The user prefers centralized error handling at the controller level over individual message-level error handling.
Learnt from: CayoPOliveira
PR: fazer-ai/chatwoot#53
File: app/services/whatsapp/baileys_handlers/messages_update.rb:61-61
Timestamp: 2025-05-26T20:14:34.814Z
Learning: In Whatsapp::BaileysHandlers::MessagesUpdate#handle_edited_content, reassigning @raw_message to the nested edited message data is acceptable because this method executes at the end of the corresponding flow, with no subsequent operations depending on the original @raw_message structure.
🪛 ast-grep (0.38.6)
app/jobs/channels/whatsapp/retry_send_reply_job.rb
[warning] 22-26: Found the use of an hardcoded passphrase for RSA. The passphrase can be easily discovered, and therefore should not be stored in source-code. It is recommended to remove the passphrase from source-code, and use system environment variables or a restricted configuration file.
Context: Message.joins(:conversation)
.joins('JOIN inboxes ON conversations.inbox_id = inboxes.id')
.where(inboxes: { channel: channel })
.where(status: :failed)
.where('messages.created_at > ?', 24.hours.ago)
Note: [CWE-798]: Use of Hard-coded Credentials [OWASP A07:2021]: Identification and Authentication Failures [REFERENCES]
https://cwe.mitre.org/data/definitions/522.html
(hardcoded-secret-rsa-passphrase-ruby)
🔇 Additional comments (1)
app/jobs/channels/whatsapp/retry_send_reply_job.rb (1)
1-6: LGTM! Proper job setup with early return guard.The job structure follows Rails conventions with appropriate queue assignment and early exit for non-Baileys channels.
Comment on lines
8
to
18
| failed_messages = find_failed_messages(channel) | ||
| return if failed_messages.empty? | ||
|
|
||
| Rails.logger.info "Processing #{failed_messages.count} failed messages for Baileys channel #{channel_id}" | ||
|
|
||
| failed_messages.in_batches(of: 10).with_index do |batch, _index| | ||
| process_message_batch(batch) | ||
|
|
||
| sleep(60) unless batch.last == failed_messages.last | ||
| end | ||
| end |
There was a problem hiding this comment.
Fix potential race condition in batch processing.
The current implementation has a timing issue where the status is updated to :sent before the retry job is actually enqueued, creating a window where the message status doesn't reflect reality.
- failed_messages.in_batches(of: 10).with_index do |batch, _index|
+ failed_messages.in_batches(of: 10).with_index do |batch, index|
process_message_batch(batch)
- sleep(60) unless batch.last == failed_messages.last
+ sleep(60) unless index == (failed_messages.count / 10.0).ceil - 1
endAlso consider moving the status update after successful job enqueuing in process_message_batch.
📝 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.
Suggested change
| failed_messages = find_failed_messages(channel) | |
| return if failed_messages.empty? | |
| Rails.logger.info "Processing #{failed_messages.count} failed messages for Baileys channel #{channel_id}" | |
| failed_messages.in_batches(of: 10).with_index do |batch, _index| | |
| process_message_batch(batch) | |
| sleep(60) unless batch.last == failed_messages.last | |
| end | |
| end | |
| failed_messages = find_failed_messages(channel) | |
| return if failed_messages.empty? | |
| Rails.logger.info "Processing #{failed_messages.count} failed messages for Baileys channel #{channel_id}" | |
| failed_messages.in_batches(of: 10).with_index do |batch, index| | |
| process_message_batch(batch) | |
| sleep(60) unless index == (failed_messages.count / 10.0).ceil - 1 | |
| end | |
| end |
🤖 Prompt for AI Agents
In app/jobs/channels/whatsapp/retry_send_reply_job.rb around lines 8 to 18, the
status of messages is updated to :sent before the retry job is enqueued, causing
a race condition where the status may not reflect the actual state. To fix this,
move the status update inside the process_message_batch method so it happens
only after the retry job has been successfully enqueued, ensuring the message
status accurately represents the current processing state.
Comment on lines
32
to
43
| def process_message_batch(messages) | ||
| messages.each do |message| | ||
| message.update!(status: :sent) | ||
|
|
||
| SendReplyJob.perform_later(message.id) | ||
|
|
||
| Rails.logger.info "Scheduled retry for failed message #{message.id}" | ||
| rescue StandardError => e | ||
| Rails.logger.error "Error processing failed message #{message.id}: #{e.message}" | ||
| message.update!(status: :failed) | ||
| end | ||
| end |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Improve atomicity of status updates and job enqueuing.
The current implementation updates the message status to :sent before enqueuing the retry job, which creates an inconsistent state if the job enqueuing fails.
def process_message_batch(messages)
messages.each do |message|
- message.update!(status: :sent)
-
- SendReplyJob.perform_later(message.id)
-
- Rails.logger.info "Scheduled retry for failed message #{message.id}"
+ ActiveRecord::Base.transaction do
+ SendReplyJob.perform_later(message.id)
+ message.update!(status: :sent)
+ Rails.logger.info "Scheduled retry for failed message #{message.id}"
+ end
rescue StandardError => e
Rails.logger.error "Error processing failed message #{message.id}: #{e.message}"
message.update!(status: :failed)
end
endThis ensures the status is only updated after successful job enqueuing.
📝 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.
Suggested change
| def process_message_batch(messages) | |
| messages.each do |message| | |
| message.update!(status: :sent) | |
| SendReplyJob.perform_later(message.id) | |
| Rails.logger.info "Scheduled retry for failed message #{message.id}" | |
| rescue StandardError => e | |
| Rails.logger.error "Error processing failed message #{message.id}: #{e.message}" | |
| message.update!(status: :failed) | |
| end | |
| end | |
| def process_message_batch(messages) | |
| messages.each do |message| | |
| ActiveRecord::Base.transaction do | |
| SendReplyJob.perform_later(message.id) | |
| message.update!(status: :sent) | |
| Rails.logger.info "Scheduled retry for failed message #{message.id}" | |
| end | |
| rescue StandardError => e | |
| Rails.logger.error "Error processing failed message #{message.id}: #{e.message}" | |
| message.update!(status: :failed) | |
| end | |
| end |
🤖 Prompt for AI Agents
In app/jobs/channels/whatsapp/retry_send_reply_job.rb around lines 32 to 43, the
message status is updated to :sent before enqueuing the retry job, which can
cause inconsistent state if enqueuing fails. To fix this, move the
message.update!(status: :sent) call to after the
SendReplyJob.perform_later(message.id) call, so the status is only updated after
the job is successfully enqueued. Keep the rescue block to handle errors and
update the status to :failed if any exception occurs.
- Remove retry job trigger from setup_channel_provider in inbox controller - Add retry job trigger to update_provider_connection when connection == 'open' - Ensures retry job only runs when Baileys connection actually opens - Maintains existing RetrySendReplyJob functionality for failed message processing Co-Authored-By: cayo@fazer.ai <cayoproliveira@gmail.com>
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
app/controllers/api/v1/accounts/inboxes_controller.rb(1 hunks)app/models/channel/whatsapp.rb(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- app/controllers/api/v1/accounts/inboxes_controller.rb
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: CayoPOliveira
PR: fazer-ai/chatwoot#53
File: app/services/whatsapp/baileys_handlers/messages_upsert.rb:7-16
Timestamp: 2025-05-26T20:14:33.177Z
Learning: In the Whatsapp Baileys handlers for this codebase, errors should propagate to the WhatsappController level rather than being caught and handled individually within message processing methods. The user prefers centralized error handling at the controller level over individual message-level error handling.
Learnt from: CayoPOliveira
PR: fazer-ai/chatwoot#53
File: app/services/whatsapp/baileys_handlers/helpers.rb:59-70
Timestamp: 2025-05-26T20:14:33.151Z
Learning: In WhatsApp Baileys message handling, audio, file, sticker, protocol, and unsupported message types don't have extractable text content. Only text messages, image captions, video captions, and reaction text should be handled in message_content extraction methods.
Learnt from: CayoPOliveira
PR: fazer-ai/chatwoot#20
File: app/services/whatsapp/incoming_message_baileys_service.rb:0-0
Timestamp: 2025-04-08T21:26:26.063Z
Learning: Chatwoot message model (app/models/message.rb) only supports four status states: sent (0), delivered (1), read (2), and failed (3). When mapping from Baileys WhatsApp statuses, only ERROR (0), SERVER_ACK (2), DELIVERY_ACK (3), and READ (4) can be mapped to Chatwoot's statuses. Baileys statuses like PENDING (1) and PLAYED (5) don't have Chatwoot equivalents and should be handled as special cases or logged as unsupported.
Learnt from: CayoPOliveira
PR: fazer-ai/chatwoot#53
File: app/services/whatsapp/baileys_handlers/messages_update.rb:61-61
Timestamp: 2025-05-26T20:14:34.814Z
Learning: In Whatsapp::BaileysHandlers::MessagesUpdate#handle_edited_content, reassigning @raw_message to the nested edited message data is acceptable because this method executes at the end of the corresponding flow, with no subsequent operations depending on the original @raw_message structure.
Learnt from: CayoPOliveira
PR: fazer-ai/chatwoot#26
File: app/services/whatsapp/incoming_message_baileys_service.rb:198-207
Timestamp: 2025-04-16T11:55:12.231Z
Learning: In the WhatsApp Baileys API integration, only 'image' and 'video' message types support captions. The 'file', 'audio', and 'sticker' message types do not have caption fields.
Learnt from: CayoPOliveira
PR: fazer-ai/chatwoot#26
File: app/services/whatsapp/incoming_message_baileys_service.rb:198-207
Timestamp: 2025-04-16T11:55:12.231Z
Learning: In the WhatsApp Baileys API integration, only 'image' and 'video' message types support captions. The 'file', 'audio', and 'sticker' message types do not have caption fields.
app/models/channel/whatsapp.rb (4)
Learnt from: CayoPOliveira
PR: fazer-ai/chatwoot#53
File: app/services/whatsapp/baileys_handlers/messages_update.rb:61-61
Timestamp: 2025-05-26T20:14:34.814Z
Learning: In Whatsapp::BaileysHandlers::MessagesUpdate#handle_edited_content, reassigning @raw_message to the nested edited message data is acceptable because this method executes at the end of the corresponding flow, with no subsequent operations depending on the original @raw_message structure.
Learnt from: CayoPOliveira
PR: fazer-ai/chatwoot#20
File: app/services/whatsapp/incoming_message_baileys_service.rb:0-0
Timestamp: 2025-04-08T21:26:26.063Z
Learning: Chatwoot message model (app/models/message.rb) only supports four status states: sent (0), delivered (1), read (2), and failed (3). When mapping from Baileys WhatsApp statuses, only ERROR (0), SERVER_ACK (2), DELIVERY_ACK (3), and READ (4) can be mapped to Chatwoot's statuses. Baileys statuses like PENDING (1) and PLAYED (5) don't have Chatwoot equivalents and should be handled as special cases or logged as unsupported.
Learnt from: CayoPOliveira
PR: fazer-ai/chatwoot#53
File: app/services/whatsapp/baileys_handlers/messages_upsert.rb:7-16
Timestamp: 2025-05-26T20:14:33.177Z
Learning: In the Whatsapp Baileys handlers for this codebase, errors should propagate to the WhatsappController level rather than being caught and handled individually within message processing methods. The user prefers centralized error handling at the controller level over individual message-level error handling.
Learnt from: CayoPOliveira
PR: fazer-ai/chatwoot#53
File: app/services/whatsapp/baileys_handlers/helpers.rb:59-70
Timestamp: 2025-05-26T20:14:33.151Z
Learning: In WhatsApp Baileys message handling, audio, file, sticker, protocol, and unsupported message types don't have extractable text content. Only text messages, image captions, video captions, and reaction text should be handled in message_content extraction methods.
| # Trigger retry job for failed messages when Baileys connection opens | ||
| return unless provider == 'baileys' && provider_connection[:connection] == 'open' | ||
|
|
||
| Channels::Whatsapp::RetrySendReplyJob.perform_later(id) |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Verify job class existence and prevent duplicate job enqueues.
The code references Channels::Whatsapp::RetrySendReplyJob but this job class wasn't provided for review. Also consider preventing duplicate job enqueues if this method is called multiple times rapidly.
🏁 Script executed:
#!/bin/bash
# Description: Verify the RetrySendReplyJob exists and check for duplicate prevention mechanisms
# Check if the retry job file exists
fd "retry_send_reply_job.rb" --type f
# Search for any existing duplicate prevention mechanisms
rg -A 5 -B 5 "RetrySendReplyJob" --type ruby
# Check if there are any job queuing locks or unique job mechanisms
rg -A 3 -B 3 "perform_later.*id" --type rubyLength of output: 19588
Ensure RetrySendReplyJob exists and guard against duplicate enqueues
The job Channels::Whatsapp::RetrySendReplyJob is defined at
• app/jobs/channels/whatsapp/retry_send_reply_job.rb
However, there’s no mechanism today to prevent enqueuing the same retry job multiple times if the connection flaps open rapidly. Without deduplication, you may schedule redundant retries for the same channel ID.
Recommendations:
- Introduce a uniqueness strategy (e.g. using
sidekiq-unique-jobsor Rails’ ActiveJob plugins) so only one pending job exists per channel ID. - Or, before calling
perform_later, check/set a simple Redis lock keyed by"whatsapp:retry:#{id}"with an expiration matching your retry window.
Example using Sidekiq Unique Jobs:
class Channels::Whatsapp::RetrySendReplyJob < ApplicationJob
include Sidekiq::Worker
sidekiq_options queue: :low, lock: :until_executed, lock_args: ->(channel_id) { [channel_id] }
def perform(channel_id)
# …
end
endPlease update app/models/channel/whatsapp.rb accordingly to avoid duplicated retries.
🤖 Prompt for AI Agents
In app/models/channel/whatsapp.rb at line 75, the call to
Channels::Whatsapp::RetrySendReplyJob.perform_later(id) can enqueue duplicate
jobs if the connection flaps. To fix this, implement a uniqueness mechanism for
the job, such as using sidekiq-unique-jobs by adding lock options in the job
class, or add a Redis-based lock check before enqueueing to ensure only one
retry job per channel ID is scheduled at a time. Update the job or the enqueue
logic accordingly to prevent duplicate retries.
app/models/channel/whatsapp.rb
Outdated
Comment on lines
72
to
75
| # Trigger retry job for failed messages when Baileys connection opens | ||
| return unless provider == 'baileys' && provider_connection[:connection] == 'open' | ||
|
|
||
| Channels::Whatsapp::RetrySendReplyJob.perform_later(id) |
There was a problem hiding this comment.
Fix hash key access inconsistency and add error handling.
The code uses symbol keys to access provider_connection[:connection] but the existing provider_connection_data method uses string keys (provider_connection['connection']). Since the provider_connection attribute is stored as JSONB, it typically uses string keys.
Additionally, consider adding error handling for the job enqueue and nil checks.
- # Trigger retry job for failed messages when Baileys connection opens
- return unless provider == 'baileys' && provider_connection[:connection] == 'open'
-
- Channels::Whatsapp::RetrySendReplyJob.perform_later(id)
+ # Trigger retry job for failed messages when Baileys connection opens
+ return unless provider == 'baileys' && provider_connection&.dig('connection') == 'open'
+
+ begin
+ Channels::Whatsapp::RetrySendReplyJob.perform_later(id)
+ rescue StandardError => e
+ Rails.logger.error "Failed to enqueue retry job for WhatsApp channel #{id}: #{e.message}"
+ end📝 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.
Suggested change
| # Trigger retry job for failed messages when Baileys connection opens | |
| return unless provider == 'baileys' && provider_connection[:connection] == 'open' | |
| Channels::Whatsapp::RetrySendReplyJob.perform_later(id) | |
| # Trigger retry job for failed messages when Baileys connection opens | |
| return unless provider == 'baileys' && provider_connection&.dig('connection') == 'open' | |
| begin | |
| Channels::Whatsapp::RetrySendReplyJob.perform_later(id) | |
| rescue StandardError => e | |
| Rails.logger.error "Failed to enqueue retry job for WhatsApp channel #{id}: #{e.message}" | |
| end |
🤖 Prompt for AI Agents
In app/models/channel/whatsapp.rb around lines 72 to 75, change the hash key
access from symbol to string for provider_connection by using
provider_connection['connection'] instead of provider_connection[:connection] to
match the JSONB storage format. Add nil checks to ensure provider_connection is
present before accessing its keys. Wrap the job enqueue call in error handling
to catch and log any exceptions during perform_later invocation.
- Add discard_on StandardError to prevent retries after 3 attempts - Set message status to failed when job is discarded - Add proper error logging for discarded messages - Follows existing Chatwoot job patterns for retry handling Co-Authored-By: cayo@fazer.ai <cayoproliveira@gmail.com>
- Create CustomExceptions::SendReplyJob::MaxRetriesExceeded error class - Replace discard_on StandardError with custom error in SendReplyJob - Add I18n translation for max retries exceeded message - Follow existing Chatwoot custom exception patterns - Ensures proper error handling when messages fail after 3 retry attempts Co-Authored-By: cayo@fazer.ai <cayoproliveira@gmail.com>
- Replace custom error with existing Whatsapp::Providers::WhatsappBaileysService::MessageNotSentError - Remove unnecessary custom error class lib/custom_exceptions/send_reply_job.rb - Clean up I18n translation that's no longer needed - Use Baileys-specific error for more precise error handling - Maintains message status update to failed when Baileys send fails Co-Authored-By: cayo@fazer.ai <cayoproliveira@gmail.com>
…n RetrySendReplyJob
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
app/jobs/channels/whatsapp/retry_send_reply_job.rb (1)
32-41: Address race condition and improve atomicity.The current implementation has timing issues where the status is updated to
:sentbefore the retry job is enqueued, creating inconsistent state if enqueuing fails.def process_message_batch(message) - message.update!(status: :sent) - - SendReplyJob.perform_later(message.id) - - Rails.logger.info "Scheduled retry for failed message #{message.id}" + ActiveRecord::Base.transaction do + SendReplyJob.perform_later(message.id) + message.update!(status: :sent) + Rails.logger.info "Scheduled retry for failed message #{message.id}" + end rescue StandardError => e Rails.logger.error "Error processing failed message #{message.id}: #{e.message}" message.update!(status: :failed) end
🧹 Nitpick comments (3)
app/jobs/channels/whatsapp/retry_send_reply_job.rb (3)
32-32: Rename method to reflect single message processing.The method name
process_message_batchis misleading since it only processes a single message, not a batch.- def process_message_batch(message) + def process_message(message)
13-17: Consider optimizing sequential processing for scalability.The current implementation processes messages sequentially with a 10-second sleep between each message. This approach may not scale well for channels with many failed messages and could lead to job timeouts.
Consider implementing batch processing with configurable batch sizes:
- failed_messages.each do |message| - process_message(message) - - sleep(10) unless message == failed_messages.last - end + failed_messages.in_batches(of: 10) do |batch| + batch.each { |message| process_message(message) } + sleep(60) unless batch.last == failed_messages.last + end
22-30: Optimize database query with eager loading.The current query performs N+1 queries when accessing conversation and inbox data. Consider eager loading related associations to improve performance.
def find_failed_messages(channel) - Message.joins(:conversation) - .joins('JOIN inboxes ON conversations.inbox_id = inboxes.id') + Message.joins(conversation: :inbox) .includes(conversation: :inbox) .where(inboxes: { channel: channel }) .where(status: :failed) .where('messages.created_at > ?', 24.hours.ago) .where(message_type: :outgoing) .order(:created_at) end
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
app/jobs/channels/whatsapp/retry_send_reply_job.rb(1 hunks)app/jobs/send_reply_job.rb(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- app/jobs/send_reply_job.rb
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: CayoPOliveira
PR: fazer-ai/chatwoot#53
File: app/services/whatsapp/baileys_handlers/messages_upsert.rb:7-16
Timestamp: 2025-05-26T20:14:33.177Z
Learning: In the Whatsapp Baileys handlers for this codebase, errors should propagate to the WhatsappController level rather than being caught and handled individually within message processing methods. The user prefers centralized error handling at the controller level over individual message-level error handling.
Learnt from: CayoPOliveira
PR: fazer-ai/chatwoot#53
File: app/services/whatsapp/baileys_handlers/helpers.rb:59-70
Timestamp: 2025-05-26T20:14:33.151Z
Learning: In WhatsApp Baileys message handling, audio, file, sticker, protocol, and unsupported message types don't have extractable text content. Only text messages, image captions, video captions, and reaction text should be handled in message_content extraction methods.
Learnt from: CayoPOliveira
PR: fazer-ai/chatwoot#20
File: app/services/whatsapp/incoming_message_baileys_service.rb:0-0
Timestamp: 2025-04-08T21:26:26.063Z
Learning: Chatwoot message model (app/models/message.rb) only supports four status states: sent (0), delivered (1), read (2), and failed (3). When mapping from Baileys WhatsApp statuses, only ERROR (0), SERVER_ACK (2), DELIVERY_ACK (3), and READ (4) can be mapped to Chatwoot's statuses. Baileys statuses like PENDING (1) and PLAYED (5) don't have Chatwoot equivalents and should be handled as special cases or logged as unsupported.
Learnt from: CayoPOliveira
PR: fazer-ai/chatwoot#53
File: app/services/whatsapp/baileys_handlers/messages_update.rb:61-61
Timestamp: 2025-05-26T20:14:34.814Z
Learning: In Whatsapp::BaileysHandlers::MessagesUpdate#handle_edited_content, reassigning @raw_message to the nested edited message data is acceptable because this method executes at the end of the corresponding flow, with no subsequent operations depending on the original @raw_message structure.
Learnt from: CayoPOliveira
PR: fazer-ai/chatwoot#26
File: app/services/whatsapp/incoming_message_baileys_service.rb:198-207
Timestamp: 2025-04-16T11:55:12.231Z
Learning: In the WhatsApp Baileys API integration, only 'image' and 'video' message types support captions. The 'file', 'audio', and 'sticker' message types do not have caption fields.
Learnt from: CayoPOliveira
PR: fazer-ai/chatwoot#26
File: app/services/whatsapp/incoming_message_baileys_service.rb:198-207
Timestamp: 2025-04-16T11:55:12.231Z
Learning: In the WhatsApp Baileys API integration, only 'image' and 'video' message types support captions. The 'file', 'audio', and 'sticker' message types do not have caption fields.
app/jobs/channels/whatsapp/retry_send_reply_job.rb (4)
Learnt from: CayoPOliveira
PR: fazer-ai/chatwoot#20
File: app/services/whatsapp/incoming_message_baileys_service.rb:0-0
Timestamp: 2025-04-08T21:26:26.063Z
Learning: Chatwoot message model (app/models/message.rb) only supports four status states: sent (0), delivered (1), read (2), and failed (3). When mapping from Baileys WhatsApp statuses, only ERROR (0), SERVER_ACK (2), DELIVERY_ACK (3), and READ (4) can be mapped to Chatwoot's statuses. Baileys statuses like PENDING (1) and PLAYED (5) don't have Chatwoot equivalents and should be handled as special cases or logged as unsupported.
Learnt from: CayoPOliveira
PR: fazer-ai/chatwoot#53
File: app/services/whatsapp/baileys_handlers/messages_upsert.rb:7-16
Timestamp: 2025-05-26T20:14:33.177Z
Learning: In the Whatsapp Baileys handlers for this codebase, errors should propagate to the WhatsappController level rather than being caught and handled individually within message processing methods. The user prefers centralized error handling at the controller level over individual message-level error handling.
Learnt from: CayoPOliveira
PR: fazer-ai/chatwoot#53
File: app/services/whatsapp/baileys_handlers/messages_update.rb:61-61
Timestamp: 2025-05-26T20:14:34.814Z
Learning: In Whatsapp::BaileysHandlers::MessagesUpdate#handle_edited_content, reassigning @raw_message to the nested edited message data is acceptable because this method executes at the end of the corresponding flow, with no subsequent operations depending on the original @raw_message structure.
Learnt from: CayoPOliveira
PR: fazer-ai/chatwoot#53
File: app/services/whatsapp/baileys_handlers/helpers.rb:59-70
Timestamp: 2025-05-26T20:14:33.151Z
Learning: In WhatsApp Baileys message handling, audio, file, sticker, protocol, and unsupported message types don't have extractable text content. Only text messages, image captions, video captions, and reaction text should be handled in message_content extraction methods.
🪛 ast-grep (0.38.6)
app/jobs/channels/whatsapp/retry_send_reply_job.rb
[warning] 22-26: Found the use of an hardcoded passphrase for RSA. The passphrase can be easily discovered, and therefore should not be stored in source-code. It is recommended to remove the passphrase from source-code, and use system environment variables or a restricted configuration file.
Context: Message.joins(:conversation)
.joins('JOIN inboxes ON conversations.inbox_id = inboxes.id')
.where(inboxes: { channel: channel })
.where(status: :failed)
.where('messages.created_at > ?', 24.hours.ago)
Note: [CWE-798]: Use of Hard-coded Credentials [OWASP A07:2021]: Identification and Authentication Failures [REFERENCES]
https://cwe.mitre.org/data/definitions/522.html
(hardcoded-secret-rsa-passphrase-ruby)
🔇 Additional comments (2)
app/jobs/channels/whatsapp/retry_send_reply_job.rb (2)
1-2: LGTM: Appropriate job configuration.The job is correctly configured with high priority queue, which is suitable for retry operations that should be processed promptly.
4-6: LGTM: Proper provider validation.The early return for non-Baileys providers is correct and prevents unnecessary processing.
- Add debug logging to track when SendReplyJob starts executing - Fix trailing whitespace issue that could cause syntax problems - Improve error message in discard_on clause for better debugging Co-Authored-By: cayo@fazer.ai <cayoproliveira@gmail.com>
- Fix NameError: uninitialized constant Net::TimeoutError - Use correct Timeout::Error class for connection timeout handling - Ensures proper error wrapping in MessageNotSentError for discard_on clause Co-Authored-By: cayo@fazer.ai <cayoproliveira@gmail.com>
…tempt retry - Replace discard_on with retry_on to allow 3 retry attempts - Add callback to set message status to failed after all retries exhausted - Use exponential backoff for retry timing - This ensures proper retry behavior before marking messages as failed Co-Authored-By: cayo@fazer.ai <cayoproliveira@gmail.com>
… process_message_batch Co-Authored-By: cayo@fazer.ai <cayoproliveira@gmail.com>
…nger to polynomially_longer
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
app/jobs/channels/whatsapp/retry_send_reply_job.rb (1)
1-18: Consider implementing exponential backoff and improved throttling.While the current implementation is functional, it lacks the exponential backoff mechanism mentioned in the PR objectives. The fixed 10-second sleep between messages might not be sufficient for preventing thundering herd problems at scale.
Consider implementing:
- Exponential backoff based on retry attempts
- Dynamic throttling based on API response rates
- Batch processing with configurable delays
- sleep(10) unless message == failed_messages.last + sleep(calculate_backoff_delay(message)) unless message == failed_messages.lastWhere
calculate_backoff_delaycould consider factors like retry count and recent API response times.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
app/jobs/channels/whatsapp/retry_send_reply_job.rb(1 hunks)app/jobs/send_reply_job.rb(1 hunks)app/models/channel/whatsapp.rb(1 hunks)app/services/whatsapp/send_on_whatsapp_service.rb(1 hunks)spec/jobs/channels/whatsapp/retry_send_reply_job_spec.rb(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- app/jobs/send_reply_job.rb
🚧 Files skipped from review as they are similar to previous changes (1)
- app/models/channel/whatsapp.rb
🧰 Additional context used
🧠 Learnings (4)
📓 Common learnings
Learnt from: CayoPOliveira
PR: fazer-ai/chatwoot#53
File: app/services/whatsapp/baileys_handlers/messages_upsert.rb:7-16
Timestamp: 2025-05-26T20:14:33.177Z
Learning: In the Whatsapp Baileys handlers for this codebase, errors should propagate to the WhatsappController level rather than being caught and handled individually within message processing methods. The user prefers centralized error handling at the controller level over individual message-level error handling.
Learnt from: CayoPOliveira
PR: fazer-ai/chatwoot#53
File: app/services/whatsapp/baileys_handlers/helpers.rb:59-70
Timestamp: 2025-05-26T20:14:33.151Z
Learning: In WhatsApp Baileys message handling, audio, file, sticker, protocol, and unsupported message types don't have extractable text content. Only text messages, image captions, video captions, and reaction text should be handled in message_content extraction methods.
Learnt from: CayoPOliveira
PR: fazer-ai/chatwoot#20
File: app/services/whatsapp/incoming_message_baileys_service.rb:0-0
Timestamp: 2025-04-08T21:26:26.063Z
Learning: Chatwoot message model (app/models/message.rb) only supports four status states: sent (0), delivered (1), read (2), and failed (3). When mapping from Baileys WhatsApp statuses, only ERROR (0), SERVER_ACK (2), DELIVERY_ACK (3), and READ (4) can be mapped to Chatwoot's statuses. Baileys statuses like PENDING (1) and PLAYED (5) don't have Chatwoot equivalents and should be handled as special cases or logged as unsupported.
Learnt from: CayoPOliveira
PR: fazer-ai/chatwoot#53
File: app/services/whatsapp/baileys_handlers/messages_update.rb:61-61
Timestamp: 2025-05-26T20:14:34.814Z
Learning: In Whatsapp::BaileysHandlers::MessagesUpdate#handle_edited_content, reassigning @raw_message to the nested edited message data is acceptable because this method executes at the end of the corresponding flow, with no subsequent operations depending on the original @raw_message structure.
Learnt from: CayoPOliveira
PR: fazer-ai/chatwoot#26
File: app/services/whatsapp/incoming_message_baileys_service.rb:198-207
Timestamp: 2025-04-16T11:55:12.231Z
Learning: In the WhatsApp Baileys API integration, only 'image' and 'video' message types support captions. The 'file', 'audio', and 'sticker' message types do not have caption fields.
Learnt from: CayoPOliveira
PR: fazer-ai/chatwoot#26
File: app/services/whatsapp/incoming_message_baileys_service.rb:198-207
Timestamp: 2025-04-16T11:55:12.231Z
Learning: In the WhatsApp Baileys API integration, only 'image' and 'video' message types support captions. The 'file', 'audio', and 'sticker' message types do not have caption fields.
app/services/whatsapp/send_on_whatsapp_service.rb (3)
Learnt from: CayoPOliveira
PR: fazer-ai/chatwoot#53
File: app/services/whatsapp/baileys_handlers/messages_update.rb:61-61
Timestamp: 2025-05-26T20:14:34.814Z
Learning: In Whatsapp::BaileysHandlers::MessagesUpdate#handle_edited_content, reassigning @raw_message to the nested edited message data is acceptable because this method executes at the end of the corresponding flow, with no subsequent operations depending on the original @raw_message structure.
Learnt from: CayoPOliveira
PR: fazer-ai/chatwoot#20
File: app/services/whatsapp/incoming_message_baileys_service.rb:0-0
Timestamp: 2025-04-08T21:26:26.063Z
Learning: Chatwoot message model (app/models/message.rb) only supports four status states: sent (0), delivered (1), read (2), and failed (3). When mapping from Baileys WhatsApp statuses, only ERROR (0), SERVER_ACK (2), DELIVERY_ACK (3), and READ (4) can be mapped to Chatwoot's statuses. Baileys statuses like PENDING (1) and PLAYED (5) don't have Chatwoot equivalents and should be handled as special cases or logged as unsupported.
Learnt from: CayoPOliveira
PR: fazer-ai/chatwoot#53
File: app/services/whatsapp/baileys_handlers/messages_upsert.rb:7-16
Timestamp: 2025-05-26T20:14:33.177Z
Learning: In the Whatsapp Baileys handlers for this codebase, errors should propagate to the WhatsappController level rather than being caught and handled individually within message processing methods. The user prefers centralized error handling at the controller level over individual message-level error handling.
spec/jobs/channels/whatsapp/retry_send_reply_job_spec.rb (2)
Learnt from: CayoPOliveira
PR: fazer-ai/chatwoot#20
File: app/services/whatsapp/incoming_message_baileys_service.rb:0-0
Timestamp: 2025-04-08T21:26:26.063Z
Learning: Chatwoot message model (app/models/message.rb) only supports four status states: sent (0), delivered (1), read (2), and failed (3). When mapping from Baileys WhatsApp statuses, only ERROR (0), SERVER_ACK (2), DELIVERY_ACK (3), and READ (4) can be mapped to Chatwoot's statuses. Baileys statuses like PENDING (1) and PLAYED (5) don't have Chatwoot equivalents and should be handled as special cases or logged as unsupported.
Learnt from: CayoPOliveira
PR: fazer-ai/chatwoot#53
File: app/services/whatsapp/baileys_handlers/messages_upsert.rb:7-16
Timestamp: 2025-05-26T20:14:33.177Z
Learning: In the Whatsapp Baileys handlers for this codebase, errors should propagate to the WhatsappController level rather than being caught and handled individually within message processing methods. The user prefers centralized error handling at the controller level over individual message-level error handling.
app/jobs/channels/whatsapp/retry_send_reply_job.rb (4)
Learnt from: CayoPOliveira
PR: fazer-ai/chatwoot#20
File: app/services/whatsapp/incoming_message_baileys_service.rb:0-0
Timestamp: 2025-04-08T21:26:26.063Z
Learning: Chatwoot message model (app/models/message.rb) only supports four status states: sent (0), delivered (1), read (2), and failed (3). When mapping from Baileys WhatsApp statuses, only ERROR (0), SERVER_ACK (2), DELIVERY_ACK (3), and READ (4) can be mapped to Chatwoot's statuses. Baileys statuses like PENDING (1) and PLAYED (5) don't have Chatwoot equivalents and should be handled as special cases or logged as unsupported.
Learnt from: CayoPOliveira
PR: fazer-ai/chatwoot#53
File: app/services/whatsapp/baileys_handlers/messages_upsert.rb:7-16
Timestamp: 2025-05-26T20:14:33.177Z
Learning: In the Whatsapp Baileys handlers for this codebase, errors should propagate to the WhatsappController level rather than being caught and handled individually within message processing methods. The user prefers centralized error handling at the controller level over individual message-level error handling.
Learnt from: CayoPOliveira
PR: fazer-ai/chatwoot#53
File: app/services/whatsapp/baileys_handlers/messages_update.rb:61-61
Timestamp: 2025-05-26T20:14:34.814Z
Learning: In Whatsapp::BaileysHandlers::MessagesUpdate#handle_edited_content, reassigning @raw_message to the nested edited message data is acceptable because this method executes at the end of the corresponding flow, with no subsequent operations depending on the original @raw_message structure.
Learnt from: CayoPOliveira
PR: fazer-ai/chatwoot#53
File: app/services/whatsapp/baileys_handlers/helpers.rb:59-70
Timestamp: 2025-05-26T20:14:33.151Z
Learning: In WhatsApp Baileys message handling, audio, file, sticker, protocol, and unsupported message types don't have extractable text content. Only text messages, image captions, video captions, and reaction text should be handled in message_content extraction methods.
🪛 ast-grep (0.38.6)
app/jobs/channels/whatsapp/retry_send_reply_job.rb
[warning] 22-26: Found the use of an hardcoded passphrase for RSA. The passphrase can be easily discovered, and therefore should not be stored in source-code. It is recommended to remove the passphrase from source-code, and use system environment variables or a restricted configuration file.
Context: Message.joins(:conversation)
.joins('JOIN inboxes ON conversations.inbox_id = inboxes.id')
.where(inboxes: { channel: channel })
.where(status: :sent)
.where('messages.created_at > ?', 24.hours.ago)
Note: [CWE-798]: Use of Hard-coded Credentials [OWASP A07:2021]: Identification and Authentication Failures [REFERENCES]
https://cwe.mitre.org/data/definitions/522.html
(hardcoded-secret-rsa-passphrase-ruby)
🔇 Additional comments (4)
app/services/whatsapp/send_on_whatsapp_service.rb (1)
120-121: Excellent idempotency guard clause.This prevents duplicate message sends by checking if a message already has a
source_idbefore attempting to send it. This is particularly important for the retry mechanism implementation where the same message might be processed multiple times.spec/jobs/channels/whatsapp/retry_send_reply_job_spec.rb (1)
1-63: Comprehensive test coverage for the retry job.The test suite effectively covers all key scenarios:
- Job enqueuing with correct arguments and queue assignment
- Provider filtering (only processing baileys channels)
- Message selection criteria (status, age, type)
- Error handling that properly updates message status
The test logic accurately reflects the expected behavior and includes both positive and negative test cases.
app/jobs/channels/whatsapp/retry_send_reply_job.rb (2)
32-37: Good error handling implementation.The error handling correctly catches exceptions and updates the message status to
:failed, which prevents infinite retry loops and provides proper error tracking.
22-30: Verify retry filter targets the correct message statusI didn’t find any other retry jobs under app/jobs/channels/whatsapp—please double-check that the retry logic in
find_failed_messagesis intentionally filtering onstatus: :sent(i.e. only “sent” messages from the last 24 hours). If the intent is to requeue failed deliveries, you may need to usestatus: :failed(or include both states) instead.• Location:
– app/jobs/channels/whatsapp/retry_send_reply_job.rb →find_failed_messages(lines 22–30)
CayoPOliveira
commented
Jul 15, 2025
Collaborator
Author
CayoPOliveira
left a comment
CayoPOliveira
left a comment
There was a problem hiding this comment.
@coderabbitai[bot] review
-work-in-progress +(review needed)
Reviewed 4 of 9 files at r2, 8 of 8 files at r6, all commit messages.
Reviewable status:complete! all files reviewed, all discussions resolved (waiting on @CayoPOliveira)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Tested successfully with message - retry mechanism working correctly with proper tracking attributes (baileys_retry_count, baileys_last_attempt_at) added to messages.
Fixes linting issues and follows Ruby style guidelines.
Pull Request Template
Description
Please include a summary of the change and issue(s) fixed. Also, mention relevant motivation, context, and any dependencies that this change requires.
Fixes # (issue)
Type of change
Please delete options that are not relevant.
How Has This Been Tested?
Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce. Please also list any relevant details for your test configuration.
Checklist:
This change is
Summary by CodeRabbit
New Features
Bug Fixes
Tests