fix(baileys): always emit MESSAGES_UPSERT for media even when S3 upload is skipped or fails - #2684
Open
pastoriniMatheus wants to merge 1 commit into
Conversation
…ad is skipped or fails
In the messages.upsert handler, the S3 upload block returned early from the whole
handler in two cases (video upload disabled; getBase64FromMediaMessage returns
null), aborting before sendDataWebhook(Events.MESSAGES_UPSERT). The message is
persisted but the webhook carrying its content is never emitted.
This silently drops media messages whose upload is skipped or fails — notably
fromMe media sent from another device, where getBase64FromMediaMessage cannot
fetch the file (its mediaKey belongs to that device). Measured at ~94% media loss
for fromMe messages on a production deployment (S3 enabled).
Restructure the block so it skips only the upload, never the handler, so the
webhook is always delivered regardless of the storage outcome. The inline comment
("returning early from this block") shows the original intent was to skip the
upload only; another method in the same file already uses throw for the
equivalent case.
Contributor
Reviewer's GuideEnsures the WhatsApp Baileys messages.upsert handler always emits MESSAGES_UPSERT for media messages, even when S3 upload is disabled, skipped, or fails, by restructuring the S3 upload block to avoid early returns that previously aborted the handler. Sequence diagram for updated Baileys messages.upsert media handlingsequenceDiagram
participant BaileysStartupService
participant S3Service as s3Service
participant PrismaMedia as prismaRepository_media
participant PrismaMessage as prismaRepository_message
participant Webhook as sendDataWebhook
BaileysStartupService->>BaileysStartupService: messages.upsert(received)
alt isMedia && S3.ENABLE
alt isVideo && !S3.SAVE_VIDEO
BaileysStartupService->>BaileysStartupService: logger.warn('Video upload is disabled. Skipping video upload.')
note over BaileysStartupService: Skip upload only, continue handler
else nonVideo or S3.SAVE_VIDEO
BaileysStartupService->>BaileysStartupService: hasValidMediaContent(message)
alt !hasRealMedia
BaileysStartupService->>BaileysStartupService: logger.warn('Message detected as media but contains no valid media content')
else hasRealMedia
BaileysStartupService->>BaileysStartupService: getBase64FromMediaMessage(message, true)
alt media is null
BaileysStartupService->>BaileysStartupService: logger.verbose('No valid media to upload (messageContextInfo only), skipping MinIO')
note over BaileysStartupService: No upload, continue handler
else media available
BaileysStartupService->>S3Service: uploadFile(fullName, buffer, size, headers)
BaileysStartupService->>PrismaMedia: media.create(data)
BaileysStartupService->>S3Service: getObjectUrl(fullName)
BaileysStartupService->>PrismaMessage: message.update({ id: msg.id }, messageRaw)
end
end
end
else !isMedia or !S3.ENABLE
BaileysStartupService->>BaileysStartupService: proceed without S3 upload
end
BaileysStartupService->>Webhook: sendDataWebhook(Events.MESSAGES_UPSERT, messageRaw)
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Hey - I've found 1 issue, and left some high level feedback:
- The S3 upload block is now quite deeply nested; consider extracting the media upload and persistence logic into a separate helper to simplify the messages.upsert handler’s control flow and improve readability.
- You are calling this.configService.get('S3') multiple times in close proximity; caching the S3 config locally within the handler would reduce repetition and make it clearer which configuration values are being used.
- The new explanatory comment above the try block is detailed but lengthy; trimming it down or moving the deeper context into a commit message would keep the code more focused while still documenting the behavior change.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The S3 upload block is now quite deeply nested; consider extracting the media upload and persistence logic into a separate helper to simplify the messages.upsert handler’s control flow and improve readability.
- You are calling this.configService.get<S3>('S3') multiple times in close proximity; caching the S3 config locally within the handler would reduce repetition and make it clearer which configuration values are being used.
- The new explanatory comment above the try block is detailed but lengthy; trimming it down or moving the deeper context into a commit message would keep the code more focused while still documenting the behavior change.
## Individual Comments
### Comment 1
<location path="src/api/integrations/channel/whatsapp/whatsapp.baileys.service.ts" line_range="1603-1604" />
<code_context>
+ if (!media) {
+ this.logger.verbose('No valid media to upload (messageContextInfo only), skipping MinIO');
+ } else {
+ const { buffer, mediaType, fileName, size } = media;
+ const mimetype = mimeTypes.lookup(fileName).toString();
+ const fullName = join(
+ `${this.instance.id}`,
</code_context>
<issue_to_address>
**issue (bug_risk):** Guard against mimeTypes.lookup returning a falsy value before calling toString.
`mimeTypes.lookup(fileName)` can return `false`/`null` for unknown types, so `.toString()` may throw and break the upload flow. Please handle the falsy case (e.g. with a default like `'application/octet-stream'` or an explicit check) before converting to string.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Comment on lines
+1603
to
+1604
| const { buffer, mediaType, fileName, size } = media; | ||
| const mimetype = mimeTypes.lookup(fileName).toString(); |
Contributor
There was a problem hiding this comment.
issue (bug_risk): Guard against mimeTypes.lookup returning a falsy value before calling toString.
mimeTypes.lookup(fileName) can return false/null for unknown types, so .toString() may throw and break the upload flow. Please handle the falsy case (e.g. with a default like 'application/octet-stream' or an explicit check) before converting to string.
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
Problem
When a media message (audio, image, video, document) is sent from the phone linked to the instance (
fromMe,source = 'android'/'ios'), theMESSAGES_UPSERTwebhook is never emitted whenS3_ENABLED=true. The message is persisted correctly andMESSAGES_UPDATE(status) is emitted normally, but the event carrying the actual content never reaches the webhook consumer. Text messages from the same phone work; media sent through the API works. The failure is specific to media originated on another device.For any webhook consumer (CRM, chatbot, archiving), this is silent, permanent message loss — no error, no warning, no retry. The consumer only sees a status update referencing a message it never received.
Root cause
src/api/integrations/channel/whatsapp/whatsapp.baileys.service.ts, handlermessages.upsert: the S3 upload block has tworeturnstatements that exit the entire handler, andsendDataWebhook(Events.MESSAGES_UPSERT, ...)sits after them:getBase64FromMediaMessage()commonly fails for media sent from another device (the media key belongs to that device) → path (2) fires → the message is dropped from the webhook stream. The inline comment ("Skip video upload by returning early from this block") shows the intent was to skip only the upload. Notably, another method in the same file already handles the equivalent case withthrow, confirming thereturns are an oversight rather than intended behavior.Fix
Restructure the block so
sendDataWebhookis always reached — skip only the upload, never the handler (without usingthrowfor control flow). When the upload succeeds the behavior is unchanged; when it is skipped or fails, the message is still delivered via the webhook. Webhook delivery must not depend on the outcome of the storage step.Impact (measured — 48h, production, 4 instances)
fromMefromMeRoughly 136 media messages lost per day on a single deployment. Because the failure is silent, it can go unnoticed for a long time.
Verification
fromMemedia from the phone went from 0/21 to 4/4 (android) and 2/2 (web) delivered, files intact in storage; themessages.upsert fromMe=true audioMessage/imageMessageevents that never fired started firing.npm ci·eslint src·npm run build=tsc --noEmit && tsup) — passes.Suggested regression test
The repo has no test suite today (
npm testpoints at a non-existent./test/all.test.ts, and the quality CI runs lint + build only), so no test is added here. When a test harness exists, this is the case to lock in:Steps to reproduce
S3_ENABLED=trueand a webhook subscribing toMESSAGES_UPSERT.Message,source = 'android'), onlyMESSAGES_UPDATEreaches the webhook, and noMESSAGES_UPSERTis emitted for it. A text message from the same phone emitsMESSAGES_UPSERTnormally.Summary by Sourcery
Ensure WhatsApp Baileys media messages always trigger MESSAGES_UPSERT webhooks regardless of S3 upload success or skip conditions.
Bug Fixes:
Enhancements: