fix(pdf): navigation-tolerant readiness gate; skip screenshot poll for painted pages

[Kikobeats]

Problem

prepare()'s auto readiness gates on a screenshot poll (waitUntilAuto): screenshot → isWhite? → repeat. In the GPU-less fleet (llvmpipe) this has two failure modes:

• Slow. Screenshots are software-rasterized (seconds each), so readiness alone burns 7–14s even for tiny documents.
• Fragile. The poll throws Execution context was destroyed, most likely because of a navigation / Page.captureScreenshot: Target closed when a page client-navigates under it. Many pages (webtoon readers, scribd) re-commit their own URL right after load, so the capture races the re-nav and the whole render fails.

Downstream (microlink-api) this surfaces as PDF requests silently falling back to a full single-shot render and timing out at 28s.

Change

Add waitForReady to @browserless/screenshot: a navigation-tolerant gate that resolves once the page is visually quiet — document height stable, every image decoded, readyState === 'complete' — held for quietMs, taking no screenshots. When page.evaluate throws on a destroyed context it resets the quiet window and keeps polling instead of failing.

prepare() now runs waitForReady first. Real painted content (decoded images in a document taller than the viewport) returns immediately; only a page that settles still-blank falls back to the original screenshot poll, so the blank-SPA protection is preserved.

Validation

• 4 deterministic unit tests for the gate (quiet→resolve, nav-reset→resolve, never-settles→timeout, imageless page).
• Real-seam A/B on a problem URL: the new path yields a complete, non-blank, deterministic PDF (25 pages both runs) where the old poll captured early and varied (23 pages, size swinging run-to-run).
• No API-surface change; waitForReady is additive.

🤖 Generated with Claude Code

---

Note

Medium Risk
Changes core PDF readiness timing and fast-path heuristics for all waitUntil: 'auto' renders; misclassification could skip blank checks or still time out, but behavior is heavily tested and the screenshot fallback remains.

Overview
Adds waitForReady in @browserless/screenshot: a screenshot-free gate that polls in-page paint/settle signals (stable height, images decoded, complete, visible images/text/fonts) and absorbs SPA navigations by resetting on destroyed execution contexts instead of failing.

PDF prepare() in auto mode now runs waitForReady first (half the action timeout), then skips the white-screenshot poll when the gate settles with real painted content (visible images, ≥200 chars of visible text with fonts loaded, tall document)—with guards for tracking pixels, pending webfonts, and timed-out gates. Still-blank pages keep the existing screenshot poll; gate + poll + capture retries share one timeout budget so worst-case prepare cannot exceed a single timeout.

Exports waitForReady for reuse. Tests cover the gate (unit + live DOM), PDF fast-path/skip cases, and shared-budget behavior.

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

Summary by CodeRabbit

• Improvements
  • Added a navigation-tolerant “visual quiet” readiness gate to determine when screenshot-based blank-page protection is necessary.
  • Unified readiness and fallback timing so the overall timeout budget is respected.
  • Exposed the readiness helper for reuse by other components.
• Bug Fixes
  • Reduced unnecessary screenshot polling when meaningful painted content is already stable.
• Tests
  • Expanded readiness and PDF flow coverage (SPA navigation recovery, timeout handling, imageless readiness, and painted fast-path screenshot skipping).

[coderabbitai]

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

• @coderabbitai resume to resume automatic reviews.
• @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

• ▶️ Resume reviews
• 🔍 Trigger review

📝 Walkthrough

Walkthrough

Adds a navigation-tolerant waitForReady polling helper, exports it from the screenshot package, and integrates it into PDF waitUntil: 'auto' handling before blank-page screenshot retries. Tests cover readiness states, navigation resets, timeouts, and painted-content fast paths.

Changes

Readiness gate and PDF integration

Layer / File(s)	Summary
Readiness polling contract and validation packages/screenshot/src/wait-for-ready.js, packages/screenshot/src/index.js, packages/screenshot/test/wait-for-ready.js	Adds readiness snapshots with painted-image tracking, quiet-window polling, navigation recovery, public exports, and deterministic tests for success, resets, timeout, validation, and imageless pages.
PDF auto readiness and blank-page fallback packages/pdf/src/index.js, packages/browserless/test/pdf.js	Uses a budgeted readiness gate for painted-content early returns, shares the timeout with white-screen retries, and tests painted, pixel-only, and timed-out readiness scenarios.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Sequence Diagram(s)

sequenceDiagram
  participant PDFPrepare
  participant WaitForReady
  participant BrowserPage
  participant ScreenshotCapture
  PDFPrepare->>WaitForReady: wait for stable height, decoded images, and complete readyState
  WaitForReady->>BrowserPage: evaluate readiness snapshot
  BrowserPage-->>WaitForReady: snapshot or navigation error
  WaitForReady-->>PDFPrepare: readiness result
  PDFPrepare->>ScreenshotCapture: check for likely white screen
  ScreenshotCapture->>BrowserPage: capture screenshot
  BrowserPage-->>ScreenshotCapture: screenshot pixels
  ScreenshotCapture-->>PDFPrepare: blank or non-white result

Loading

Possibly related PRs

• microlinkhq/browserless#835: Updates the same PDF prepare() auto-navigation and blank-page handling logic.

🚥 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 describes the main PDF readiness-gating change and the painted-page screenshot skip.

✨ 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/pdf-readiness-gate

---

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.}

[coderabbitai]

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

packages/pdf/src/index.js (1)

93-138: 🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy lift

Fix global timeout leakage and redundant screenshots.

This block introduces two significant issues that degrade stability and performance:

1. Global Timeout Leakage: timePdf is initialized after waitForReady and the first screenshot check. Since both of those operations can take up to timeout milliseconds, the global timeout can be massively exceeded before the fallback loop even starts tracking time. timePdf should be initialized at the top of waitUntilAuto, and subsequent operations must use the remaining time.
2. Redundant Screenshots: If the initial single white-screen check correctly detects a blank page, the code falls into the do-while loop and immediately takes another screenshot without waiting. This duplicates an expensive operation.

Refactoring to initialize the timer early, update the remaining timeout on each call, and use a while loop resolves both issues.

🛠️ Proposed refactoring

     async function waitUntilAuto (page) {
       await waitForDomStabilityResult(page)
       const timeout = goto.timeouts.action(rest.timeout)
+      const timePdf = timeSpan()
 
       // Cheap, navigation-tolerant readiness — no screenshots. Resolves once the
       // page is visually quiet (height stable, images decoded, load complete),
       // absorbing the client-side re-navigation that makes a screenshot poll
       // throw `Execution context was destroyed`.
       const readyTime = timeSpan()
-      const ready = await waitForReady(page, { timeout })
+      const ready = await waitForReady(page, { timeout: Math.max(0, timeout - timePdf()) })
       debug('ready', { ...ready, duration: readyTime() })
 
       // Fast path: real painted content — decoded images in a document taller
       // than the viewport can't be a blank shell, so skip the screenshot poll.
       const viewportHeight = (page.viewport() || {}).height || 0
       if (ready.decoded > 0 && ready.height > viewportHeight) return
 
+      if (timePdf() >= timeout) return
+
       // Otherwise a single white-screen check, now that the page has settled (so
       // it won't race a navigation). A genuinely blank page falls back to the
       // original screenshot poll to keep the blank-SPA protection.
       const shot = await pReflect(
         captureWithNavigationRetry(
           () => page.screenshot({ ...rest, optimizeForSpeed: true, type: 'jpeg', quality: 30 }),
-          { page, goto, timeout }
+          { page, goto, timeout: Math.max(0, timeout - timePdf()) }
         )
       )
-      if (shot.isFulfilled && !(await isWhiteScreenshot(shot.value))) return
-
-      let isWhite = true
-      let retry = -1
-      const timePdf = timeSpan()
-      do {
-        ++retry
-        const screenshotTime = timeSpan()
-        const screenshot = await captureWithNavigationRetry(
-          () =>
-            page.screenshot({
-              ...rest,
-              optimizeForSpeed: true,
-              type: 'jpeg',
-              quality: 30
-            }),
-          { page, goto, timeout }
-        )
-        isWhite = await isWhiteScreenshot(screenshot)
-        if (isWhite) await goto.waitUntilAuto(page, { timeout: rest.timeout })
-        debug('retry', { waitUntil, isWhite, retry, duration: screenshotTime() })
-      } while (isWhite && timePdf() < timeout)
+      let isWhite = shot.isRejected || (await isWhiteScreenshot(shot.value))
+      if (!isWhite) return
+
+      let retry = 1
+      while (isWhite && timePdf() < timeout) {
+        await goto.waitUntilAuto(page, { timeout: Math.max(0, timeout - timePdf()) })
+
+        const screenshotTime = timeSpan()
+        const screenshot = await captureWithNavigationRetry(
+          () =>
+            page.screenshot({
+              ...rest,
+              optimizeForSpeed: true,
+              type: 'jpeg',
+              quality: 30
+            }),
+          { page, goto, timeout: Math.max(0, timeout - timePdf()) }
+        )
+        isWhite = await isWhiteScreenshot(screenshot)
+        debug('retry', { waitUntil, isWhite, retry, duration: screenshotTime() })
+        retry++
+      }
 
       debug({ waitUntil, isWhite, timeout, duration: require('pretty-ms')(timePdf()) })
     }

🤖 Prompt for 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.

In `@packages/pdf/src/index.js` around lines 93 - 138, Update the waitUntilAuto
flow around waitForReady and captureWithNavigationRetry to initialize timePdf
before any readiness or screenshot work, pass the remaining timeout to each
subsequent operation, and stop when that budget is exhausted. Replace the
do-while fallback with a while loop so the first retry screenshot occurs only
after the existing white-page wait, avoiding an immediate duplicate capture.

🤖 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 31-35: Validate timeout at the start of waitForReady before
calculating deadline, and fail fast when it is missing or undefined instead of
allowing Date.now() + timeout to produce NaN. Preserve the existing behavior for
valid timeout values and the subsequent readiness loop.

---

Outside diff comments:
In `@packages/pdf/src/index.js`:
- Around line 93-138: Update the waitUntilAuto flow around waitForReady and
captureWithNavigationRetry to initialize timePdf before any readiness or
screenshot work, pass the remaining timeout to each subsequent operation, and
stop when that budget is exhausted. Replace the do-while fallback with a while
loop so the first retry screenshot occurs only after the existing white-page
wait, avoiding an immediate duplicate capture.

🪄 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: 9097e869-3473-4c82-bbf7-b0217bcc20d8

📥 Commits

Reviewing files that changed from the base of the PR and between fea7396 and 44d624d.

📒 Files selected for processing (4)

• packages/pdf/src/index.js
• packages/screenshot/src/index.js
• packages/screenshot/src/wait-for-ready.js
• packages/screenshot/test/wait-for-ready.js

[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`:
- Line 30: Update the timeout validation in the wait-for-ready logic to use
Number.isFinite(timeout), rejecting NaN and Infinity while preserving the
existing TypeError message and behavior for invalid values.

🪄 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: 24e1cdd3-3ee8-46e9-897d-e2726ee3519c

📥 Commits

Reviewing files that changed from the base of the PR and between 44d624d and 0c1f873.

📒 Files selected for processing (4)

• packages/pdf/src/index.js
• packages/screenshot/src/index.js
• packages/screenshot/src/wait-for-ready.js
• packages/screenshot/test/wait-for-ready.js

🚧 Files skipped from review as they are similar to previous changes (2)

• packages/screenshot/test/wait-for-ready.js
• packages/pdf/src/index.js

[coveralls]

coverage: 82.052% (-1.0%) from 83.062% — fix/pdf-readiness-gate 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 18-28: Update the snapshot function’s painted-image calculation to
count only images that are visibly rendered in the viewport, excluding
visibility:hidden, opacity:0, and off-screen elements before incrementing
painted. Preserve decoded counting and the existing size threshold for eligible
visible images.

🪄 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: 8d0445bb-5b85-4887-8111-acd0c0607ca9

📥 Commits

Reviewing files that changed from the base of the PR and between 8cbe46a and 5f6b7b5.

📒 Files selected for processing (4)

• packages/browserless/test/pdf.js
• packages/pdf/src/index.js
• packages/screenshot/src/wait-for-ready.js
• packages/screenshot/test/wait-for-ready.js

🚧 Files skipped from review as they are similar to previous changes (2)

• packages/screenshot/test/wait-for-ready.js
• packages/pdf/src/index.js

[cursor]

Auto lazy images stall gate

High Severity

The settle gate only skips below-fold images when loading === 'lazy', but Chromium also defers offscreen images with the default loading="auto". Those incomplete images are still counted toward images without ever decoding during PDF prep, so decoded stays below images, the quiet condition never holds, and waitForReady burns its full budget on every long page before falling back to the screenshot poll.

 

^{Reviewed by Cursor Bugbot for commit 9003b31. Configure here.}

[Kikobeats]

Not applicable to the Chromium versions this project ships. Automatic lazy-loading of loading="auto" images only ever existed behind Lite Mode / Data Saver on Android (LazyImageLoading feature), and was removed when Lite Mode was discontinued (M100, 2022). In current headless Chromium the default loading behavior is eager: offscreen images without an explicit loading="lazy" do start fetching immediately, flip complete, and satisfy decoded >= images. The gate's live test suite exercises exactly this (eager offscreen images resolve; only an explicit below-fold loading="lazy" image is excluded, wait-for-ready-live.js). Even in the hypothetical, the failure mode is bounded: the gate times out at 50% of the action budget and falls back to the screenshot poll — degraded latency, not a correctness break.

[cursor]

Cursor Bugbot has reviewed your changes and found 2 potential issues.

There are 3 total unresolved issues (including 1 from previous review).

^{❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.}

^{Reviewed by Cursor Bugbot for commit b496542. Configure here.}