feat(config): let the LightPanda escalation target a renderer that exists - #506
Conversation
|
All contributors have signed the CLA ✍️ ✅ |
|
Thanks for this one, and for the write-up. The diagnosis is right, and it is worse than the PR I have pushed a commit onto your branch that takes it a different way, and I want to explain There is exactly one correct answer at that line, "the strongest tier this deployment actually What is on the branch now: // crw-renderer: the tiers the auto chain may enter on its own
pub fn auto_ladder_names(&self) -> Vec<&str>
// crw-renderer: which tier a post-LightPanda escalation should aim at
pub fn lightpanda_escalation_target(&self) -> Option<&str>// crw-crawl/src/single.rs
let escalation_target: Option<&str> = if prior_renderer == Some("lightpanda") {
renderer.lightpanda_escalation_target()
} else {
pinned
};Three details that took a second review round to get right, and are worth calling out because
Tests, all passing:
The config key is gone from the branch. If you would still like an operator override on top, One housekeeping note: the branch now carries your |
07a02bb to
90f8585
Compare
The post-LightPanda escalation pinned the literal "chrome". A pinned name the pool does not hold is a hard error, not a fallback, so on any deployment without a Chrome CDP sidecar every escalation failed on the pin rather than on the page, and a stronger tier that was configured and healthy was never reached. The shipped config.default.toml is one of those deployments: it sets [renderer.lightpanda] with [renderer.chrome] commented out. lightpanda_escalation_target() keeps chrome as the first choice, so a pool that holds it behaves exactly as before, and otherwise falls back to the next tier the auto chain would enter by itself. auto_ladder_names() supplies that list and drops the tiers a name-pin would otherwise smuggle past a gate: a camoufox held out by include_in_auto = false, and chrome_proxy under auto_egress_escalation, where the chain lifts it out of the ladder and fires it only on a hard block. playwright is never chosen because it has no RendererKind, so a render on it would carry no breaker, renderDecision or creditCost. None means there is nothing above lightpanda. The escalation is then skipped rather than dispatched, because "auto" would re-render the same tier for the same thin result, and the response carries a warning naming the missing chrome tier so the operator learns what to configure.
90f8585 to
3e2ac05
Compare
|
collapsed the branch into one |
The dead end
scrape_url_innerescalates after a thin LightPanda body and hardcodes the next tier:https://github.com/us/crw/blob/c74bcf3/crates/crw-crawl/src/single.rs#L456-L462
The pool has no tolerance for a name it does not hold — a pinned renderer that is absent is a hard error, not a fallback:
https://github.com/us/crw/blob/c74bcf3/crates/crw-renderer/src/lib.rs#L2401
"requested renderer '{}' not in pool [{}]"Those two compose into a dead end on any deployment that runs no Chrome CDP sidecar. Every post-LightPanda escalation is pinned to a tier that cannot be constructed, so it fails on the pin rather than on the page — and stronger tiers that are configured and healthy (camoufox in particular) are never reached. The
elsearm already defers to the chain so the http tier can find chrome through the normal failover path; only the LightPanda arm is nailed shut.The change
extraction.lightpanda_escalation_renderer,#[serde(default)]→"chrome". An install that does not set it is byte-identical to today; the only behaviour that changes is for a config that opts in. Env:CRW_EXTRACTION__LIGHTPANDA_ESCALATION_RENDERER.Why this is not visible upstream
Same shape as #485: a dead path that only a particular deployment form exposes. If you run the Chrome sidecar, the hardcoded name is always correct and the branch is invisible. It is only wrong for installs whose ladder tops out somewhere else, and those installs cannot report it as a renderer bug because the error surfaces as a pool-membership error, not a scrape failure.
Tests
The sister key
lightpanda_retry_threshold_bytesis covered by a default assertion and a TOML-override assertion; the new key gets one of each, in the same two tests:config::tests::extraction_config_defaults— asserts the default is"chrome"config::tests::extraction_config_toml_scalar_overrides— asserts a TOML override landscargo test -p crw-crawl --libis green exceptpdf::tests::convert_pdf_bytes_*(2), which fail identically on unmodifiedc74bcf3on this machine — a local pdfium/native-lib issue on Windows, not related to this change.