Skip to content

fix(web_fetch): return HTTP 4xx/5xx responses as error results - #47

Open
senamakel wants to merge 2 commits into
tinyhumansai:mainfrom
senamakel:tinytools-web-fetch-http-errors
Open

senamakel wants to merge 2 commits into
tinyhumansai:mainfrom
senamakel:tinytools-web-fetch-http-errors

Conversation

@senamakel

Copy link
Copy Markdown
Member

Summary

web_fetch returned every HTTP response as a successful ToolResult (status=403 url=...), including 4xx/5xx. A host that budgets, retries or halts on is_error (OpenHuman's research budget, openhuman#6959 follow-up) therefore counted blocked or rate-limited fetches as completed research.

Redirects are not followed (Policy::none()), so the "final response" is the first response; 3xx handling is unchanged and still a successful "redirect not followed" report.

Changes

  • 4xx/5xx responses now produce ToolResult::error with model-useful text:
    • HTTP 403 Forbidden from <host>; the site refused the request. Try another source.
    • HTTP 429 Too Many Requests from <host>; the site is rate limiting requests. Retry-After: <v>. Try another source, or retry later. (Retry-After only when the header is present)
    • 404/410: page does not exist; 5xx: server failed, retry later or try another source; other 4xx: request rejected.
    • A short (300 char), single-line, text-only body excerpt on a following Response excerpt: line. HTML goes through the host HtmlExtractor unless raw: true.
  • Extracted the post-validation half of execute into a private fetch_validated so tests can drive a loopback server (the SSRF guard refuses loopback in execute; same approach as http_request_tests).
  • Debug log [tool.web_fetch] http error status=... host=... retry_after_present=... (no body, no URL path or query).

Tests (TDD: red first)

New in web_fetch_tests.rs: 200 stays OK with status= header; 403, 404, 5xx are errors; 429 is an error with rate-limit wording and Retry-After (and without it when the header is absent); excerpt is bounded and stripped of markup; 301 stays successful.

  • cargo test --workspace: all pass (tinytools-std 500 passed)
  • cargo clippy --workspace --all-targets -- -D warnings and cargo fmt --all --check: clean

Compatibility note

Callers that matched on the status=4xx/5xx success preamble will now see an error result instead. Hosts that classify failures from the error text (e.g. keyword matching on "403"/"Forbidden") will now see these where they previously did not.

senamakel and others added 2 commits October 3, 2026 20:01
Moved the HTTP GET and response rendering logic from the `WebFetchTool` implementation into a new private method `fetch_validated` on `WebFetchTool`. This separates the SSRF guard and gate disclosure from the actual fetch, making the code easier to test and reuse. Also changed the `url` parameter from `&str` to `&str` to avoid an unnecessary borrow.

Auto-committed-on: dragonfly
Co-authored-by: Medulla <medulla@tinyhumans.ai>
…cesses

HTTP error status codes (4xx and 5xx) are now returned as tool errors rather than successful page reports, so that rate-limited, blocked, or unavailable pages are properly surfaced to the agent for retry or alternative sourcing. The error message includes the status code, host, a short body excerpt, and a Retry-After hint when present, while 3xx redirects continue to be reported as successful but unfollowed responses.

Auto-committed-on: dragonfly
Co-authored-by: Medulla <medulla@tinyhumans.ai>
@tinysweeper

tinysweeper Bot commented Oct 3, 2026 •

Copy link
Copy Markdown

Tiny Sweeper review

Tiny Sweeper reviewed this change across 6 lane(s) and found 0 active actionable finding(s). Detailed lane evidence and any incomplete work are listed below.

State: Incomplete
Priority: none
Reviewed head: a6b188b8b72b
Updated: 1791047093 (Unix time)

Review snapshot

Change surface Files Review signal Count
Production 1 Active findings 0
Tests 1 Noted findings 0
Documentation 0 Resolved findings 0
Configuration 0 Pending checks/questions 4

Completeness: Incomplete
Test assessment: No supported feature-to-test mapping was available; this does not mean tests are absent or passed.

What changed

The review could not produce a supported behavioral summary; inspect the cited changed surface and lane details below.

Features

None identified with supported citations.

Tests

No supported feature-to-test mapping was produced. Test execution is not inferred.

Findings

No active actionable findings.

Could not review: crates/tinytools-std/src/network/web_fetch.rs, crates/tinytools-std/src/network/web_fetch_tests.rs

Before merge

  • Complete the critique review for crates/tinytools-std/src/network/web_fetch.rs, crates/tinytools-std/src/network/web_fetch_tests.rs.
  • Complete the security review for crates/tinytools-std/src/network/web_fetch.rs, crates/tinytools-std/src/network/web_fetch_tests.rs.

How this fits together

flowchart LR
  n0["WebFetchTool<br/>changed"]:::changed
  n1["execute_blocks_when_rate_limited<br/>changed"]:::changed
  n2["fetch"]:::impacted
  n3["test_security"]:::impacted
  n4["Tool"]:::impacted
  n5["NetGate"]:::impacted
  n6["..._the_schema_into_extra_optional_arguments"]:::impacted
  n7["HtmlExtractor"]:::impacted
  n0 -->|implements| n4
  n0 -->|uses| n5
  n0 -->|uses| n7
  n1 -->|calls| n2
  n1 -->|tests| n2
  n2 -->|uses| n0
  n6 -->|calls| n2
  n6 -->|tests| n2
  n6 -->|calls| n3
  n6 -->|tests| n3
  classDef changed fill:#0d4429,stroke:#238636,color:#e6edf3
  classDef impacted fill:#161b22,stroke:#6e7681,color:#c9d1d9
  classDef flagged fill:#5a1e02,stroke:#d93f0b,color:#ffffff
  classDef blocking fill:#67060c,stroke:#f85149,color:#ffffff
Loading
Agent review details

critique

  • Conclusion: Neutral
  • Scope reviewed: incomplete; unanswered: crates/tinytools-std/src/network/web_fetch.rs, crates/tinytools-std/src/network/web_fetch_tests.rs
  • Lane summary: Reviewed 0 files; 0 findings. 2 files could not be reviewed: crates/tinytools-std/src/network/web_fetch.rs, crates/tinytools-std/src/network/web_fetch_tests.rs.

security

  • Conclusion: Neutral
  • Scope reviewed: incomplete; unanswered: crates/tinytools-std/src/network/web_fetch.rs, crates/tinytools-std/src/network/web_fetch_tests.rs
  • Lane summary: Reviewed 0 files; 0 findings. 2 files could not be reviewed: crates/tinytools-std/src/network/web_fetch.rs, crates/tinytools-std/src/network/web_fetch_tests.rs.

tests

  • Conclusion: Success
  • Scope reviewed: all assigned evidence
  • Lane summary: Extracts `fetch_validated` from the public `invoke` method and adds a new 4xx/5xx error path that returns a `ToolResult::error` with a descriptive message and a short body excerpt, plus tests covering the major status codes. The tests exercise the new error path directly, including excerpt length limiting, HTML stripping, and the redirect-stays-successful invariant. Looks sound. _Code retrieval was unavailable (model: ladder embeddings returned 502 Bad Gateway: {"error":{"message":"no rung of ladder vectors could serve the request","skipped":[{"model":"text-embedding-bge-m3","provider":"venice","reason":"rate limited, retry in 23s","rung":0}],"type":"ladder_router_error"}}), so this review saw the diff alone._ _Memory was unavailable (model: cortex: v1/recall: timed out after 10s), so this review ran without it._

commits

  • Conclusion: Neutral
  • Scope reviewed: all assigned evidence
  • Lane summary: Nothing sensitive found in what this pull request commits.

description

  • Conclusion: Success
  • Scope reviewed: all assigned evidence
  • Lane summary: The change correctly turns 4xx/5xx responses into error results, adds `fetch_validated` for testability, and includes thorough tests. It follows the repository's coding rules and has no issues that would prevent merging. _Code retrieval was unavailable (model: ladder embeddings returned 502 Bad Gateway: {"error":{"message":"no rung of ladder vectors could serve the request","skipped":[{"model":"text-embedding-bge-m3","provider":"venice","reason":"rate limited, retry in 23s","rung":0}],"type":"ladder_router_error"}}), so this review saw the diff alone._ _Memory was unavailable (model: cortex: v1/recall: timed out after 10s), so this review ran without it._

e2e

  • Conclusion: Neutral
  • Scope reviewed: all assigned evidence
  • Lane summary: No end-to-end harness in this repository: no e2e test files and no e2e workflow.
Evidence and run details
  • Models: deepseek/deepseek-v4-flash
  • Spend: $0.001276
  • Tokens: 44944 input · 7290 output · 5289 cached · 0 embedding
Head State Pass summary
a6b188b8b72b incomplete 0 active finding(s), 0 resolved finding(s) (at 1791047093)

tinysweeper 0.1.0

@coderabbitai

coderabbitai Bot commented Oct 3, 2026

Copy link
Copy Markdown

Warning

Review limit reached

  • Run on-demand review

This review includes 2 billable files and costs up to $0.50.

Or wait 46 minutes for your next included review.

Check out review usage here.

View limit details

Limit details: You’ve used the included review currently available.

Learn how review limits work.

Review configuration:

⚙️ Run configuration
  • Configuration used: Organization UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: d1ac6368-b597-4434-bbae-11edf30f4983
📥 Commits

Reviewing files that changed from the base of the PR and between 1c58197 and a6b188b.

📒 Files selected for processing (2)
  • crates/tinytools-std/src/network/web_fetch.rs
  • crates/tinytools-std/src/network/web_fetch_tests.rs
  • Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Comment @coderabbitai help to get the list of available commands.

@tinysweeper tinysweeper Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

tinysweeper found nothing blocking, but could not review everything, so this is not an approval: crates/tinytools-std/src/network/web_fetch.rs, crates/tinytools-std/src/network/web_fetch_tests.rs.

             $0.0013 · 44,944 in / 7,290 out · 5,289 cached (12%) · deepseek/deepseek-v4-flash
tests:       $0.0008 · 25,783 in / 738 out   · 0 cached (0%)      · deepseek/deepseek-v4-flash
description: $0.0002 · 7,404 in  / 61 out    · 0 cached (0%)      · deepseek/deepseek-v4-flash

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant