Skip to content

fix: make inject() resilient to page navigation during initialization#127083

Closed
Adi1231234 wants to merge 2 commits intowwebjs:mainfrom
Adi1231234:fix/inject-navigation-resilience-upstream
Closed

fix: make inject() resilient to page navigation during initialization#127083
Adi1231234 wants to merge 2 commits intowwebjs:mainfrom
Adi1231234:fix/inject-navigation-resilience-upstream

Conversation

@Adi1231234
Copy link
Copy Markdown
Contributor

@Adi1231234 Adi1231234 commented Feb 28, 2026

Summary

Fixes #127082

inject() uses page.evaluate() in polling loops to wait for window.Debug?.VERSION and window.Store. After sleep/resume, Chrome may perform an internal page navigation (IndexedDB recovery) that destroys the execution context, causing page.evaluate() to throw "Execution context was destroyed".

This PR:

  • Replaces the two manual page.evaluate() polling loops with page.waitForFunction(), which natively survives execution context destruction
  • Moves the framenavigated listener registration to before inject() in initialize(), so the recovery mechanism is in place from the start

No retry logic, no try/catch wrapping, no error swallowing. Uses the correct Puppeteer API that handles execution context lifecycle internally.

Changes

src/Client.js

  1. First polling loop (auth timeout): Replace manual while loop with page.evaluate() -> page.waitForFunction('window.Debug?.VERSION != undefined', { timeout })

  2. Second polling loop (ready timeout): Replace manual while loop with page.evaluate() -> page.waitForFunction('window.Store != undefined', { timeout: 30000 })

  3. initialize() ordering: Move this.pupPage.on('framenavigated', ...) registration to before await this.inject() so that if inject fails, the framenavigated handler is already in place for recovery

Why waitForFunction works

Puppeteer's WaitTask treats "Execution context was destroyed" as a non-fatal error (getBadError() returns undefined). When a new context is created after navigation, IsolatedWorld calls taskManager.rerunAll() to re-evaluate all waiting tasks in the new context. This is the intended Puppeteer mechanism for handling navigation during waits.

Verified in both Puppeteer 18.x (used by wwjs) and 24.x.

Test plan

  • E2E tests proving page.evaluate() throws during navigation (the bug)
  • E2E tests proving page.waitForFunction() survives navigation (the fix)
  • A/B comparison tests (old code vs new code) with HTTP server
  • Full flow tests combining framenavigated + waitForFunction
  • Sanity tests confirming no regression when there is no navigation
  • All 11 tests passing

@Adi1231234
Copy link
Copy Markdown
Contributor Author

Hey @purpshell @BenyFilho, friendly bump on this one 🙂

This fixes inject() crashing after the machine goes to sleep and wakes up. Happens because the page navigates during sleep and the execution context gets destroyed.

Let me know if you'd like me to change anything, happy to discuss on Discord too!

@BenyFilho
Copy link
Copy Markdown
Member

Hey @purpshell @BenyFilho, friendly bump on this one 🙂

This fixes inject() crashing after the machine goes to sleep and wakes up. Happens because the page navigates during sleep and the execution context gets destroyed.

Let me know if you'd like me to change anything, happy to discuss on Discord too!

Need user tests to validate it, and update your changes with branch main

@Adi1231234 Adi1231234 force-pushed the fix/inject-navigation-resilience-upstream branch from 6cc0468 to c4b1202 Compare March 9, 2026 23:04
@Adi1231234
Copy link
Copy Markdown
Contributor Author

Hey @BenyFilho, just rebased onto the latest main, resolved the conflicts and CI is passing.

These PRs keep getting conflicts because other things get merged while mine are waiting for review. I'd really appreciate getting them merged so I don't have to keep rebasing 😅

You mentioned user tests, could you explain what you mean exactly? Happy to add whatever helps move this forward.

@BenyFilho
Copy link
Copy Markdown
Member

Hey @BenyFilho, just rebased onto the latest main, resolved the conflicts and CI is passing.

These PRs keep getting conflicts because other things get merged while mine are waiting for review. I'd really appreciate getting them merged so I don't have to keep rebasing 😅

You mentioned user tests, could you explain what you mean exactly? Happy to add whatever helps move this forward.

Need user tests to validate it, and update your changes with branch main

Replace manual evaluate-based polling loops with waitForFunction,
which natively survives execution context destruction caused by
page navigation (e.g. Chrome's internal IndexedDB recovery after
system sleep/resume).

Also move the framenavigated listener registration before the
initial inject() call, so navigation events during inject are
handled by the existing listener.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@Adi1231234 Adi1231234 force-pushed the fix/inject-navigation-resilience-upstream branch from c4b1202 to 929c81c Compare March 10, 2026 01:40
@github-actions github-actions bot added the api changes API modifications label Mar 10, 2026
@Adi1231234 Adi1231234 force-pushed the fix/inject-navigation-resilience-upstream branch from 929c81c to b199c22 Compare March 10, 2026 01:48
@Adi1231234
Copy link
Copy Markdown
Contributor Author

Hey @BenyFilho, thanks!

Just rebased onto main again and resolved conflicts, CI is passing.

This one is important for anyone running whatsapp-web.js on machines that go to sleep/resume (laptops, servers with power management). I've tested it thoroughly in my own setup and it's been solid. If anyone else has hit this crash and wants to validate, please give it a try!

I really need these fixes merged, they're all critical for my use case. If anyone wants to help review or test, I'd be more than happy to pay for your time. Feel free to reach out.

@BenyFilho
Copy link
Copy Markdown
Member

Hey @BenyFilho, thanks!

Just rebased onto main again and resolved conflicts, CI is passing.

This one is important for anyone running whatsapp-web.js on machines that go to sleep/resume (laptops, servers with power management). I've tested it thoroughly in my own setup and it's been solid. If anyone else has hit this crash and wants to validate, please give it a try!

I really need these fixes merged, they're all critical for my use case. If anyone wants to help review or test, I'd be more than happy to pay for your time. Feel free to reach out.

Hi,

This is a community project, so changes need to be reviewed and validated by other users. They should be useful for the community in general and must not break existing functionality.

I understand that these changes are important for your use case, but they also need to make sense for the community as a whole. The approval decision is based on technical and community impact, not on financial aspects.

Thanks for your understanding.

@Adi1231234
Copy link
Copy Markdown
Contributor Author

Hey @BenyFilho, just want to clarify - I absolutely did not mean paying for approvals. I meant compensating someone for their time reviewing and discussing the best approach, even if the conclusion is that my implementation needs rework.

I share the same philosophy I've seen you promote - if you have a problem, fix it yourself. That's why I've put in the work and opened multiple PRs. Unfortunately it's been hard to get people to test and review them.

I truly believe in this project and would rather contribute back than maintain my own fork with all the fixes. So yes, I'm genuinely willing to pay for help and support to move these fixes forward - for the benefit of the whole community.

Thanks! 🙏

Adi1231234 added a commit to Adi1231234/whatsapp-web.js that referenced this pull request Mar 10, 2026
Integrate changes from upstream PR wwebjs#127083:
- Replace manual while polling loops with page.waitForFunction()
  which natively survives execution context destruction
- Move framenavigated listener registration before inject() in
  initialize() so recovery is in place from the start
@Adi1231234
Copy link
Copy Markdown
Contributor Author

Closing in favor of #201653, which consolidates this PR together with #127090 into a single, more comprehensive fix.

The new PR includes all the changes from both PRs plus additional fixes (listener cleanup, concurrency guard, atomic hasSynced check, qrRetries reset) - all validated with A/B diagnostic testing across 5 real-world scenarios.

Thanks everyone for the feedback and reviews here - it really helped shape the final solution!

@Adi1231234 Adi1231234 closed this Mar 12, 2026
Adi1231234 added a commit to Adi1231234/whatsapp-web.js that referenced this pull request Mar 16, 2026
…127083

- RemoteAuth: pass session name directly to store.save instead of full
  path (fixes PR wwebjs#201660 - path.join was passing the full filesystem
  path instead of just the session identifier)

- Client: register framenavigated listener before calling inject() so
  any page navigation that occurs during initialization is captured
  (fixes PR wwebjs#127083 ordering issue)
Adi1231234 added a commit to Adi1231234/whatsapp-web.js that referenced this pull request Mar 16, 2026
…127083 (#65)

- RemoteAuth: pass session name directly to store.save instead of full
  path (fixes PR wwebjs#201660 - path.join was passing the full filesystem
  path instead of just the session identifier)

- Client: register framenavigated listener before calling inject() so
  any page navigation that occurs during initialization is captured
  (fixes PR wwebjs#127083 ordering issue)
Adi1231234 added a commit to Adi1231234/whatsapp-web.js that referenced this pull request Mar 19, 2026
…127083

- RemoteAuth: pass session name directly to store.save instead of full
  path (fixes PR wwebjs#201660 - path.join was passing the full filesystem
  path instead of just the session identifier)

- Client: register framenavigated listener before calling inject() so
  any page navigation that occurs during initialization is captured
  (fixes PR wwebjs#127083 ordering issue)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

api changes API modifications waiting for testers Needs testing

Projects

None yet

Development

Successfully merging this pull request may close these issues.

inject() fails after sleep/resume due to execution context destruction during page navigation

3 participants