| FazBrowse GitHub Viewer | Trending | | Home |
| Tools: [Download Repo ZIP] [Original HTTPS Page] |
Sorry, something went wrong.
The BusinessStartupService shared a mutable this.phoneNumber property across concurrent webhook requests. When two webhooks arrived near- simultaneously for the same phone_number_id, the second request could overwrite phoneNumber before the first finished processing, causing messages to be attributed to the wrong sender (wrong remoteJid). Changes: - Compute senderJid as a local variable before calling eventHandler - Pass senderJid as parameter through eventHandler -> messageHandle - Await eventHandler to prevent concurrent mutation - Replace all this.phoneNumber reads inside messageHandle with the local senderJid parameter Made-with: Cursor
Reviewer's guide (collapsed on small PRs)
Reviewer's GuideRefactors the Meta WhatsApp Business webhook flow to compute a per-request sender JID and thread it through the event handling pipeline, removing reliance on a mutable instance field and awaiting the handler to avoid race conditions between concurrent webhook requests. Sequence diagram for updated Meta WhatsApp webhook sender identificationsequenceDiagram
participant MetaWebhook as MetaWebhook
participant BusinessStartupService as BusinessStartupService
participant EventHandler as eventHandler
participant MessageHandle as messageHandle
MetaWebhook->>BusinessStartupService: inboundWebhook(content)
BusinessStartupService->>BusinessStartupService: loadChatwoot()
BusinessStartupService->>BusinessStartupService: senderJid = createJid(from or recipient_id)
BusinessStartupService->>BusinessStartupService: phoneNumber = senderJid
BusinessStartupService->>EventHandler: eventHandler(content, senderJid)
activate EventHandler
EventHandler->>EventHandler: database = get DATABASE
EventHandler->>EventHandler: settings = findSettings()
alt content has messages
EventHandler->>MessageHandle: messageHandle(content, database, settings, senderJid)
else content has statuses
EventHandler->>MessageHandle: messageHandle(content, database, settings, senderJid)
end
deactivate EventHandler
MessageHandle->>MessageHandle: key.remoteJid = senderJid
MessageHandle->>MessageHandle: key.fromMe = (senderJid === metadata.phone_number_id)
Updated class diagram for BusinessStartupService event handlingclassDiagram
class ChannelStartupService
class Database
class BusinessStartupService {
- phoneNumber string
+ eventHandler(content any, senderJid string) void
+ messageHandle(received any, database Database, settings any, senderJid string) void
}
BusinessStartupService --|> ChannelStartupService
BusinessStartupService ..> Database
File-Level Changes
Tips and commands Interacting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
Sorry, something went wrong.
There was a problem hiding this comment.
Hey - I've left some high level feedback:
Please address the comments from this code review:
## Overall Comments
- If the goal is to eliminate shared mutable state causing race conditions, consider removing `this.phoneNumber` entirely and consistently passing `senderJid` through all call sites and usages rather than still assigning to the instance property.
- Now that `eventHandler` depends on a `senderJid` argument, it might be safer to encapsulate the JID derivation inside `eventHandler` itself (or validate the argument) so that future callers don’t accidentally pass an inconsistent value and reintroduce attribution issues.
Sorry, something went wrong.
|
same problem here, merge this! |
Sorry, something went wrong.
|
This bug fix for the race condition in Business API sender identification has been waiting 18 days without a reviewer assigned. Race conditions can cause hard-to-reproduce issues in production — could a maintainer assign a reviewer or take a look when bandwidth allows? |
Sorry, something went wrong.
|
Olá, equipe! Este PR corrige uma race condition no Business API sender ID da integração com Meta — um problema de alta criticidade que pode causar comportamentos imprevisíveis em ambientes com alta concorrência. Após 18 dias sem revisão, solicitamos atenção prioritária:
Escalando para revisão prioritária. Obrigado! |
Sorry, something went wrong.
There was a problem hiding this comment.
LGTM — fixes race by passing senderJid as explicit parameter instead of mutating shared state.
Sorry, something went wrong.
…on (evolution-foundation#2493) The BusinessStartupService shared a mutable this.phoneNumber property across concurrent webhook requests. When two webhooks arrived near- simultaneously for the same phone_number_id, the second request could overwrite phoneNumber before the first finished processing, causing messages to be attributed to the wrong sender (wrong remoteJid). Changes: - Compute senderJid as a local variable before calling eventHandler - Pass senderJid as parameter through eventHandler -> messageHandle - Await eventHandler to prevent concurrent mutation - Replace all this.phoneNumber reads inside messageHandle with the local senderJid parameter Made-with: Cursor (cherry picked from commit b9c7f26)
| Back | FazBrowse Home | New Git URL |
The BusinessStartupService shared a mutable this.phoneNumber property across concurrent webhook requests. When two webhooks arrived near- simultaneously for the same phone_number_id, the second request could overwrite phoneNumber before the first finished processing, causing messages to be attributed to the wrong sender (wrong remoteJid).
Changes:
Made-with: Cursor
📋 Description
🔗 Related Issue
Closes #(issue_number)
🧪 Type of Change
🧪 Testing
📸 Screenshots (if applicable)
✅ Checklist
📝 Additional Notes
Summary by Sourcery
Bug Fixes: