feat: enhance crawl error diagnostics and configurable headers

crates/crw-cli/src/commands/crawl.rs

@@ -153,6 +153,8 @@ pub async fn run(mut args: CrawlArgs) -> Result<(), CmdError> {
         country: None,
         proxy_list: Vec::new(),
         proxy_rotation: None,
+        // No CLI flag for per-request headers, same as `crw scrape`.
+        headers: std::collections::HashMap::new(),
     };
 
     let id = Uuid::new_v4();
@@ -244,7 +246,19 @@ pub async fn run(mut args: CrawlArgs) -> Result<(), CmdError> {
         // Check if done
         match state.status {
             CrawlStatus::Completed => {
-                eprintln!("Crawl completed: {} pages", state.completed);
+                // `completed` counts every URL the crawl finished with, and
+                // that now includes the ones it could not read. Report the
+                // pages the caller actually got, and name the rest rather than
+                // folding them into the same number.
+                let scraped = state.completed.saturating_sub(state.blocked);
+                if state.blocked > 0 {
+                    eprintln!(
+                        "Crawl completed: {scraped} pages ({} unreadable)",
+                        state.blocked
+                    );
+                } else {
+                    eprintln!("Crawl completed: {scraped} pages");
+                }
                 break;
             }
             CrawlStatus::Failed => {

crates/crw-core/src/types.rs

@@ -477,7 +477,7 @@ fn default_true() -> bool {
 }
 
 /// Metadata about a scraped page.
-#[derive(Debug, Clone, Serialize, Deserialize)]
+#[derive(Debug, Clone, Serialize, Deserialize, Default)]
 #[serde(rename_all = "camelCase")]
 pub struct PageMetadata {
     pub title: Option<String>,
@@ -652,7 +652,7 @@ pub struct ScrapedImage {
 }
 
 /// Data returned for a single scraped page.
-#[derive(Debug, Clone, Serialize, Deserialize)]
+#[derive(Debug, Clone, Serialize, Deserialize, Default)]
 #[serde(rename_all = "camelCase")]
 pub struct ScrapeData {
     #[serde(skip_serializing_if = "Option::is_none")]
@@ -981,10 +981,13 @@ pub struct BlockOutcome {
 /// See `message()` for why the customer-facing wording splits here.
 pub const STRUCTURAL_FAILURE_VENDOR: &str = "structural_failure";
 
-/// `BlockOutcome::vendor` for "the origin answered with an error status and this
-/// is its error page". Not an anti-bot verdict, but the same consequence for the
-/// caller and for billing, and the crawl/batch surfaces have nowhere else to say
-/// it: they return an array of documents, not an envelope with an error code.
+/// `BlockOutcome::vendor` for an HTTP-level failure to get the page: the origin
+/// answered with an error status and this is its error page, its CDN answered
+/// that it could not reach the origin, or, on the crawl path, the request did
+/// not complete at all and `metadata.statusCode` is `0`. Not an anti-bot
+/// verdict, but the same consequence for the caller and for billing, and the
+/// crawl/batch surfaces have nowhere else to say it: they return an array of
+/// documents, not an envelope with an error code.
 pub const HTTP_ERROR_VENDOR: &str = "http_error";
 
 /// `BlockOutcome::vendor` for a registrar parking page, a domain-marketplace listing
@@ -1147,6 +1150,13 @@ pub struct CrawlRequest {
     /// `sticky_per_host`). `None` = server default (`sticky_per_host`).
     #[serde(default, alias = "proxy_rotation")]
     pub proxy_rotation: Option<crate::proxy::ProxyRotation>,
+    /// Extra request headers applied to every page this crawl fetches, with the
+    /// same semantics as [`ScrapeRequest::headers`], including its warning: on
+    /// a browser render `Network.setExtraHTTPHeaders` decorates every request
+    /// the page makes, subresources included, so cross-origin-sensitive
+    /// credentials do not belong here.
+    #[serde(default)]
+    pub headers: HashMap<String, String>,
 }
 
 /// Resolve the effective `render_js` decision from a per-request value and the
@@ -3281,6 +3291,23 @@ mod tests {
         assert!(req.max_pages.is_none());
     }
 
+    #[test]
+    fn crawl_request_headers_round_trip_and_default_empty() {
+        let req: CrawlRequest = serde_json::from_value(serde_json::json!({
+            "url": "https://example.com",
+            "headers": { "X-Custom": "1", "User-Agent": "test" }
+        }))
+        .unwrap();
+        assert_eq!(req.headers.get("X-Custom"), Some(&"1".to_string()));
+        assert_eq!(req.headers.get("User-Agent"), Some(&"test".to_string()));
+
+        // Absent `headers` must stay an empty map, not a deserialize error, so
+        // every crawl body written before this field existed still parses.
+        let bare: CrawlRequest =
+            serde_json::from_value(serde_json::json!({ "url": "https://example.com" })).unwrap();
+        assert!(bare.headers.is_empty());
+    }
+
     #[test]
     fn crawl_request_requires_url_field() {
         let result: Result<CrawlRequest, _> = serde_json::from_value(serde_json::json!({}));

crates/crw-crawl/src/crawl.rs

@@ -90,9 +90,14 @@ fn enqueue_discovered_links(
             if !is_safe_url(&link_url) {
                 continue;
             }
-            let link_host = link_url.host_str().unwrap_or("");
-            let link_origin = format!("{}://{}", link_url.scheme(), link_host);
-            if link_origin != origin {
+            // Full origin, port included. `map` next door already compares
+            // this way (`discover_urls`). Building it as scheme + host dropped
+            // the port, so a seed on `https://example.com/` treated
+            // `https://example.com:8443/` as the same site and crawled it. That
+            // was merely over-broad while the crawl sent no headers of its own;
+            // with `CrawlRequest::headers` it would replay the caller's
+            // credentials to a different service on the same host.
+            if link_url.origin().ascii_serialization() != origin {
                 continue;
             }
             let normalized = normalize_url(&link);
@@ -104,6 +109,53 @@ fn enqueue_discovered_links(
     }
 }
 
+/// Build the placeholder document a crawl returns for a URL it could not read.
+///
+/// Carries only the URL, the status (0 when there was no response at all) and
+/// the reason, stamped through the same `block` field the scrape and batch
+/// paths already use, so every surface that already understands "this document
+/// is not a page you asked for" understands this one too, and the caller's
+/// `completed - blocked` billing keeps it free.
+fn failed_page(url: &str, status_code: u16, reason: String) -> ScrapeData {
+    ScrapeData {
+        metadata: crw_core::types::PageMetadata {
+            source_url: url.to_string(),
+            status_code,
+            ..Default::default()
+        },
+        block: Some(crw_core::types::BlockOutcome {
+            vendor: crw_core::types::HTTP_ERROR_VENDOR.to_string(),
+            reason,
+        }),
+        ..Default::default()
+    }
+}
+
+/// Record a failed page and publish progress, mirroring the success path's
+/// bookkeeping so `completed`, `blocked` and `total` never disagree.
+fn push_failed_page(
+    data: ScrapeData,
+    id: Uuid,
+    state_tx: &tokio::sync::watch::Sender<CrawlState>,
+    results: &mut Vec<ScrapeData>,
+    blocked: &mut u32,
+    total: u32,
+) {
+    results.push(data);
+    *blocked += 1;
+    // Progress carries no data: the full array ships once, in Completed.
+    let _ = state_tx.send(CrawlState {
+        id,
+        success: true,
+        status: CrawlStatus::InProgress,
+        total,
+        completed: results.len() as u32,
+        blocked: *blocked,
+        data: Vec::new(),
+        error: None,
+    });
+}
+
 /// Run a BFS crawl starting from a URL.
 pub async fn run_crawl(opts: CrawlOptions<'_>) {
     // Propagate crawl-level country to every page-fetch through the renderer
@@ -176,13 +228,11 @@ async fn run_crawl_inner(opts: CrawlOptions<'_>) {
         }
     };
 
-    let origin = match base_url.host_str() {
-        Some(host) => format!("{}://{}", base_url.scheme(), host),
-        None => {
-            send_failed(id, &state_tx, "URL has no host".into());
-            return;
-        }
-    };
+    if base_url.host_str().is_none() {
+        send_failed(id, &state_tx, "URL has no host".into());
+        return;
+    }
+    let origin = base_url.origin().ascii_serialization();
 
     // Robots/sitemap egress must match the page egress: prefer the per-crawl
     // BYOP pool, then the config rotator, then the legacy single `proxy`. Never
@@ -285,11 +335,9 @@ async fn run_crawl_inner(opts: CrawlOptions<'_>) {
             }
             None => renderer.pick_proxy_for_url(&url),
         };
-        let empty_headers: std::collections::HashMap<String, String> =
-            std::collections::HashMap::new();
         let fetch_fut = renderer.fetch(
             &url,
-            &empty_headers,
+            &req.headers,
             effective_render_js,
             req.wait_for,
             pinned_renderer,
@@ -302,21 +350,43 @@ async fn run_crawl_inner(opts: CrawlOptions<'_>) {
             Ok(r) => r,
             Err(e) => {
                 tracing::warn!(url, error = %e, "Crawl: failed to fetch page");
+                push_failed_page(
+                    failed_page(&url, 0, e.to_string()),
+                    id,
+                    &state_tx,
+                    &mut results,
+                    &mut blocked,
+                    visited.len() as u32,
+                );
                 continue;
             }
         };
 
         // The CDN answered for a dead origin, so this page has no content and no
-        // links worth following — its body is the CDN's error page. Skipped like
-        // any other failed fetch (the `continue` above) rather than counted as a
-        // crawled page: a crawl of a site whose origin is down was reporting
-        // `completed: 1` with Cloudflare's apology as the page.
+        // links worth following: its body is the CDN's error page. It is
+        // recorded as a blocked page rather than a crawled one: a crawl of a
+        // site whose origin is down was reporting `completed: 1` with
+        // Cloudflare's apology as the page. `blocked` keeps it out of the bill
+        // (the caller charges `completed - blocked`) while the caller can still
+        // see which URL failed and why.
         if crate::single::is_cdn_origin_error(fetch_result.status_code) {
             tracing::warn!(
                 url,
                 status = fetch_result.status_code,
                 "Crawl: CDN could not reach the origin"
             );
+            push_failed_page(
+                failed_page(
+                    &url,
+                    fetch_result.status_code,
+                    "CDN could not reach origin".to_string(),
+                ),
+                id,
+                &state_tx,
+                &mut results,
+                &mut blocked,
+                visited.len() as u32,
+            );
             continue;
         }
 
@@ -355,6 +425,18 @@ async fn run_crawl_inner(opts: CrawlOptions<'_>) {
                 Ok(data) => data,
                 Err(err) => {
                     tracing::warn!(url, error = %err, "Crawl: PDF conversion failed");
+                    push_failed_page(
+                        failed_page(
+                            &url,
+                            fetch_result.status_code,
+                            format!("PDF conversion failed: {err}"),
+                        ),
+                        id,
+                        &state_tx,
+                        &mut results,
+                        &mut blocked,
+                        visited.len() as u32,
+                    );
                     continue;
                 }
             }
@@ -393,6 +475,18 @@ async fn run_crawl_inner(opts: CrawlOptions<'_>) {
                 Ok(data) => data,
                 Err(err) => {
                     tracing::warn!(url, error = %err, "Crawl: extraction failed");
+                    push_failed_page(
+                        failed_page(
+                            &url,
+                            fetch_result.status_code,
+                            format!("extraction failed: {err}"),
+                        ),
+                        id,
+                        &state_tx,
+                        &mut results,
+                        &mut blocked,
+                        visited.len() as u32,
+                    );
                     continue;
                 }
             }
@@ -1131,6 +1225,40 @@ mod tests {
         );
     }
 
+    /// A page the crawl could not read must come back marked, not silently
+    /// dropped and not billable: `block` is what every surface (v1, v2, the
+    /// SaaS biller's `completed - blocked`) already reads to tell a real page
+    /// from a refused one.
+    #[test]
+    fn failed_page_is_marked_blocked_and_carries_the_reason() {
+        let d = failed_page("https://example.com/dead", 0, "connect timeout".into());
+        assert_eq!(d.metadata.source_url, "https://example.com/dead");
+        // No response at all, so there is no status to report.
+        assert_eq!(d.metadata.status_code, 0);
+        assert!(d.markdown.is_none() && d.html.is_none());
+        let block = d.block.expect("a failed page must be marked");
+        assert_eq!(block.vendor, crw_core::types::HTTP_ERROR_VENDOR);
+        assert_eq!(block.reason, "connect timeout");
+        // Not priced by the engine; the caller excludes blocked pages anyway.
+        assert_eq!(d.credit_cost, 0);
+    }
+
+    /// The CDN-origin-error case keeps the status it answered with, so a caller
+    /// can tell a 523 apart from a transport failure.
+    #[test]
+    fn failed_page_keeps_an_upstream_status_code() {
+        let d = failed_page(
+            "https://example.com/x",
+            523,
+            "CDN could not reach origin".into(),
+        );
+        assert_eq!(d.metadata.status_code, 523);
+        assert_eq!(
+            d.block.map(|b| b.reason).as_deref(),
+            Some("CDN could not reach origin")
+        );
+    }
+
     #[test]
     fn normalize_url_lowercase() {
         assert_eq!(
@@ -1767,33 +1895,45 @@ mod tests {
         assert_eq!(queue[0].0, "https://example.com/real-page");
     }
 
+    /// A different port is a different origin, matching `discover_urls`'s BFS
+    /// phase. This used to be pinned the other way and labelled a known bug:
+    /// harmless while the crawl sent no caller headers, a credential-scope hole
+    /// the moment `CrawlRequest::headers` existed, since an admin console on
+    /// `:9999` would receive the seed's `Authorization`.
     #[test]
-    fn enqueue_same_host_different_port_treated_as_same_origin() {
-        // BUG: `enqueue_discovered_links` (and the `origin` it's called
-        // with, built in `run_crawl_inner` as `scheme://host` with no port)
-        // ignores the port entirely when deciding same-origin. A link to a
-        // different port on the same host is followed as if it were the
-        // same site — unlike `discover_urls`'s BFS phase, which explicitly
-        // uses the full `Url::origin()` (scheme+host+port) and documents
-        // why dropping the port would be wrong. Documenting current
-        // behavior; production code left untouched per the test rules.
+    fn enqueue_same_host_different_port_is_a_different_origin() {
         let html = links_html(&["https://example.com:9999/admin"]);
         let mut visited = HashSet::new();
         let mut queue = VecDeque::new();
         enqueue_discovered_links(
             &html,
             "https://example.com/",
-            "https://example.com", // port-less origin, as run_crawl_inner builds it
+            "https://example.com",
             100,
             &mut visited,
             &mut queue,
             0,
         );
-        assert_eq!(
-            queue.len(),
-            1,
-            "a different port on the same host is currently treated as same-origin"
+        assert!(queue.is_empty(), "a different port must not be followed");
+    }
+
+    /// The default port is not a different origin: `Url::origin()` omits it, so
+    /// an explicit `:443` link from a plain `https://` seed still gets crawled.
+    #[test]
+    fn enqueue_explicit_default_port_is_still_same_origin() {
+        let html = links_html(&["https://example.com:443/page"]);
+        let mut visited = HashSet::new();
+        let mut queue = VecDeque::new();
+        enqueue_discovered_links(
+            &html,
+            "https://example.com/",
+            "https://example.com",
+            100,
+            &mut visited,
+            &mut queue,
+            0,
         );
+        assert_eq!(queue.len(), 1);
     }
 
     #[test]