fix(screenshot): never let the text walk fail a capture

[Kikobeats]

Bug

waitForReady's snapshot builds a TreeWalker over document.body || document.documentElement. A document that is mid-parse or detached has neither, and createTreeWalker throws on a null root.

That throw is not a context-destroyed error, so the gate rethrows it by design (added in #841 so genuine evaluation bugs would not masquerade as timeouts). The effect is that a survivable, transient DOM state fails the entire capture.

Observed on production 3.42.8, three times in six hours, across very different documents:

pages   n  message
    6   1  Failed to execute 'createTreeWalker' on 'Document'
   25   3  Failed to execute 'createTreeWalker' on 'Document'
  426   6  Failed to execute 'createTreeWalker' on 'Document'

Fix

Guard the root, and wrap the walk so a DOM mutating underneath it keeps whatever was counted:

const textRoot = document.body || document.documentElement
const walker = textRoot
  ? document.createTreeWalker(textRoot, window.NodeFilter.SHOW_TEXT)
  : { nextNode: () => null }
try {
  while (text < 200) { ... }
} catch {
  // keep whatever was counted before the DOM shifted
}

text is only a paint signal. Failing to read it means "no text seen yet", which leaves the gate more conservative, not less: it re-polls, or falls through to the blank-page screenshot check. It can never make a blank page look painted.

The rethrow for genuine evaluation errors is untouched, and its test still passes.

Tests: 8 pass.

🤖 Generated with Claude Code

---

Note

Low Risk
Localized defensive changes to screenshot readiness heuristics; behavior when the DOM is normal is unchanged, and the failure mode becomes more conservative polling rather than looser capture.

Overview
Fixes production capture failures where waitForReady's in-page snapshot() threw on transient DOM states (mid-parse or detached documents with no body/documentElement), which bubbled out of page.evaluate and aborted the screenshot because only context-destroyed errors are retried.

snapshot() now routes layout reads through a guarded root (scrollHeight, viewport fallbacks), only creates a TreeWalker when document.body || root exists (otherwise a no-op walker), and wraps the text walk in try/catch so DOM changes mid-scan keep partial counts instead of killing the poll. Missing text is treated as "not painted yet," so the gate stays conservative and re-polls rather than falsely declaring readiness.

Adds a live browser test that removes documentElement and asserts the gate returns zeroed signals without timing out as a hard failure.

^{Reviewed by Cursor Bugbot for commit f70e7d2. Bugbot is set up for automated code reviews on this repo. Configure here.}

Summary by CodeRabbit

• Bug Fixes
  • Improved screenshot readiness checks to handle pages that are still parsing or have an incomplete document structure.
  • Made text-paint sampling more resilient to DOM mutations during traversal.
  • Updated height and metrics handling so captures degrade gracefully when the document root is missing, avoiding failures/timeouts.
• Tests
  • Added an end-to-end test covering the “missing document root” scenario to verify the operation settles and returns zeroed metrics without timing out.

[coderabbitai]

📝 Walkthrough

Walkthrough

Snapshot readiness sampling now handles missing document roots, detached DOM traversal, and traversal mutations without aborting. A live test verifies that removing the document root produces zero measurements and a non-timeout result.

Changes

Snapshot readiness resilience

Layer / File(s)	Summary
Guard snapshot sampling packages/screenshot/src/wait-for-ready.js	Uses null-safe roots for viewport and height calculations, falls back when no text root exists, and catches text traversal failures while retaining counted text.
Validate missing-root behavior packages/screenshot/test/wait-for-ready-live.js	Removes document.documentElement in a live browser test and verifies readiness settles with zero height, text, and painted values without timing out.

Estimated code review effort: 2 (Simple) | ~10 minutes

Possibly related PRs

• microlinkhq/browserless#837: Updates the same waitForReady snapshot readiness implementation.

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly matches the main change: hardening the screenshot text walk so captures do not fail.

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch fix/tree-walker-guard

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[coveralls]

coverage: 80.429% (-0.4%) from 80.796% — fix/tree-walker-guard into master

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@packages/screenshot/src/wait-for-ready.js`:
- Around line 130-133: Safeguard the return payload in the wait-for-ready
capture logic so accesses to document.documentElement.scrollHeight and related
dimensions remain safe when documentElement is null; reuse the existing
document/body fallback where appropriate. Also update the viewport dimension
fallback near the initial width/height calculations to avoid dereferencing
document.documentElement when window.innerWidth or innerHeight is falsy.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: bc3cf399-3c20-48a8-bed8-ae61902a227f

📥 Commits

Reviewing files that changed from the base of the PR and between e1745de and e59c00c.

📒 Files selected for processing (1)

• packages/screenshot/src/wait-for-ready.js