fix(deadline): auto-extend implicit deadline to fit CDP ladder#38
Merged
Conversation
…#35) `chrome_timeout_ms = 30000` was silently clamped to ~2.7s because the CDP fetch path runs `min(internal_timeout, deadline.remaining())` and `deadline_ms_default` defaulted to 8s — too small to cover even one CDP tier (LP 2.5s + Chrome 30s + 28s overhead). Operators raising the per-tier budget saw no effect. Add `request.auto_extend_deadline_for_ladder` (default true): when a request omits `deadlineMs`, widen the effective deadline to `max(deadline_ms_default, sum(per-tier timeouts) + N_cdp_tiers * 28s)`. Explicit per-request `deadlineMs` always wins. Tower outer timeout is sized off the same envelope so middleware can't cancel a healthy handler. The CDP tier count and ladder budget collapse to HTTP-only when the binary is built without the `cdp` feature, so HTTP-only deployments keep their strict default. `wait_for_ms` is clamped to `MAX_WAIT_FOR_MS` (60s) so the inner deadline can never escape the outer Tower envelope. Adds 14 unit tests, 1 drift guard between renderer constants and `CDP_TIER_OVERHEAD_MS`, and 3 ignored integration tests gated behind `CRW_CDP_WS_URL` for live verification.
There was a problem hiding this comment.
Pull request overview
Adjusts request deadline handling so configured CDP tier timeouts (e.g., chrome_timeout_ms) aren’t unintentionally crushed by a small implicit deadline_ms_default (issue #35), and aligns the Tower outer timeout with the same worst-case envelope.
Changes:
- Add
request.auto_extend_deadline_for_ladder(defaulttrue) and compute an auto-extended implicit deadline based on the renderer ladder minimum + boundedwait_for. - Size the Tower
TimeoutLayerusingeffective_request_timeout_secs()and wire the new effective deadline into scrape/search/map/mcp/crawl paths. - Add drift/behavior tests (CDP overhead constant lockstep, plus ignored live-CDP integration tests) and document the new knob in default/docker configs.
Reviewed changes
Copilot reviewed 15 out of 15 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| crates/crw-server/tests/deadline_auto_extend_test.rs | Adds ignored live-CDP integration coverage for auto-extend vs strict deadlines. |
| crates/crw-server/tests/cdp_constants_test.rs | Adds a drift guard asserting core’s CDP overhead constant matches renderer constants. |
| crates/crw-server/src/state.rs | Uses effective per-page deadlines for crawl jobs (includes wait_for when present). |
| crates/crw-server/src/routes/search.rs | Uses effective deadline for enrichment scrapes to avoid ladder clamping. |
| crates/crw-server/src/routes/scrape.rs | Computes request Deadline from effective_deadline_ms(...) instead of raw default. |
| crates/crw-server/src/routes/mcp.rs | Uses effective deadline for MCP scrape + crawl options. |
| crates/crw-server/src/routes/map.rs | Uses effective per-page deadline for map/discover requests. |
| crates/crw-server/src/main.rs | Logs startup transparency when auto-extension raises the implicit default. |
| crates/crw-server/src/app.rs | Sets Tower outer timeout from effective_request_timeout_secs() envelope. |
| crates/crw-renderer/src/cdp.rs | Exposes CDP budget constants/overhead sum; improves clamp diagnostics by snapshotting remaining once. |
| crates/crw-renderer/Cargo.toml | Makes crw-renderer/cdp also enable crw-core/cdp to keep feature-gated logic consistent. |
| crates/crw-core/src/config.rs | Adds deadline auto-extension policy, ladder sizing helpers, max wait clamp, and outer timeout sizing. |
| crates/crw-core/Cargo.toml | Introduces a cdp feature flag used for feature-gated ladder logic in core config. |
| config.docker.toml | Documents auto_extend_deadline_for_ladder and its effect in container defaults. |
| config.default.toml | Documents outer timeout widening behavior and the new request knob (commented). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+36
to
+37
| /// can never escape the outer envelope. Documented as `(0, 60000]` in | ||
| /// `types.rs::ScrapeRequest::wait_for`. |
Comment on lines
+795
to
+800
| // Mirrors crw_renderer::cdp::SPA_SELECTOR_MAX_MS. The CDP module | ||
| // adds `wait_for_ms.unwrap_or(SPA_SELECTOR_MAX_MS)` to its internal | ||
| // timeout, so when the caller exceeds the default we need to extend | ||
| // the deadline per active CDP tier. | ||
| const SPA_DEFAULT_MS: u64 = 8_000; | ||
| // Clamp `wait_for_ms` to MAX_WAIT_FOR_MS so the inner deadline never |
This was referenced May 9, 2026
Closed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
chrome_timeout_ms = 30000was silently clamped to ~2.7s becausemin(internal_timeout, deadline.remaining())ran against an 8sdeadline_ms_default.request.auto_extend_deadline_for_ladder(defaulttrue). When a request omitsdeadlineMs, the effective deadline widens tomax(deadline_ms_default, ladder_min)whereladder_min = sum(per-tier timeouts) + N_cdp_tiers * 28s. Explicit per-requestdeadlineMsalways wins.effective_request_timeout_secs) so middleware can't cancel a healthy handler.cdpfeature) keep their strict default —cdp_tier_count()collapses to 0 and the extension is a no-op.wait_for_msis clamped toMAX_WAIT_FOR_MS = 60sso the inner deadline can never escape the outer envelope.Why
David's Chrome trace shows
Timeout after 2771msdespitechrome_timeout_ms = 30000— the per-tier budget was being silently overruled by the request deadline. Fix gives operators what they configured.Test plan
--features crw-server/cdpcrw-renderer::cdpconstants andcrw-core::config::CDP_TIER_OVERHEAD_MSwait_forextension, HTTP-only short-circuit, no-cdp build, Playwright tier,effective_request_timeout_secsenvelope, search enrichment fan-out, map ceilingCRW_CDP_WS_URL(require live Chrome) — to run locally withcargo test -p crw-server --features cdp --test deadline_auto_extend_test -- --ignoredhttps://x.com/trq212/status/2033949937936085378should now produceTimeout after ~30000ms(full chrome budget) instead ofTimeout after 2771msFiles
crates/crw-core/src/config.rs—CDP_TIER_OVERHEAD_MS,MAX_WAIT_FOR_MS,RequestConfig::auto_extend_deadline_for_ladder,RendererConfig::cdp_tier_count/min_deadline_for_full_ladder_ms,AppConfig::effective_deadline_ms/effective_request_timeout_secscrates/crw-renderer/src/cdp.rs— pub budget constants,cdp_tier_overhead_ms(), snapshot remaining once + diagnostic logcrates/crw-server/src/{app,main,routes/{scrape,search,map,mcp},state}.rs— wireeffective_deadline_mseverywherecrates/crw-server/tests/{cdp_constants_test,deadline_auto_extend_test}.rs— drift + integration coverageconfig.{default,docker}.toml— documented new knob