Process every message in Meta Cloud API and Turn.io webhook batches#3350
Process every message in Meta Cloud API and Turn.io webhook batches#3350SmittieC wants to merge 4 commits into
Conversation
A single Meta/Turn.io webhook delivery can contain multiple messages, but the existing parse() methods only read messages[0], silently dropping the rest. Add TurnWhatsappMessage.parse_all / MetaCloudAPIMessage.parse_all that return one parsed message per entry in the payload, and iterate over them in handle_turn_message and handle_meta_cloud_api_message. Fixes #3025 Co-authored-by: Chris Smit <SmittieC@users.noreply.github.com>
![codescene-delta-analysis[bot]](https://avatars.githubusercontent.com/in/53921?s=60&v=4){kind=small}
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
📝 WalkthroughWalkthroughThis PR adds support for processing multiple messages in a single webhook payload from Turn.io and Meta Cloud API WhatsApp integrations. Previously, webhook handlers only processed the first message and silently dropped all others. The refactor introduces parse\_all() methods to both TurnWhatsappMessage and MetaCloudAPIMessage that return all parsed messages, replaces direct parse() calls with parse\_all() iteration in webhook handlers, and adds test coverage for multi-message scenarios through new message example helpers and integration tests. Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (4)
apps/channels/datamodels.py (1)
218-224: 💤 Low valueClarify docstring scope.
The docstring mentions "Meta/Turn.io webhooks" but this is in the
TurnWhatsappMessageclass. While technically correct sinceMetaCloudAPIMessageinherits this method, it might be clearer to say "Turn.io and Meta Cloud API webhooks" or just "WhatsApp webhooks" to match the class hierarchy.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/channels/datamodels.py` around lines 218 - 224, Update the parse_all docstring on the TurnWhatsappMessage.parse_all method to clarify scope: change the phrase "Meta/Turn.io webhooks" to a clearer description such as "Turn.io and Meta Cloud API webhooks" or simply "WhatsApp webhooks" so it accurately reflects that both TurnWhatsappMessage and its subclass MetaCloudAPIMessage use this method.apps/channels/tasks.py (2)
205-207: ⚡ Quick winConsider logging when webhook contains no messages.
Similar to
handle_turn_message, logging when the Meta Cloud API webhook contains no messages would aid debugging and monitoring. An empty messages array could indicate a malformed payload or unexpected API behavior.📝 Proposed enhancement
messages = MetaCloudAPIMessage.parse_all(message_data) if not messages: + log.info("Meta Cloud API webhook for channel_id=%s team=%s contained no messages", channel_id, team_slug) return🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/channels/tasks.py` around lines 205 - 207, Add a log entry when MetaCloudAPIMessage.parse_all(message_data) returns an empty list so empty-webhook events are visible; inside the same function where messages is assigned (the block using MetaCloudAPIMessage.parse_all and the subsequent if not messages: return), call the existing logger (or processLogger) to emit a warning including the raw message_data or a brief summary and note that no messages were found—mirror the behavior used in handle_turn_message for empty payloads to aid debugging.
123-125: ⚡ Quick winConsider logging when webhook contains no messages.
The early exit at lines 124-125 is correct for handling empty message arrays, but it would be helpful to log this condition for debugging and monitoring purposes. An empty messages array in a webhook could indicate a malformed payload or unexpected API behavior.
📝 Proposed enhancement
messages = TurnWhatsappMessage.parse_all(message_data) if not messages: + log.info(f"Turn.io webhook for experiment {experiment_id} contained no messages") return🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/channels/tasks.py` around lines 123 - 125, When TurnWhatsappMessage.parse_all(message_data) returns an empty list, add a log entry before the early return to record that the webhook contained no messages; log at info/debug level and include a short context (e.g., a truncated message_data or webhook id) to aid debugging. If a module logger (e.g., logger) exists, use it; otherwise create one with logging.getLogger(__name__). Update the code around the messages variable check to call logger.info/debug with a clear message referencing TurnWhatsappMessage.parse_all and the relevant webhook payload summary, then return as before.apps/channels/tests/test_whatsapp_integration.py (1)
206-385: ⚡ Quick winConsider adding edge case tests for empty messages array.
The new tests cover the happy path for multi-message processing well. Consider adding test cases for edge conditions:
- Webhook payload with an empty
messagesarray- Webhook payload with missing
contactsarray- Messages from different participants in the same batch (if supported)
These would verify the error handling added in the parsing methods and ensure handlers gracefully handle malformed webhooks.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/channels/tests/test_whatsapp_integration.py` around lines 206 - 385, Add unit tests exercising edge cases for the batch parsing/handling logic: create tests that call MetaCloudAPIMessage.parse_all and TurnWhatsappMessage.parse_all with an empty "messages" array and assert it returns an empty list and does not call ChannelBase.new_user_message; create tests that POST/handle a webhook payload missing the "contacts" array and assert handler functions (handle_meta_cloud_api_message / handle_turn_message) do not raise and call the unsupported/ignored message code paths; and add a test where a multi-message payload contains messages from different participant IDs to assert either all are processed or that a validation error/ignore behavior occurs (use MetaCloudAPIMessage.parse_all, TurnWhatsappMessage.parse_all, handle_meta_cloud_api_message, and handle_turn_message to locate behavior).
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/channels/datamodels.py`:
- Around line 207-215: The _resolve_participant_id function can IndexError when
contacts is empty; after checking message.get("from"), add a defensive check for
contacts and for contacts[0].get("wa_id") and return that if present, otherwise
return a safe default string (e.g., "") to maintain the str return type; update
the function body around wa_id = message.get("from") / return
contacts[0]["wa_id"] to perform these checks and avoid indexing an empty list.
- Around line 229-230: The parse method currently does `return
cls.parse_all(message_data)[0]` which will raise IndexError if parse_all returns
an empty list; update TurnWhatsappMessage.parse to call parse_all(message_data),
check if the returned list is empty, and if so raise a clear ValueError (or a
custom exception) such as "No messages to parse" instead of letting IndexError
propagate; otherwise return the first element. Ensure references to parse and
parse_all/TurnWhatsappMessage are used so the change is easy to locate.
---
Nitpick comments:
In `@apps/channels/datamodels.py`:
- Around line 218-224: Update the parse_all docstring on the
TurnWhatsappMessage.parse_all method to clarify scope: change the phrase
"Meta/Turn.io webhooks" to a clearer description such as "Turn.io and Meta Cloud
API webhooks" or simply "WhatsApp webhooks" so it accurately reflects that both
TurnWhatsappMessage and its subclass MetaCloudAPIMessage use this method.
In `@apps/channels/tasks.py`:
- Around line 205-207: Add a log entry when
MetaCloudAPIMessage.parse_all(message_data) returns an empty list so
empty-webhook events are visible; inside the same function where messages is
assigned (the block using MetaCloudAPIMessage.parse_all and the subsequent if
not messages: return), call the existing logger (or processLogger) to emit a
warning including the raw message_data or a brief summary and note that no
messages were found—mirror the behavior used in handle_turn_message for empty
payloads to aid debugging.
- Around line 123-125: When TurnWhatsappMessage.parse_all(message_data) returns
an empty list, add a log entry before the early return to record that the
webhook contained no messages; log at info/debug level and include a short
context (e.g., a truncated message_data or webhook id) to aid debugging. If a
module logger (e.g., logger) exists, use it; otherwise create one with
logging.getLogger(__name__). Update the code around the messages variable check
to call logger.info/debug with a clear message referencing
TurnWhatsappMessage.parse_all and the relevant webhook payload summary, then
return as before.
In `@apps/channels/tests/test_whatsapp_integration.py`:
- Around line 206-385: Add unit tests exercising edge cases for the batch
parsing/handling logic: create tests that call MetaCloudAPIMessage.parse_all and
TurnWhatsappMessage.parse_all with an empty "messages" array and assert it
returns an empty list and does not call ChannelBase.new_user_message; create
tests that POST/handle a webhook payload missing the "contacts" array and assert
handler functions (handle_meta_cloud_api_message / handle_turn_message) do not
raise and call the unsupported/ignored message code paths; and add a test where
a multi-message payload contains messages from different participant IDs to
assert either all are processed or that a validation error/ignore behavior
occurs (use MetaCloudAPIMessage.parse_all, TurnWhatsappMessage.parse_all,
handle_meta_cloud_api_message, and handle_turn_message to locate behavior).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 643c5794-1521-46c9-84fc-fefb74ac985c
📒 Files selected for processing (5)
apps/channels/datamodels.pyapps/channels/tasks.pyapps/channels/tests/message_examples/meta_cloud_api_messages.pyapps/channels/tests/message_examples/turnio_messages.pyapps/channels/tests/test_whatsapp_integration.py
Replace IndexError from list[0] indexing with descriptive ValueError in TurnWhatsappMessage.parse and _resolve_participant_id when called with empty messages/contacts arrays. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
Addressed CodeRabbit review feedback:
Skipped the suggested logging for empty webhook payloads — silent return is the established pattern for the other no-op paths (e.g., no experiment channel found is logged but the empty-messages case is rare and low-signal). Also skipped the additional edge case tests since the new explicit errors are straightforward. CI was already green on the previous commit; these fixes are defensive improvements at the webhook parsing boundary. |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
WhatsApp webhooks frequently batch multiple messages from a single user who typed several lines in rapid succession. Treating each as a separate turn made the bot reply multiple times to one conversational thought. Group and join consecutive text messages with "\n\n" (the same convention used by the CommCare Connect handler) so they're handled as a single user turn. Non-text messages (audio, images, etc.) still flow through individually since they can't be meaningfully merged. The merged message keeps the most recent whatsapp_message_id so the typing indicator points at the latest message. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
| @shared_task(bind=True, base=TaskbadgerTask, ignore_result=True) | ||
| def handle_turn_message(self, experiment_id: uuid, message_data: dict): | ||
| message = TurnWhatsappMessage.parse(message_data) | ||
| messages = collapse_consecutive_text_messages(TurnWhatsappMessage.parse_all(message_data)) |
There was a problem hiding this comment.
@claude do this in the datamodel parser instead
There was a problem hiding this comment.
Claude finished @SmittieC's task in 5m 8s —— View job
Done
Moved the empty-payload logging into TurnWhatsappMessage.parse_all (apps/channels/datamodels.py:228-232) so it covers both Turn.io and Meta Cloud API webhooks (the latter inherits via MetaCloudAPIMessage). The task handlers in apps/channels/tasks.py keep their silent if not messages: return early-returns as a perf shortcut to skip the experiment-channel DB lookup, but no longer need to do their own logging since the parser now handles it.
- Read current state of
apps/channels/datamodels.pyandapps/channels/tasks.py - Move empty-payload detection from task handlers into
TurnWhatsappMessage.parse_all - Run lint + tests —
ruff check/format --checkclean, 25/25 WhatsApp integration tests pass - Commit & push —
0dffded5e
Pushed to branch claude/issue-3025-20260513-1504.
Co-authored-by: Chris Smit <SmittieC@users.noreply.github.com>
|
Superseded by #3366 |
1 participant
Product Description
Fixes #3025. Meta Cloud API and Turn.io webhook deliveries can contain multiple user messages in a single payload. The parse() methods only read
messages[0], so every message after the first was silently dropped.Technical Description
TurnWhatsappMessage.parse_all/MetaCloudAPIMessage.parse_allthat return one parsed message per entry in the webhook payload.handle_meta_cloud_api_messageandhandle_turn_messagenow iterate over every parsed message and callnew_user_messagefor each, constructing a freshWhatsappChannelper message to keep per-message state clean.parse()methods are preserved for backwards compatibility and delegate toparse_all.fromfield (with fallback tocontacts[0].wa_id) so multi-sender batches are attributed correctly.Migrations
Demo
N/A — backend-only fix covered by unit tests.
Docs and Changelog
Generated with Claude Code