fix: improve agent request error handling with structured details - #226
Merged
mintybasil merged 1 commit intoJun 29, 2026
Merged
Conversation
Replace the opaque HarnessError::Http(#[from] reqwest::Error) variant with a structured variant that captures the URL, timeout/connect status, and full cause chain. The previous error message was just 'HTTP request failed: error sending request for url (...)' with no indication of *why* the request failed. The new from_reqwest_error() helper walks the reqwest::Error source chain to surface the root cause (DNS failure, connection refused, timeout, etc.) and includes timeout/connect indicators. Closes mintybasil#225
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Closes #225
The error message reported in issue #225 was:
This error message is lacking details — there is no indication of why the request failed (timeout, connection refused, DNS error, etc.) and no status code.
Root cause
HarnessError::Httpused#[from] reqwest::Error, which delegates toreqwest::Error's defaultDisplayimpl. That impl only produceserror sending request for url (...)and does not include the underlying cause (timeout, connection refused, DNS failure) or a status code if one was received.Additionally, when
response.text().await?failed on the body read, the already-captured HTTP status code was silently lost.Changes
src/harness.rsReplaced
HarnessError::Http(#[from] reqwest::Error)with a structured variant:Added
HarnessError::from_reqwest_error(err, url)— a helper that builds a descriptiveHttperror by:is_timeout()) astimeout reachedis_connect()) asconnection failederr.status())std::error::Error::source()cause chain to get the real root cause (e.g.dns error: failed to lookup address information,Connection refused (os error 111))Updated
execute_step_with_error_pathto use.map_err(|e| HarnessError::from_reqwest_error(e, &url))?instead of the bare?operator on both the request send and the body read, so both failure paths produce a structured, descriptive error.Before (issue #225):
After (timeout example):
After (connection refused example):
After (DNS failure example):
Test plan
cargo build— compiles cleanlycargo test— all 120+ tests pass (lib + integration)cargo clippy --lib -- -W clippy::all— no warningsHarnessError::Httpvariant:test_harness_error_http_includes_url— verifies URL, timeout indicator, and cause chain appear in the displaytest_harness_error_http_connection_refused— verifies connection-failure indicator and causetest_harness_error_http_minimal— verifies the message is still meaningful with no extra detailstest_harness_http_error_includes_details(regression test for issue Improve agent request error handling #225)