Skip to content

batch scrape drops a URL from the results array when its scrape errors #487

Description

@us

What happens

When one URL in a batch scrape fails, it disappears from the job's results array entirely. The completed counter still advances for it, so a caller comparing completed against data.length sees a mismatch with nothing explaining it, and there is no per-URL error anywhere in the batch response to say which URL fell out or why.

The crawl surface does not behave this way. It records a placeholder for the failed page with the reason attached and counts it in blocked, so the caller can see exactly which URL failed and read the error.

Where

crates/crw-server/src/state.rs, in the batch loop inside start_batch_job:

let scraped = scrape_url(...).await.ok();   // Err becomes None here
tx.send_modify(|st| {
    if let Some(mut d) = scraped {
        if d.block.is_some() { st.blocked += 1; }
        st.data.push(d);
    }
    st.completed += 1;                       // advances either way
    ...
});

The .ok() discards the error, so on the Err path nothing is pushed and blocked is not incremented, while completed is.

Why it matters beyond the missing entry

completed is what the caller's usage is derived from, and blocked is what excludes a page from it. A URL that errors advances the first without the second, so it is accounted for as if it had produced a document. Getting the placeholder into the results array fixes the reporting and the accounting in the same move, because the placeholder carries block.

This is pre-existing and is already noted in a comment in crates/crw-server/src/routes/v2/adapters.rs.

Suggested fix

The crawl path already has the shape:

  • crates/crw-crawl/src/crawl.rs failed_page(url, status_code, reason) builds a ScrapeData with block: Some(BlockOutcome { vendor: HTTP_ERROR_VENDOR, reason }).
  • push_failed_page(...) pushes it, increments blocked, and publishes the new state.

Both are private. Exporting failed_page and matching on the scrape_url result instead of calling .ok() on it would bring batch in line with crawl.

The part that needs care

The job-completion gate keys off completed, so anything that changes what completed counts has to be checked against the gate, not just against the counter. That is why this is its own issue rather than a one-line change: it wants a test that a batch containing a failing URL still reports completed and still terminates.

Reproducing

Any batch containing a URL the engine cannot turn into a document works. The simplest one today is a URL whose origin serves a body that is not a page, for example a .docx or an .xls, which now returns unsupported_content_type (#485). The job finishes with data.length one short of completed and no indication of which URL is missing.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions