Skip to content

feat: enhance crawl error diagnostics and configurable headers - #483

Merged
us merged 2 commits into
us:mainfrom
VIVAAN-DHAWAN:feature/enhanced-crawl-errors
Aug 31, 2026
Merged

feat: enhance crawl error diagnostics and configurable headers#483
us merged 2 commits into
us:mainfrom
VIVAAN-DHAWAN:feature/enhanced-crawl-errors

Conversation

@VIVAAN-DHAWAN

Copy link
Copy Markdown
Contributor

This PR adds configurable User-Agent headers to the Crawl request, and emits failed fetches as ScrapeData objects with an error field to improve visibility.

@github-actions

github-actions Bot commented Aug 30, 2026

Copy link
Copy Markdown
Contributor

All contributors have signed the CLA ✍️ ✅
Posted by the CLA Assistant Lite bot.

@VIVAAN-DHAWAN

Copy link
Copy Markdown
Contributor Author

I have read the CLA Document and I hereby sign the CLA

github-actions Bot added a commit that referenced this pull request Aug 30, 2026
VIVAAN-DHAWAN and others added 2 commits August 31, 2026 14:11
This commit adds configurable User-Agent support for crawls by exposing  on CrawlRequest, and improves error visibility by yielding ScrapeData for pages that fail during a crawl, with an  field mapped from the fetch error.
- CrawlRequest.headers now reaches every page fetch. /v1 accepts it flat or
  under scrapeOptions; /v2 reads scrapeOptions.headers and rejects a malformed
  value instead of silently dropping it.
- A URL the crawl cannot read is returned marked through the existing block
  field rather than vanishing from the results array, so the caller learns
  which URL failed and why while completed minus blocked keeps it unbilled.
  Applied to transport failures, CDN origin errors, PDF conversion failures
  and extraction failures alike.
- Link discovery compares the full origin, port included, matching the map
  path. A link on another port of the same host is no longer followed, which
  also stops the caller's headers reaching a different service.
- A blocked document now reports zero credits on /v2 instead of one. The
  envelope total already excluded it; only the per-document field disagreed.
- crw crawl reports the pages it returned and the unreadable ones separately
  instead of folding both into one count.
- docs/docs/crawling.md and both OpenAPI copies document the new field,
  including the warning that a browser render applies headers to every
  subresource request.
@us
us force-pushed the feature/enhanced-crawl-errors branch from 8e364b3 to ba12e8a Compare August 31, 2026 11:26
@us

us commented Aug 31, 2026

Copy link
Copy Markdown
Owner

thanks for this, and sorry for the slow reply. both halves of the idea are right and worth having: a crawl that quietly drops the URLs it could not read is genuinely hard to debug, and the crawl path was passing an empty header map while /v1/scrape had honoured headers for a while. good catch on both.

i pushed a top-up commit to your branch (ba12e8a) rather than sending you a long list, so the PR is ready to go as it stands. what changed and why:

it did not compile. total_pages in crawl.rs is not bound anywhere, and adding error to ScrapeData broke 14 exhaustive struct literals across 11 files (crw-extract/src/lib.rs:881, crw-crawl/src/pdf.rs, crw-cli, crw-server/src/state.rs, the v2 routes, four test files). you would not have seen this: PRs from forks need a maintainer to approve workflow runs, so only the CLA check had run and the other six were sitting pending. that is on our side, not yours. cargo fmt --check and clippy -D warnings were also red (trailing whitespace, and field_reassign_with_default on the let mut x = X::default() pattern).

ScrapeData.error came back out. the struct already has block: Option<BlockOutcome> for exactly this, and the billing contract is built on it: /v1 and /v2 turn block into a not-billed page, and the SaaS charges completed - blocked. your diff set error and block.reason to the same string on the fetch-failure path. dropping the field made all 14 compile errors go away and kept the behaviour you wanted.

a few things your version surfaced that turned out to be older bugs, now fixed in the same commit since the headers field is what makes them bite:

  • the same-origin gate built scheme://host and dropped the port, so a crawl seeded at https://example.com/ followed https://example.com:9999/admin as the same site. harmless before; with headers it would replay the caller's Authorization to a different service. it now compares the full origin, matching what map next door already did.
  • a blocked document reported metadata.creditsUsed: 1 on /v2 even though the envelope total correctly excluded it. now zero.
  • crw crawl printed state.completed, which after your change includes unreadable pages, so it would announce "10 pages" having printed 7. it now reports the two separately.

also filled in: the placeholder is applied to PDF-conversion and extraction failures too, not just the two paths you touched, so the contract holds everywhere. and docs/docs/crawling.md plus both OpenAPI copies now document headers, carrying the same warning scraping.md does about a browser render applying headers to every subresource request.

one note on tests. crawl_request_headers_round_trip passes whether or not the wiring exists, so it could not catch a regression. i added crates/crw-crawl/tests/crawl_headers_and_failures.rs, four wiremock tests that drive run_crawl end to end, and checked they are load-bearing by mutation: reverting the fetch call to an empty header map fails the header test, and deleting the CDN failure branch fails the failure test.

verified green locally on check/fmt/clippy -D warnings/cargo test --workspace, and confirmed against a running server that /v1/crawl headers reach the target and a crawl of a dead origin comes back as completed 1, blocked 1, creditsUsed 0.

CI should go green now. thanks again for finding this one, it was a real gap.

@us
us merged commit c58a8f6 into us:main Aug 31, 2026
12 checks passed
@github-actions github-actions Bot locked and limited conversation to collaborators Aug 31, 2026
@us

us commented Aug 31, 2026

Copy link
Copy Markdown
Owner

merged as 93b4c11 (your commit) plus c58a8f6 on top. all 12 checks green, rebased onto main so your authorship stays on the feature commit.

heads up on timing: opencore is the engine, and a merge to main is not gated on a release. CI on main auto-bumps deploy/opencore.pin in the SaaS repo, which rebuilds the production engine from that SHA. so this is live within the hour rather than at the next tag.

thanks for the report and the fix. the header gap was real, and chasing it down is what surfaced two older bugs we would not have found otherwise: the same-origin check had been dropping the port since forever, and blocked pages were reporting a credit they were never charged. both are fixed in the same commit.

if you want another one, crawl.rs still skips robots-disallowed URLs silently, which is arguably correct, but there is no way for a caller to see which URLs a crawl chose not to visit. happy to talk through it on an issue first if you are interested.

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.

2 participants