Skip to content

fix: prevent duplicate ready events on SPA re-injection#201653

Open
Adi1231234 wants to merge 8 commits intowwebjs:mainfrom
Adi1231234:fix/upstream-spa-reinject-v2
Open

fix: prevent duplicate ready events on SPA re-injection#201653
Adi1231234 wants to merge 8 commits intowwebjs:mainfrom
Adi1231234:fix/upstream-spa-reinject-v2

Conversation

@Adi1231234
Copy link
Contributor

@Adi1231234 Adi1231234 commented Mar 12, 2026

Consolidates #127083 and #127090 into one PR with a few more fixes on top.

Root cause

framenavigated fires for iframes too (URLs like : and chrome-error://chromewebdata/), not just the main frame. Since inject() runs on every framenavigated without filtering, you get concurrent inject() calls racing each other, Backbone listeners stacking up (3-4 sets after a single QR scan), and eventually duplicate ready events.

I hit this in production - after a QR scan + destroy + re-init cycle, I was getting 3x READY events. In another case READY took ~10s (normally ~2s) because 3 concurrent LoadUtils calls were fighting over resources.

What changed

Only src/Client.js is modified. The diff looks big because the try/finally guard re-indents the inject() body - use "Hide whitespace" for a cleaner view.

The main fix:

  • Filter framenavigated to main frame only (frame.parentFrame() !== null -> skip). This is what actually solves the problem.

Preventing re-injection when unnecessary:

  • Check if WWebJS is already loaded before re-injecting on navigation. Skips inject() on SPA-internal navigations where the store is still fine.
  • _injectInProgress boolean guard so inject() can't run concurrently (safety net).

Listener cleanup:

  • Store Backbone listener refs in window._wwjsListeners, .off() them before registering new ones on re-inject. This is what prevents the accumulation that causes duplicate READY.

QR fixes (from #127090):

  • Null guard on change:ref - skip onQRChangedEvent when ref is null/undefined (was generating malformed QR strings starting with "undefined,")
  • Reset qrRetries when loading screen appears, so if linking fails and WA falls back to QR mode you get fresh retry attempts
  • Remove change:ref listener once change:hasSynced fires (no more stale QR events after auth)

Other:

  • try/catch on inject() from framenavigated (prevents unhandled rejection on detached frames during destroy)
  • After registering change:hasSynced listener, check if already true and trigger manually (race condition where state transitions before listener is registered)
  • Replace while + sleep(200) polling with waitForFunction for Debug.VERSION / socket state / WWebJS readiness

How I tested

Ran the same 5 scenarios on both upstream and this branch with diagnostic logging:

  • initialize() with existing session
  • QR scan flow
  • authenticated then destroy()
  • QR scan after destroy()
  • remote logout from phone then QR scan

Upstream had multiple inject() calls per session, listener accumulation, and duplicate READY in the destroy/re-init case. With this branch every scenario produces exactly 1 inject, 1 listener set, 1 READY. The ~10s READY issue is also gone.

All changes use upstream APIs only (WAWebSocketModel.Socket, WAWebCmd.Cmd).

Adi1231234 and others added 4 commits March 11, 2026 16:43
1. Add _injectInProgress concurrency guard
2. Replace polling loops with waitForFunction
3. Deduplicate Backbone listeners via tuple array with cleanup
4. Atomic hasSynced check after listener registration
5. isMainFrame guard and storeAvailable SPA skip in framenavigated

co-Authored-By: Claude Opus 4.6 <[email protected]>
- try/finally around inject() to always reset _injectInProgress
- Atomic hasSynced check inside listener registration evaluate()
- Fix framenavigated: capture isLogout before async, skip re-inject
  only when not logout AND store available
- try/catch around obj.off() in listener cleanup
- Null guard in QR ref change handler
- Reset qrRetries on LOADING_SCREEN event
- Add exposeFunctionIfAbsent in requestPairingCode()

co-Authored-By: Claude Opus 4.6 <[email protected]>
extract named onRefChange handler so it can be removed via
socket.on('change:hasSynced') once authentication succeeds,
matching PR #41's cleanup behavior.

co-Authored-By: Claude Opus 4.6 <[email protected]>
@Adi1231234
Copy link
Contributor Author

Adi1231234 commented Mar 12, 2026

Hey @BenyFilho, @williamcostaramos, @tuyuribr!

I consolidated my two previous PRs (#127083 and #127090) into this single, more comprehensive fix. After your great feedback on those PRs, I went back and did a deep dive into the root cause - turns out the core issue was framenavigated firing for iframes, not just the main frame.

On top of the original fixes, I added several more improvements (listener cleanup, concurrency guard, atomic hasSynced check, qrRetries reset) and ran thorough A/B diagnostic testing across 5 different real-world scenarios to make sure everything works correctly. The full results are in the PR description.

Note: the try/finally concurrency guard re-indents the inject() body, so the diff looks bigger than it is - "Hide whitespace" gives a cleaner view.

Would love it if you could take a look when you get a chance. Any feedback is very welcome!

Thanks :)

@Adi1231234 Adi1231234 changed the title fix: prevent duplicate ready events and listener accumulation on SPA re-injection fix: prevent duplicate ready events on SPA re-injection Mar 12, 2026
@BenyFilho BenyFilho added the waiting for testers Needs testing label Mar 14, 2026
@github-actions github-actions bot added the api changes API modifications label Mar 14, 2026
@BenyFilho BenyFilho added the AI pr AI-assisted PR; Review very carefully! label Mar 14, 2026
@Adi1231234
Copy link
Contributor Author

Hey @BenyFilho , thanks for the feedback on the previous PRs - I really appreciate you taking the time.

Quick note - yeah, I used AI to help write the PR description and for some of the investigation/research. Honestly who doesn't at this point. But the actual code changes are mine, and I personally ran all the testing scenarios on real WhatsApp sessions to validate everything works.

Either way, this is a real problem that I've been dealing with in production for a while now, and I put a lot of work into tracking down the root cause and fixing it properly. I also cleaned up the description to be more concise.

Would love to get your thoughts on the code whenever you have time. Happy to rework anything if needed!
Thanks!

the onAuthAppStateChangedEvent callback is registered with
exposefunctionifabsent, so it runs in Node.js context, not the browser.
calling window.require() there throws ReferenceError: window is not defined,
which means the QR code was never refreshed on UNPAIRED_IDLE state.
@Adi1231234
Copy link
Contributor Author

Adi1231234 commented Mar 16, 2026

This PR also fixes #201663.

The onAuthAppStateChangedEvent callback is registered via exposeFunction, so it runs in Node.js, not the browser. Calling window.require() there throws ReferenceError: window is not defined, which means the QR was never refreshed on UNPAIRED_IDLE. Wrapped the call in pupPage.evaluate() to fix it.

@BenyFilho
Copy link
Member

Please review your try/catch blocks and check if they are really necessary. If they are needed, the catch should throw the error instead of failing silently.

@BenyFilho BenyFilho added the bad pr Below project standards label Mar 18, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

AI pr AI-assisted PR; Review very carefully! api changes API modifications bad pr Below project standards waiting for testers Needs testing

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants