Repository navigation
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the ✨ Finishing Touches🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
|
Thanks @charlaie — useful addition. Merged into develop (applied manually due to conflicts with newer code). We also added initialization for non-goto code paths (file://, raw:, js_only) that would have caused a NameError, and added integration tests. Your name has been added to CONTRIBUTORS.md. |
Applied manually due to conflicts (PR based on older code). Also fixed missing variable initialization for non-goto paths (file://, raw:, js_only) that would have caused NameError. Closes #1434
- Add tests for device_scale_factor (config + integration) - Add tests for redirected_status_code (model + redirect + raw HTML) - Document device_scale_factor in browser config docs and API reference - Document redirected_status_code in crawler result docs and API reference - Add TristanDonze and charlaie to CONTRIBUTORS.md - Update PR-TODOLIST with session results
|
This has been merged into develop at commit 37a49c5 (applied manually due to conflicts, with bug fix for missing variable initialization on non-goto paths). Closing. |
…nder calls Two upstream defects found in the 2026-07-30 WAA eval, fixed together because they share the retry loop and the capture path. 1. Block detection judged the wrong hop of a redirect chain CrawlResult.status_code carries the FIRST hop (upstream design, PR unclecode#1435) while the HTML comes from the LAST, kept as redirected_status_code. All three is_blocked() call sites fed the first hop, so a 301 was judged instead of the 403 it led to -- and no status rule in is_blocked can fire on a 3xx. Every site that redirects and then serves a block page was returned as success:true with the block page as its content. Finnish company sites redirect almost universally, so this poisoned the corpus at scale. Fix: antibot_detector.effective_status(), used at the three call sites. status_code itself is unchanged -- upstream semantics, and MAS may branch on it. 2. page.content() and page.evaluate() are bounded by nothing Root cause isolated by reproduction (local fixture origin + await-chain dump), not inference: both are sent to the Playwright driver with no timeout field, so no timer is armed, and they wait on the frame's execution-context promise -- which every navigation replaces with a fresh unresolved one. page_timeout reaches only page.goto and the wait_* family, which is why the forensics matrix saw 80s -> 30s change nothing, and both call sites sit inside swallow-all try/except, which is why 172s passed in silence before the fence fired. Fix, outermost last: bounded_evaluate() in all three adapters (30s, per-call override); 10s on the optional DOM steps; _capture_html() with settle-and-retry for page.content() (15s/attempt, 25s group budget); 10s on page.close(); and a new CrawlerRunConfig.total_timeout shared by every attempt in arun(), set to 100s in config.yml (server-side only -- not in UNTRUSTED_FIELD_ALLOWLIST). Measured end-to-end against a fixture origin, MAS V14-shaped request: - navigation race (the maitokolmio.fi shape): 504 @ 180s -> HTTP 200, full content, 5.0s - permanently wedged page: 504 @ 180s with no diagnostic -> HTTP 500 @ 94s with the exact reason logged - benign 301 -> 200: unchanged, 200 + success, 3.7s - 301 -> 403 block: 200 + block page as content -> HTTP 500 Tests: two new offline suites (28 tests, no server/browser). Offline gate is now 64 tests. The redirect suite was verified to fail on the unpatched tree. NOT deployed. Deploy is gated on Tero warning MAS: after this lands, a redirect-to-block host returns an opaque HTTP 500 (server.py all-failed -> 500, security handler strips the detail), so the redirected_status_code >= 400 client-side mitigation we recommended stops being available for those hosts. That is an enlargement of an existing gap, not a new one -- every full-mode failure already reaches MAS as an opaque 500 -- and it is what Q2 must settle. Recorded in the forensics record and in the classification task. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Summary
Add redirected_status_code to CrawlResult so that there is a way to tell whether the redirected response is successful or not
Fixes #1434
List of files changed and why
_crawl_webasync_responseto thecrawl_resultredirected_status_codefield toCrawlResultandAsyncCrawlResponseHow Has This Been Tested?
I have tested with an example site containing working and failing redirect links, and the redirected status code is correctly set
Checklist:
Results
Here is the new response from the fail and success redirect site (some fields removed for conciseness)