fix(instance): do not open a second connection on repeated clicks - #35
fix(instance): do not open a second connection on repeated clicks#35michumichifu wants to merge 1 commit into
Conversation
Every click on the QR code / pairing code button called GET /instance/connect with no guard. When the instance is closed that endpoint does not return a cached code: it calls connectToWhatsapp() and opens a brand new WhatsApp connection. Clicking twice therefore left two sockets racing for the same instance, each producing its own QR code, and the phone could end up scanning the one that the other socket had already invalidated. Keep a single request in flight: a ref guards against overlapping calls and is checked synchronously, since two fast clicks would both pass a state-based check before React re-renders. The buttons are disabled while the request is running, so the user gets feedback instead of clicking again.
Reviewer's GuideAdds a guard and UI feedback to prevent multiple concurrent WhatsApp connection requests when the QR or pairing code buttons are clicked repeatedly. Sequence diagram for guarded WhatsApp connection requestssequenceDiagram
actor User
participant DashboardInstance
participant Server
User->>DashboardInstance: click QRCode Button
DashboardInstance->>DashboardInstance: handleConnect(instance.name, false)
DashboardInstance->>DashboardInstance: [connectInFlight.current == false]
DashboardInstance->>DashboardInstance: connectInFlight.current = true
DashboardInstance->>DashboardInstance: setIsConnecting(true)
DashboardInstance->>DashboardInstance: setQRCode(null)
DashboardInstance->>Server: GET /instance/connect
Server-->>DashboardInstance: qrCode or pairingCode
DashboardInstance->>DashboardInstance: setQRCode / setPairingCode
DashboardInstance->>DashboardInstance: connectInFlight.current = false
DashboardInstance->>DashboardInstance: setIsConnecting(false)
User->>DashboardInstance: second click while request in flight
DashboardInstance->>DashboardInstance: handleConnect(instance.name, false)
DashboardInstance->>DashboardInstance: [connectInFlight.current == true]
DashboardInstance-->>User: return (no-op, buttons disabled by isConnecting)
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Hey - I've left some high level feedback:
- Because
handleConnectis async and updates state in afinallyblock, consider guardingsetIsConnectingwith anisMountedref or similar to avoid React warnings if the component unmounts while a request is in flight. - The
isConnectingflag is global to the component; if in the future multiple instances can appear on this page, you may want to scope the in-flight guard and disabled state to the specific instance instead of blocking all connect buttons.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Because `handleConnect` is async and updates state in a `finally` block, consider guarding `setIsConnecting` with an `isMounted` ref or similar to avoid React warnings if the component unmounts while a request is in flight.
- The `isConnecting` flag is global to the component; if in the future multiple instances can appear on this page, you may want to scope the in-flight guard and disabled state to the specific instance instead of blocking all connect buttons.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
|
Update: this is now running in production on our own deployment, built on top of Some field context on why the window is easier to hit than it looks. The o == "close" ? (await t.connectToWhatsapp(e), await (0, Vi.delay)(2e3), t.qrCode)So the request takes at least two seconds, and until this change the button stayed enabled for all of them. That is exactly the moment when a user wonders whether the click registered and clicks again, and the second click opens a second connection for the same instance. Each connection produces its own QR code, so the one on screen may be superseded by a socket the user cannot see. Reported symptom from the operator side, which is what sent us looking: "every time I press generate it starts a new one, so now there are two generating". Worth flagging for reviewers: this is deliberately a frontend guard for a backend behaviour. |
Problem
The QR code and pairing code buttons call
handleConnectwith no guard againstrepeated clicks:
That matters because
GET /instance/connectis not a read-only endpoint. Whenthe instance is closed it opens a new WhatsApp connection:
There is a 2-second delay inside that branch before anything is returned, and
the button stays enabled the whole time. Clicking again during that window
starts a second connection for the same instance, so two sockets end up racing:
each produces its own QR code, and the one the user is looking at may be
invalidated by the other. This is easy to trigger by accident, because when the
first attempt seems slow the natural reaction is to click again.
Fix
Allow a single request in flight at a time, and disable the buttons while it
runs.
The guard is a
useRefrather than state on purpose: two fast clicks would bothread the same stale value from state before React re-renders, and both would
pass the check. A ref is updated synchronously, so the second click is rejected.
isConnectingstate is still used for thedisabledprop, which gives the uservisible feedback instead of leaving them wondering whether the click registered.
Notes
same handler.
finallyblock, so a failed request does not leave thebuttons permanently disabled.
npm run type-checkandeslintpass.prettier --checkreports this file asunformatted both before and after the change, so it was left untouched rather
than mixing a whole-file reformat into the diff.
Summary by Sourcery
Guard instance connection requests so only one connect operation can run at a time and reflect this state in the UI.
Bug Fixes:
Enhancements: