Skip to content
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 9 additions & 3 deletions src/harness/providers/openai/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -224,9 +224,15 @@ into a structured `ProviderError` (HTTP status, provider error code, and a
everything else — including 401/400 — is not) and surfaced as
`TinyAgentsError::Provider`, so `harness::retry::is_retryable` can classify
retryability instead of retrying every provider failure indiscriminately.
Transport-level failures (connection errors, body-read failures) have no such
structure to preserve and surface as a plain `TinyAgentsError::Model` string
via `provider_failure_message`. Malformed JSON bodies surface as
A **send** failure — the request never reached a server, so there is no status
— still carries a provider, a model, and a computed `retryable`, so
`send_checked` raises it as `TinyAgentsError::Provider` with `status: None`
rather than flattening it; a host that classifies on the variant would
Comment thread
M3gA-Mind marked this conversation as resolved.
Outdated
otherwise see a connection reset as an unstructured error. `Display` is
identical either way.

**Body-read** failures after a 2xx (`list_models`, `invoke_responses`) remain
plain `TinyAgentsError::Model` strings. Malformed JSON bodies surface as
`TinyAgentsError::Serialization`.

## Operational constraints
Expand Down
60 changes: 60 additions & 0 deletions src/harness/providers/openai/test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2915,3 +2915,63 @@ fn stream_cleanup_scrubs_leaked_markup_from_live_deltas() {
assert_eq!(completed.tool_calls().len(), 1);
assert_eq!(completed.tool_calls()[0].name, "lookup");
}

// ── transport-level failures keep their structure ───────────────────────────

/// A transport failure must reach the caller as `TinyAgentsError::Provider`.
///
/// It carries no HTTP status — nothing answered — but it does carry the
/// provider, the model, and a computed `retryable`, and a host that classifies
/// on the variant needs all three. Flattening it into
/// `TinyAgentsError::Model(String)` sends a connection reset down the generic
/// arm of a host's logger, where it is recorded with no status, no provider and
/// no retryability, and the operator reading it cannot tell a dead endpoint
/// from a rejected request.
#[tokio::test]
async fn a_transport_failure_is_reported_as_a_structured_provider_error() {
// Bind, read the port, then drop the listener: the port is now almost
// certainly closed, so the connect fails without waiting on a timeout.
let port = {
let listener = std::net::TcpListener::bind("127.0.0.1:0").expect("bind an ephemeral port");
listener.local_addr().expect("read the bound port").port()
};

let model = OpenAiModel::compatible_provider(
"OpenHuman",
"test-key",
format!("http://127.0.0.1:{port}/openai/v1"),
"chat-v1",
);

let error = ChatModel::<()>::invoke(&model, &(), ModelRequest::new(vec![Message::user("hi")]))
.await
.expect_err("a closed port cannot answer");

let TinyAgentsError::Provider(provider_error) = &error else {
panic!("expected a structured Provider error, got: {error:?}");
};
assert_eq!(provider_error.provider, "OpenHuman");
assert_eq!(provider_error.model.as_deref(), Some("chat-v1"));
assert!(
provider_error.status.is_none(),
"nothing answered, so there is no status to report"
);
assert!(
provider_error.retryable,
"a transport failure is worth retrying; the flag is what says so"
);

// The rendered text is the no-regression half: `Display for ProviderError`
// reproduces what `provider_failure_message` used to build by hand, and both
// error variants render as `model error: {0}`, so nothing a user or a log
// reader sees changes.
let rendered = error.to_string();
assert!(
rendered.starts_with("model error: OpenHuman returned: "),
"the user-facing wording must not change, got: {rendered}"
);
assert!(
rendered.contains("/openai/v1/chat/completions"),
"the failing URL is what makes this diagnosable, got: {rendered}"
);
}
33 changes: 13 additions & 20 deletions src/harness/providers/openai/transport.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1993,9 +1993,19 @@ impl OpenAiModel {
url: &str,
) -> Result<reqwest::Response> {
let response = builder.send().await.map_err(|e| {
let error =
self.provider_error(format!("{what} to {url} failed: {e}"), None, None, None);
TinyAgentsError::Model(self.provider_failure_message(&error))
// Structured, not flattened. A transport failure has no HTTP status,
// but it does have a provider, a model, and a computed `retryable` —
// and a host that matches on `TinyAgentsError::Provider` to log
// those fields would otherwise fall through to its generic arm and
// record a connection reset as "(non-provider error)" with nothing
// to act on. `Display` renders identically either way, so only the
// machine-readable half changes.
TinyAgentsError::from_provider_error(self.provider_error(
format!("{what} to {url} failed: {e}"),
None,
None,
None,
))
})?;

let status = response.status();
Expand Down Expand Up @@ -2133,23 +2143,6 @@ impl OpenAiModel {
}
}

fn provider_failure_message(&self, error: &ProviderError) -> String {
format!(
"{} returned{}{}: {}",
error.provider,
error
.status
.map(|status| format!(" HTTP {status}"))
.unwrap_or_default(),
error
.code
.as_deref()
.map(|code| format!(" ({code})"))
.unwrap_or_default(),
error.message
)
}

/// Decodes a non-2xx body into a structured [`ProviderError`].
///
/// Two classifications are applied on top of the raw decode:
Expand Down