Skip to content

fix(renderer): stop shipping an unclearable wall as a success - #496

Merged
us merged 1 commit into
mainfrom
fix/wall-thin-not-success
Sep 3, 2026
Merged

fix(renderer): stop shipping an unclearable wall as a success#496
us merged 1 commit into
mainfrom
fix/wall-thin-not-success

Conversation

@us

@us us commented Sep 3, 2026

Copy link
Copy Markdown
Owner

Follow-up to #494. That PR fixed a real bug but not this one, and prod proved it: after #494 reached the box, the same request still came back as a billed success.

The case, live on prod today

POST /v1/scrape {"url":"https://www.prlib.ru/en/history/619410"}

success: true            creditCost: 1
markdown: "# Security Check\n\nChecking your browser before accessing the site\n\nThis will take a few seconds"   (95 chars, entire body)
metadata.renderedWith: "lightpanda"
renderDecision: {"kind":"failover","chain":["lightpanda","chrome","chrome_proxy"],"reason":"other"}

A customer paid a credit for an interstitial, and an agent consuming this reads "Checking your browser" as the page.

Why #494 did not catch it

#494 threaded the renderer's truncation flag into the wall detector, because a budget-truncated body has no closing </body> and was extracting as an empty string. Correct, and it stands. But it is not what was happening here: I pulled the real rawHtml (5,462 chars) and it has a closing </body>, 77 visible characters, and contains two phrases from the detector's list. So looks_like_generic_bot_wall returns true for this input with or without that flag.

Which meant the wall was being detected, and something downstream ignored the answer.

The actual cause

FallbackRenderer::fetch_with_js, the tail after the ladder is exhausted:

// Return the best thin result if we have one, otherwise the last error.
if let Some(mut result) = thin_result {
    result.credit_cost = credit_for(last);
    result.render_decision = Some(RenderDecision::Failover { chain, reason });
    ...
    Ok(result)

Every tier classified the body and rejected it — that rejection is exactly why it sits in thin_result rather than having been accepted. The tail then discards the verdict, stamps a page credit and returns it as a success. The existing comment above the pinned-renderer hint already names the symptom: "silently returning broken markdown with success: true". That hint only covers the hard-pinned single-tier case; the auto-failover path had nothing.

The tail now re-reads the verdict and fails the request.

Scope, deliberately narrow

Only the generic phrase-list wall converts to an error. A Cloudflare challenge or a named vendor shell keeps shipping as a thin result, because those have their own recovery arms (cloak, chrome_proxy) that read the body back — cloak_recover_on_cf_false_skips_arm_on_exhausted_deadline pins that shape and my first, broader attempt broke it. A thin but wall-free page still ships, which is what the recall invariant rests on.

Verification

  • cargo test --workspace: 4226 passed
  • cargo test -p crw-renderer --features camoufox,cloak,cdp: 1024 passed (the ladder tail lives behind those default-off features for two of the tiers)
  • clippy clean in both configurations, fmt clean
  • Two new tests, one per half of the gate. Disabling the guard fails the wall test and leaves the thin test passing, so each pins what it claims to.

Merging reaches prod within the hour; I will re-run the live case against the new engine and report the result on this PR.

When every renderer tier rejects a body, the ladder tail returns the best of
them so the caller gets something rather than nothing. That is right for a page
that is merely thin, and wrong for an anti-bot wall: prod handed a paying
customer "Security Check / Checking your browser", 95 characters, as
`success: true` with `creditCost: 1`.

Each tier had already classified it — that verdict is why the body landed in
`thin_result` instead of being accepted — and the tail was the one place that
discarded the verdict, stamped a page credit and returned the interstitial.
The tail now re-reads it and fails the request instead.

Scoped to the generic phrase-list wall on purpose. A Cloudflare challenge or a
named vendor shell keeps shipping as a thin result: those have their own
recovery arms that read the body back, and a test pins that shape. A thin but
real page still ships, which the recall invariant depends on.

Found because the earlier truncation fix did not change this case: the wall
HTML carries a closing </body>, so the truncation flag was never the reason it
went undetected.
@us

us commented Sep 3, 2026

Copy link
Copy Markdown
Owner Author

Why the per-attempt gate never ran

An independent trace of this request confirms the tail is the only path, and explains the piece I had left open — why the accept gate with && !is_bot_wall did not fire:

  1. The chrome hedge fires: lightpanda and chrome race concurrently. classify_js_attempt evaluates both, and its acceptable flag does check looks_like_generic_bot_wall. Neither tier was acceptable.
  2. try_hedge therefore returns HedgeOutcome::Thin, carrying the richer of the two bodies (max_by_key(html.len())) — lightpanda's 5,462-char wall shell. That becomes thin_result, and hedge_done = true.
  3. The serial for renderer in renderers loop opens with if hedge_done { break; }. The accept gate lives inside that loop, so on a hedged request it is never reached. It gates individual attempts, not the surviving result.
  4. saw_hard_block was set (the wall), so the chrome_proxy recovery arm fired and pushed ChromeProxy onto the chain before its fetch — which is why the chain shows three tiers. Its own result was re-validated and was not better, so thin_result stayed lightpanda's, which is why renderedWith is still "lightpanda".
  5. last_failover_reason is only assigned in the serial loop's error branch, which never ran, so it defaulted to Other — matching the observed "reason":"other".

Every tier that touched this body had already flagged it as a wall. The gap is that nothing re-read that verdict before the final Ok(result), so it shipped as "the best of what we tried". That is exactly where this patch sits.

It also independently confirms the truncation flag is orthogonal here: both <body> tags were present, so #494's parameter could not have suppressed or caused this match either way.

@us
us merged commit 82a56f6 into main Sep 3, 2026
10 checks passed
@github-actions github-actions Bot locked and limited conversation to collaborators Sep 3, 2026
@us

us commented Sep 3, 2026

Copy link
Copy Markdown
Owner Author

Confirmed on prod once #497 landed alongside it: the wall now returns success:false with no credit and no markdown. This PR's guard was firing all along (its message appears verbatim in the engine log), but the HTTP-shell fallback one layer up was substituting the same wall back in. #497 closes that. Verification detail is on #497.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant