From 81b8cec4edd9c4ba7ec7b9296bf48d44ce58e1cf Mon Sep 17 00:00:00 2001 From: M3gA-Mind Date: Mon, 24 Aug 2026 16:04:02 +0530 Subject: [PATCH 1/3] fix(openai): keep transport failures structured instead of flattening them MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `send_checked` built a fully-populated `ProviderError` for a transport failure — provider, model, and a computed `retryable` — and then threw the struct away, returning a flattened `TinyAgentsError::Model(String)`. The non-2xx branch twelve lines below kept its structure. Hosts classify on the variant. So every transport failure reached the OpenHuman backend model's logger through its generic arm and was recorded as `managed invoke failed (non-provider error): ...` — no status, no provider, no retryability — which is exactly the diagnostic gap reported against the managed path. A connection reset and a rejected request were indistinguishable in the log. The crate's own contract already says this branch is wrong: `TinyAgentsError::Provider` is documented as what adapters raise whenever they have a `ProviderError` in hand, and this one is constructed on the line above. - The `map_err` closure returns `TinyAgentsError::from_provider_error(...)`. - `provider_failure_message` loses its only caller and is deleted; the crate denies warnings, so leaving it would fail on `dead_code`. `Display for ProviderError` already emits the identical format string. - The provider README is corrected — it documented the flattening as intentional. Nothing user-facing changes. Both error variants render as `model error: {0}`, and `Display for ProviderError` reproduces `provider_failure_message` field for field, so the rendered text is byte-identical. Retry behaviour is unchanged: `provider_error` already computes `retryable` with the same classifier `is_retryable` applies to the `Model` string today — the flag simply becomes a stored fact instead of being re-derived from English prose at each call site. This does make `TinyAgentsError::Provider` reachable with `status: None` for the first time on this path. Every current consumer tolerates it — `maybe_publish_session_expired` gates on `Some(401 | 403)` and `from_provider_error` gates on `code`, which is `None` here — but anything added later that assumes `Provider` implies a status will now see `None`. Refs tinyhumansai/openhuman#5604 --- src/harness/providers/openai/README.md | 10 ++-- src/harness/providers/openai/test.rs | 60 +++++++++++++++++++++++ src/harness/providers/openai/transport.rs | 33 +++++-------- 3 files changed, 79 insertions(+), 24 deletions(-) diff --git a/src/harness/providers/openai/README.md b/src/harness/providers/openai/README.md index 297ec96e..b0bcbce6 100644 --- a/src/harness/providers/openai/README.md +++ b/src/harness/providers/openai/README.md @@ -224,10 +224,12 @@ 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 -`TinyAgentsError::Serialization`. +Transport-level failures (connection errors, body-read failures) carry no HTTP +status, but they do carry a provider, a model, and a computed `retryable`, so +they are raised as `TinyAgentsError::Provider` with `status: None` rather than +flattened — a host that classifies on the variant would otherwise see a +connection reset as an unstructured error. `Display` is identical either way. +Malformed JSON bodies surface as `TinyAgentsError::Serialization`. ## Operational constraints diff --git a/src/harness/providers/openai/test.rs b/src/harness/providers/openai/test.rs index a4f402b5..a2b21d4c 100644 --- a/src/harness/providers/openai/test.rs +++ b/src/harness/providers/openai/test.rs @@ -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}" + ); +} diff --git a/src/harness/providers/openai/transport.rs b/src/harness/providers/openai/transport.rs index 00ef30d3..2b2abfb0 100644 --- a/src/harness/providers/openai/transport.rs +++ b/src/harness/providers/openai/transport.rs @@ -1993,9 +1993,19 @@ impl OpenAiModel { url: &str, ) -> Result { 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(); @@ -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: From 39e2137c793712621f69ffc285bcf941b5e9f030 Mon Sep 17 00:00:00 2001 From: M3gA-Mind Date: Mon, 24 Aug 2026 16:15:27 +0530 Subject: [PATCH 2/3] docs(openai): scope the structured-error note to send failures The previous wording said transport-level failures including body reads are raised as `TinyAgentsError::Provider`. Only send failures are: `list_models` and `invoke_responses` still map a failed body read to `TinyAgentsError::Model`, and this change does not touch them. Naming the remaining `Model` paths explicitly is better than a sentence that is true of one call site and wrong about two. --- src/harness/providers/openai/README.md | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/src/harness/providers/openai/README.md b/src/harness/providers/openai/README.md index b0bcbce6..92cf237d 100644 --- a/src/harness/providers/openai/README.md +++ b/src/harness/providers/openai/README.md @@ -224,12 +224,16 @@ 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) carry no HTTP -status, but they do carry a provider, a model, and a computed `retryable`, so -they are raised as `TinyAgentsError::Provider` with `status: None` rather than -flattened — a host that classifies on the variant would otherwise see a -connection reset as an unstructured error. `Display` is identical either way. -Malformed JSON bodies surface as `TinyAgentsError::Serialization`. +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 +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 From a5d05934ecd6a80be7d0a19026e736c08f3eb344 Mon Sep 17 00:00:00 2001 From: M3gA-Mind Date: Mon, 24 Aug 2026 20:23:34 +0530 Subject: [PATCH 3/3] docs(openai): a send failure means no final status, not no server MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Saying the request "never reached a server" is wrong for one real case: the client keeps reqwest's default redirect policy — `OpenAiModel::new` sets only a connect timeout and never calls `.redirect()` — so a redirect loop or an exceeded redirect limit fails `send` after servers have answered with 3xx. `status: None` is still the right mapping; the reason is that no *final* status was available, which is what the text now says. --- src/harness/providers/openai/README.md | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/src/harness/providers/openai/README.md b/src/harness/providers/openai/README.md index 92cf237d..53a944e8 100644 --- a/src/harness/providers/openai/README.md +++ b/src/harness/providers/openai/README.md @@ -224,12 +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. -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` +A **send** failure — no final HTTP status was available, so there is nothing to +report as one — 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 otherwise see a connection reset as an unstructured error. `Display` is -identical either way. +identical either way. Note that "no final status" is not the same as "never +reached a server": the client keeps reqwest's default redirect policy, so a +redirect loop or an exceeded redirect limit lands here too, after servers have +answered with 3xx. **Body-read** failures after a 2xx (`list_models`, `invoke_responses`) remain plain `TinyAgentsError::Model` strings. Malformed JSON bodies surface as