fix(batch): retain failed scrape results

crates/crw-core/src/types.rs

@@ -963,6 +963,25 @@ impl ScrapeData {
         self.summary = None;
         self.chunks = None;
     }
+
+    /// Whether any page-content field survived. A document without a body is a
+    /// placeholder for a URL that produced no page (a cleared wall, or a scrape
+    /// error), as opposed to an origin error page that was kept readable.
+    /// `source_hash` and `basis` are excluded on purpose: both derive from the
+    /// fields below and never stand on their own. `screenshot` is not counted
+    /// either, so a wall captured with a screenshot still reads as no page
+    /// while the caller keeps the image.
+    pub fn has_body(&self) -> bool {
+        self.markdown.is_some()
+            || self.html.is_some()
+            || self.raw_html.is_some()
+            || self.plain_text.is_some()
+            || self.links.is_some()
+            || self.images.is_some()
+            || self.json.is_some()
+            || self.summary.is_some()
+            || self.chunks.is_some()
+    }
 }
 
 /// Typed anti-bot block verdict. `vendor` is the antibot `class_name`
@@ -1452,6 +1471,20 @@ mod tests {
         assert!(d.http_error().is_some());
     }
 
+    #[test]
+    fn has_body_separates_a_placeholder_from_a_readable_error_page() {
+        let mut d = ScrapeData::default();
+        assert!(!d.has_body(), "a failed_page or cleared wall has no body");
+        d.markdown = Some("404 Not Found".into());
+        assert!(
+            d.has_body(),
+            "an origin error page kept readable has a body"
+        );
+        d.markdown = None;
+        d.links = Some(vec!["https://example.com/a".into()]);
+        assert!(d.has_body(), "a links-only format still delivered a page");
+    }
+
     #[test]
     fn clear_body_drops_content_keeps_metadata_and_block() {
         let mut data = ScrapeData {

crates/crw-crawl/src/crawl.rs

@@ -109,14 +109,14 @@ fn enqueue_discovered_links(
     }
 }
 
-/// Build the placeholder document a crawl returns for a URL it could not read.
+/// Build the placeholder document a crawl or batch 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 {
+pub fn failed_page(url: &str, status_code: u16, reason: String) -> ScrapeData {
     ScrapeData {
         metadata: crw_core::types::PageMetadata {
             source_url: url.to_string(),

crates/crw-crawl/src/single.rs

@@ -106,6 +106,27 @@ pub async fn scrape_url(
     })
 }
 
+/// Reject the faults in a scrape template that no fetch can repair, before any
+/// network work. Shared by the single scrape and the batch route, so a bad
+/// template is one 400 on both surfaces rather than one placeholder document
+/// per URL labelled as a block.
+pub fn validate_scrape_template(req: &ScrapeRequest) -> CrwResult<()> {
+    if req.actions.is_some() {
+        return Err(crw_core::error::CrwError::InvalidRequest(
+            "The 'actions' parameter is not yet supported. Use cssSelector or xpath for element targeting.".into()
+        ));
+    }
+    // A screenshot is captured via CDP and cannot be produced on the HTTP-only
+    // path. An explicit `renderJs:false` + `screenshot` is contradictory: reject
+    // it rather than silently return a null screenshot.
+    if req.formats.contains(&OutputFormat::Screenshot) && req.render_js == Some(false) {
+        return Err(crw_core::error::CrwError::InvalidRequest(
+            "screenshot format requires JS rendering; remove renderJs:false (or omit it)".into(),
+        ));
+    }
+    Ok(())
+}
+
 #[allow(clippy::too_many_arguments)]
 async fn scrape_url_inner(
     req: &ScrapeRequest,
@@ -117,12 +138,7 @@ async fn scrape_url_inner(
     render_js_default: Option<bool>,
     deadline: Deadline,
 ) -> CrwResult<ScrapeData> {
-    // Reject unsupported `actions` parameter early with a clear error.
-    if req.actions.is_some() {
-        return Err(crw_core::error::CrwError::InvalidRequest(
-            "The 'actions' parameter is not yet supported. Use cssSelector or xpath for element targeting.".into()
-        ));
-    }
+    validate_scrape_template(req)?;
 
     // Determine whether stealth headers should be injected for this request.
     let inject_stealth = req.stealth.unwrap_or(default_stealth);
@@ -142,17 +158,10 @@ async fn scrape_url_inner(
     // render_js_default=true and a per-request proxy still reaches the JS renderer.
     let effective_render_js = resolve_render_js(effective_render_js_request, render_js_default);
 
-    // A screenshot is captured via CDP and cannot be produced on the HTTP-only
-    // path. An explicit `renderJs:false` + `screenshot` is contradictory — reject
-    // it rather than silently return a null screenshot. For the default/auto case
-    // the renderer forces the CDP path (see FallbackRenderer::fetch), and the
-    // temp HTTP fetcher below is skipped so the screenshot is never dropped.
+    // For the default/auto case the renderer forces the CDP path for a
+    // screenshot (see FallbackRenderer::fetch), and the temp HTTP fetcher below
+    // is skipped so the screenshot is never dropped.
     let wants_screenshot = req.formats.contains(&OutputFormat::Screenshot);
-    if wants_screenshot && req.render_js == Some(false) {
-        return Err(crw_core::error::CrwError::InvalidRequest(
-            "screenshot format requires JS rendering; remove renderJs:false (or omit it)".into(),
-        ));
-    }
 
     // Validate pinned renderer is available — fail fast with a 400 instead of
     // letting the request reach the dispatcher with a hard-pin to a missing pool.

crates/crw-server/src/routes/batch.rs

@@ -102,6 +102,10 @@ pub async fn start_batch(
     // Reject an unavailable pinned renderer up front (as /v1/scrape and /v1/crawl
     // do) instead of failing every URL individually deep in the pipeline.
     crate::state::validate_renderer_pin(template.renderer, template.render_js, &state)?;
+    // Same rejections `/v1/scrape` applies at the top of the scrape. Left to
+    // the per-URL path they would come back as N placeholder documents each
+    // labelled as an anti-bot block, for a fault in the caller's own request.
+    crw_crawl::single::validate_scrape_template(&template)?;
     template.url = String::new();
 
     // Partition URLs into valid / invalid (SSRF-checked, same guard as
@@ -219,6 +223,38 @@ mod tests {
         }
     }
 
+    #[tokio::test]
+    async fn start_batch_rejects_actions_before_any_url_work() {
+        let state = default_state();
+        let err = call(
+            &state,
+            json!({ "urls": ["https://example.com/"], "actions": [] }),
+        )
+        .await
+        .unwrap_err();
+        assert!(
+            invalid_request_message(&err).contains("'actions'"),
+            "got: {}",
+            invalid_request_message(&err)
+        );
+    }
+
+    #[tokio::test]
+    async fn start_batch_rejects_screenshot_without_js_before_any_url_work() {
+        let state = default_state();
+        let err = call(
+            &state,
+            json!({ "urls": ["https://example.com/"], "formats": ["screenshot"], "renderJs": false }),
+        )
+        .await
+        .unwrap_err();
+        assert!(
+            invalid_request_message(&err).contains("screenshot"),
+            "got: {}",
+            invalid_request_message(&err)
+        );
+    }
+
     #[tokio::test]
     async fn start_batch_requires_urls_field() {
         let state = default_state();

crates/crw-server/src/routes/v2/adapters.rs

@@ -123,7 +123,13 @@ pub fn to_v2_document(data: ScrapeData, proxy_used: &str, scrape_id: String) ->
         summary: data.summary,
         change_tracking: data.change_tracking,
         screenshot: data.screenshot,
-        warning: data.warning,
+        // `V2Document` has no `block`, so a URL retained only as a placeholder
+        // would otherwise reach a /v2 caller as an empty document with nothing
+        // to explain it. `warning` is part of the frozen shape and is exactly
+        // where Firecrawl surfaces a per-document problem.
+        warning: data
+            .warning
+            .or_else(|| data.block.as_ref().map(|b| b.reason.clone())),
         metadata,
     }
 }
@@ -221,12 +227,12 @@ pub fn build_crawl_status(
         None
     };
 
-    // A blocked page is not billed, so it must not be counted here either.
-    // Note this is a lower bound rather than the exact charge on the batch path:
-    // `completed` also advances for a URL whose scrape returned `Err` and pushed
-    // no document, and the SaaS bills `completed - blocked`. Pre-existing; fixing
-    // it means changing what `completed` counts, which the job-completion gate
-    // depends on.
+    // A blocked page is not billed, so it must not be counted here either. A
+    // batch URL whose scrape returns `Err` now pushes a placeholder carrying
+    // `block` and bumps `blocked`, so it is excluded here for the same reason a
+    // wall is, and `completed - blocked` no longer over-counts it. The two are
+    // still not identical: this sum prices a multi-page PDF per page, while
+    // `completed - blocked` counts documents.
     let credits_used: u32 = state
         .data
         .iter()

crates/crw-server/src/routes/v2/batch.rs

@@ -90,6 +90,10 @@ pub async fn start_batch(
         .map_err(|e| CrwError::InvalidRequest(format!("invalid batch scrape options: {e}")))?;
     let (mut template, _decomposed, _tier) = to_internal(template_v2)?;
     template.url = String::new();
+    // Same upfront rejections as /v1/batch/scrape: a fault in the caller's
+    // own template is one 400 here, not one placeholder document per URL.
+    crate::state::validate_renderer_pin(template.renderer, template.render_js, &state)?;
+    crw_crawl::single::validate_scrape_template(&template)?;
 
     // Partition URLs into valid / invalid (SSRF-checked, same as v1 scrape).
     // Validation resolves DNS per URL — run it with bounded concurrency,
@@ -174,3 +178,36 @@ pub async fn cancel_batch(state: State<AppState>, id: Path<Uuid>) -> Result<Json
 pub async fn get_errors(state: State<AppState>, id: Path<Uuid>) -> Result<Json<Value>, AppError> {
     super::crawl::get_errors(state, id).await
 }
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+    use crate::state::AppState;
+    use crw_core::config::AppConfig;
+    use serde_json::json;
+
+    async fn call(body: Value) -> Result<V2BatchStartResponse, AppError> {
+        let config: AppConfig = toml::from_str("").unwrap();
+        let state = AppState::new(config).unwrap();
+        start_batch(State(state), HeaderMap::new(), Ok(Json(body)))
+            .await
+            .map(|Json(r)| r)
+    }
+
+    /// A template fault is rejected before any URL work on the Firecrawl
+    /// surface too, instead of surfacing as one placeholder per URL.
+    #[tokio::test]
+    async fn v2_start_batch_rejects_screenshot_without_js_before_any_url_work() {
+        let err = call(json!({
+            "urls": ["https://example.com/"],
+            "formats": ["screenshot"],
+            "renderJs": false
+        }))
+        .await
+        .unwrap_err();
+        match &err.0 {
+            CrwError::InvalidRequest(msg) => assert!(msg.contains("screenshot"), "got: {msg}"),
+            other => panic!("expected InvalidRequest, got {other:?}"),
+        }
+    }
+}

crates/crw-server/src/routes/v2/crawl.rs

@@ -260,11 +260,42 @@ pub async fn get_errors(
     let job = jobs
         .get(&id)
         .ok_or_else(|| CrwError::NotFound(format!("Crawl job {id} not found")))?;
-    let err = job.rx.borrow().error.clone();
-    let errors: Vec<Value> = err
-        .into_iter()
+    // Job-level failure first (the whole job died), then the per-URL ones. A URL
+    // the engine could not turn into a document is retained as a placeholder
+    // carrying `block` and no body, and `V2Document` has no field to show the
+    // block, so without this it would reach the caller as an empty document
+    // with nothing to explain it while this route, the one documented to carry
+    // these, said there were no errors at all. An origin error page that was
+    // kept readable is a delivered document, not an error: listing it here too
+    // would make a caller retry a URL it already holds.
+    //
+    // Each entry gets its own id, the way Firecrawl keys errors per scrape
+    // rather than per job, so a caller keying them into a map keeps them all.
+    // The id is the job id plus the document's position, so it is stable
+    // across polls and a caller deduplicating by id sees each failure once.
+    let guard = job.rx.borrow();
+    let mut errors: Vec<Value> = guard
+        .error
+        .iter()
         .map(|e| serde_json::json!({ "id": id.to_string(), "error": e }))
         .collect();
+    errors.extend(
+        guard
+            .data
+            .iter()
+            .enumerate()
+            .filter(|(_, d)| !d.has_body())
+            .filter_map(|(index, d)| {
+                d.block.as_ref().map(|b| {
+                    serde_json::json!({
+                        "id": format!("{id}-{index}"),
+                        "url": d.metadata.source_url,
+                        "error": b.reason,
+                    })
+                })
+            }),
+    );
+    drop(guard);
     Ok(Json(
         serde_json::json!({ "success": true, "errors": errors, "robotsBlocked": [] }),
     ))
@@ -274,6 +305,50 @@ pub async fn get_errors(
 mod tests {
     use super::*;
 
+    /// A batch URL the engine could not turn into a document is retained with a
+    /// `block`, but `V2Document` has no field that can show it, so on this
+    /// surface it is otherwise an empty document with nothing to explain it.
+    /// This route is the one documented to carry those failures, so it has to.
+    #[tokio::test]
+    async fn get_errors_reports_the_per_url_failures_of_a_batch() {
+        let config: crw_core::config::AppConfig = toml::from_str("").unwrap();
+        let state = AppState::new(config).unwrap();
+        // `actions` is rejected per URL inside the scrape, which is the shortest
+        // deterministic way to make one URL fail without touching the network.
+        let template = crw_core::types::ScrapeRequest {
+            actions: Some(serde_json::json!([])),
+            ..Default::default()
+        };
+        let url = "https://example.com/error";
+        let id = state
+            .start_batch_job(vec![url.to_string()], template, None)
+            .await;
+
+        let mut settled = false;
+        for _ in 0..100 {
+            tokio::task::yield_now().await;
+            let jobs = state.crawl_jobs.read().await;
+            if jobs.get(&id).unwrap().rx.borrow().status != CrawlStatus::InProgress {
+                settled = true;
+                break;
+            }
+        }
+        assert!(settled, "batch job did not settle");
+
+        let Ok(Json(body)) = get_errors(State(state.clone()), Path(id)).await else {
+            panic!("errors route should succeed for an existing job");
+        };
+        let errors = body["errors"].as_array().expect("errors array");
+        assert_eq!(errors.len(), 1, "got: {body}");
+        assert_eq!(errors[0]["url"], url, "got: {body}");
+        assert!(
+            errors[0]["error"]
+                .as_str()
+                .is_some_and(|e| e.contains("actions")),
+            "got: {body}"
+        );
+    }
+
     /// Regression for #346. `scrapeOptions` is parsed key-by-key out of a raw
     /// `Value`, so a key nobody reads is silently dropped; `renderJs` was such a
     /// key and `CrawlRequest.render_js` was hardcoded `None`. A v2 caller then