From 865cf3e21fc1b306d8a7e79e9d825951e42539fa Mon Sep 17 00:00:00 2001 From: Nguyen Thanh Dat Date: Thu, 27 Aug 2026 15:57:23 +0700 Subject: [PATCH 1/2] fix(composio): surface the error body instead of discarding it MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Both Composio request paths read the response body and threw it away: let _ = response.bytes().await; anyhow::bail!("Composio direct request failed with HTTP {status}"); Composio answers a failure with a structured error whose `message` and `suggested_fix` name the actual problem and how to correct it. An entity-id mismatch, for instance, says which id to use. Callers got a bare status line and no route to a fix. `describe_failure` now builds the message from the body, for the direct and proxied paths alike. Only the known error fields are surfaced; an unrecognised body is truncated rather than echoed whole, and truncation cuts on a char boundary so a multi-byte body cannot panic the error path. The retry needles are tightened in the same change, and that is not housekeeping. `retryable_transport_error` classifies by substring, so once the message carries the response body, a body that merely mentions "HTTP 503" would turn a permanent 400 into a retry. The needles are now anchored on the `failed with HTTP ` clause this module emits, which a body cannot forge. `a_surfaced_body_cannot_forge_a_retryable_status` pins exactly that: with the old loose needles it fails (10 passed / 1 failed), so surfacing the body without this would have been a regression. The existing `retry_classification_is_by_status_not_by_substring` still passes unchanged — its own name is the property being strengthened here. `cargo test -p tinymemory-core composio::client` — 11 passed. --- .../src/sync/pipelines/composio/client.rs | 98 +++++++++++++++++-- .../sync/pipelines/composio/client_tests.rs | 75 ++++++++++++++ 2 files changed, 165 insertions(+), 8 deletions(-) diff --git a/crates/tinymemory-core/src/sync/pipelines/composio/client.rs b/crates/tinymemory-core/src/sync/pipelines/composio/client.rs index b7505f7..e11ce4b 100644 --- a/crates/tinymemory-core/src/sync/pipelines/composio/client.rs +++ b/crates/tinymemory-core/src/sync/pipelines/composio/client.rs @@ -186,8 +186,8 @@ impl ComposioClient { .map_err(|error| anyhow::anyhow!("Composio direct transport error: {error}"))?; let status = response.status(); if !status.is_success() { - let _ = response.bytes().await; - anyhow::bail!("Composio direct request failed with HTTP {status}"); + let body = response.text().await.unwrap_or_default(); + anyhow::bail!(describe_failure("direct", status, &body)); } let raw: serde_json::Value = decode_response(response, "direct").await?; Ok(decode_direct_response(raw)) @@ -218,8 +218,8 @@ impl ComposioClient { .map_err(|error| anyhow::anyhow!("Composio proxy transport error: {error}"))?; let status = response.status(); if !status.is_success() { - let _ = response.bytes().await; - anyhow::bail!("Composio proxy request failed with HTTP {status}"); + let body = response.text().await.unwrap_or_default(); + anyhow::bail!(describe_failure("proxy", status, &body)); } let raw: serde_json::Value = response .json() @@ -287,17 +287,99 @@ fn retryable_provider_error(error: Option<&str>) -> bool { /// needle that both status-bail messages also matched. fn retryable_transport_error(error: &anyhow::Error) -> bool { let message = error.to_string(); + // Anchored on the status clause this module actually emits. A bare + // "HTTP 429" needle would now be forgeable: the failure message carries the + // response body, and a body that merely mentions another status must not + // turn a permanent 400 into a retry. [ - "HTTP 429", - "HTTP 502", - "HTTP 503", - "HTTP 504", + "failed with HTTP 429", + "failed with HTTP 502", + "failed with HTTP 503", + "failed with HTTP 504", "transport error", ] .iter() .any(|needle| message.contains(needle)) } +/// Longest body snippet echoed for a response whose shape we do not recognise. +const FAILURE_BODY_LIMIT: usize = 400; + +/// Describe a non-2xx Composio response, using the body rather than throwing it away. +/// +/// Composio answers with a structured error whose `message` and `suggested_fix` +/// name the actual problem and how to correct it — an entity-id mismatch says +/// which id to use instead. Discarding it left callers with a bare status line +/// and no route to a fix. +/// +/// The `failed with HTTP {status}` clause is load-bearing: [`retryable_transport_error`] +/// keys off it, so it stays first and stays verbatim. +/// +/// Only the known error fields are surfaced. An unrecognised body is truncated +/// instead of echoed whole, so an unexpected payload cannot pour arbitrary +/// content into logs. +fn describe_failure(surface: &str, status: reqwest::StatusCode, body: &str) -> String { + let head = format!("Composio {surface} request failed with HTTP {status}"); + match failure_detail(body) { + Some(detail) => format!("{head}: {detail}"), + None => head, + } +} + +/// Pull the human-meaningful part out of a Composio error body. +fn failure_detail(body: &str) -> Option { + let trimmed = body.trim(); + if trimmed.is_empty() { + return None; + } + + if let Ok(parsed) = serde_json::from_str::(trimmed) { + if let Some(detail) = structured_detail(&parsed) { + return Some(detail); + } + } + + Some(truncate(trimmed, FAILURE_BODY_LIMIT)) +} + +/// `{"error": {"message": .., "slug": .., "suggested_fix": ..}}`, or a bare +/// `{"error": "..."}`. +fn structured_detail(parsed: &serde_json::Value) -> Option { + let error = parsed.get("error")?; + + if let Some(text) = error.as_str() { + let text = text.trim(); + return (!text.is_empty()).then(|| truncate(text, FAILURE_BODY_LIMIT)); + } + + let field = |name: &str| { + error + .get(name) + .and_then(serde_json::Value::as_str) + .map(str::trim) + .filter(|value| !value.is_empty()) + }; + + let message = field("message")?; + let mut detail = truncate(message, FAILURE_BODY_LIMIT); + if let Some(slug) = field("slug") { + detail.push_str(&format!(" [{slug}]")); + } + if let Some(fix) = field("suggested_fix") { + detail.push_str(&format!(" — suggested fix: {}", truncate(fix, FAILURE_BODY_LIMIT))); + } + Some(detail) +} + +/// Cut on a char boundary so a multi-byte body cannot panic the error path. +fn truncate(value: &str, limit: usize) -> String { + if value.chars().count() <= limit { + return value.to_owned(); + } + let kept: String = value.chars().take(limit).collect(); + format!("{kept}…") +} + async fn decode_response( response: reqwest::Response, mode: &str, diff --git a/crates/tinymemory-core/src/sync/pipelines/composio/client_tests.rs b/crates/tinymemory-core/src/sync/pipelines/composio/client_tests.rs index 8ce7e6f..30d7085 100644 --- a/crates/tinymemory-core/src/sync/pipelines/composio/client_tests.rs +++ b/crates/tinymemory-core/src/sync/pipelines/composio/client_tests.rs @@ -80,3 +80,78 @@ fn flat_proxy_response_remains_supported() { assert!(response.successful); assert_eq!(response.data["items"], serde_json::json!([1])); } + +/// The failure message now carries the response body, so a body that merely +/// mentions another status must not turn a permanent failure into a retry. +/// This is the hazard the needles were tightened against. +#[test] +fn a_surfaced_body_cannot_forge_a_retryable_status() { + let retry = |m: &str| retryable_transport_error(&anyhow::anyhow!("{m}")); + assert!(!retry( + "Composio direct request failed with HTTP 400 Bad Request: upstream said HTTP 503" + )); + assert!(!retry( + "Composio proxy request failed with HTTP 401 Unauthorized: retry after HTTP 429" + )); + // The real ones still classify. + assert!(retry( + "Composio direct request failed with HTTP 429 Too Many Requests: slow down" + )); +} + +/// Composio's structured error names the problem and how to fix it. This is the +/// payload from the report, verbatim. +#[test] +fn a_structured_error_body_reaches_the_message() { + let body = r#"{"error":{"message":"Connected account user ID does not match the provided user ID.","code":1812,"slug":"ActionExecute_ConnectedAccountEntityIdMismatch","status":400,"suggested_fix":"The connected_account_id you provided belongs to a different entity."}}"#; + let message = describe_failure("direct", reqwest::StatusCode::BAD_REQUEST, body); + + assert!( + message.starts_with("Composio direct request failed with HTTP 400"), + "the status clause must stay first and verbatim: {message}" + ); + assert!(message.contains("Connected account user ID does not match")); + assert!(message.contains("ActionExecute_ConnectedAccountEntityIdMismatch")); + assert!( + message.contains("belongs to a different entity"), + "the suggested fix is the part that turns a dead end into an action: {message}" + ); +} + +/// A bare `{"error": "..."}` string body is the other shape Composio returns. +#[test] +fn a_bare_error_string_body_reaches_the_message() { + let body = r#"{"error":"You have exceeded your credits limit.","tag":"NO_MORE_CREDITS"}"#; + let message = describe_failure("proxy", reqwest::StatusCode::PAYMENT_REQUIRED, body); + assert!(message.contains("exceeded your credits limit"), "{message}"); +} + +/// An unrecognised body is echoed but bounded, so an unexpected payload cannot +/// pour arbitrary content into the logs. +#[test] +fn an_unrecognised_body_is_truncated() { + let body = "x".repeat(5_000); + let message = describe_failure("direct", reqwest::StatusCode::BAD_GATEWAY, &body); + assert!(message.contains('…'), "expected an elision marker: {message}"); + assert!( + message.chars().count() < 600, + "the message grew to {} chars", + message.chars().count() + ); +} + +/// Truncation must cut on a char boundary — a multi-byte body must not panic +/// the error path. +#[test] +fn truncation_survives_multibyte_bodies() { + let body = "é".repeat(5_000); + let message = describe_failure("direct", reqwest::StatusCode::BAD_GATEWAY, &body); + assert!(message.contains('…'), "{message}"); +} + +/// No body, no change: the status line stands on its own as before. +#[test] +fn an_empty_body_leaves_the_status_line_alone() { + let message = describe_failure("direct", reqwest::StatusCode::NOT_FOUND, " "); + assert_eq!(message, "Composio direct request failed with HTTP 404 Not Found"); +} From 6338d17760fbf1b40163fa7e40d4f8eceaae77d2 Mon Sep 17 00:00:00 2001 From: Nguyen Thanh Dat Date: Thu, 27 Aug 2026 20:17:36 +0700 Subject: [PATCH 2/2] style(composio): satisfy rustfmt Three spots in the previous commit were over the width rustfmt wants and would have failed the format gate. --- .../src/sync/pipelines/composio/client.rs | 5 ++++- .../src/sync/pipelines/composio/client_tests.rs | 10 ++++++++-- 2 files changed, 12 insertions(+), 3 deletions(-) diff --git a/crates/tinymemory-core/src/sync/pipelines/composio/client.rs b/crates/tinymemory-core/src/sync/pipelines/composio/client.rs index e11ce4b..a3dd900 100644 --- a/crates/tinymemory-core/src/sync/pipelines/composio/client.rs +++ b/crates/tinymemory-core/src/sync/pipelines/composio/client.rs @@ -366,7 +366,10 @@ fn structured_detail(parsed: &serde_json::Value) -> Option { detail.push_str(&format!(" [{slug}]")); } if let Some(fix) = field("suggested_fix") { - detail.push_str(&format!(" — suggested fix: {}", truncate(fix, FAILURE_BODY_LIMIT))); + detail.push_str(&format!( + " — suggested fix: {}", + truncate(fix, FAILURE_BODY_LIMIT) + )); } Some(detail) } diff --git a/crates/tinymemory-core/src/sync/pipelines/composio/client_tests.rs b/crates/tinymemory-core/src/sync/pipelines/composio/client_tests.rs index 30d7085..2209586 100644 --- a/crates/tinymemory-core/src/sync/pipelines/composio/client_tests.rs +++ b/crates/tinymemory-core/src/sync/pipelines/composio/client_tests.rs @@ -132,7 +132,10 @@ fn a_bare_error_string_body_reaches_the_message() { fn an_unrecognised_body_is_truncated() { let body = "x".repeat(5_000); let message = describe_failure("direct", reqwest::StatusCode::BAD_GATEWAY, &body); - assert!(message.contains('…'), "expected an elision marker: {message}"); + assert!( + message.contains('…'), + "expected an elision marker: {message}" + ); assert!( message.chars().count() < 600, "the message grew to {} chars", @@ -153,5 +156,8 @@ fn truncation_survives_multibyte_bodies() { #[test] fn an_empty_body_leaves_the_status_line_alone() { let message = describe_failure("direct", reqwest::StatusCode::NOT_FOUND, " "); - assert_eq!(message, "Composio direct request failed with HTTP 404 Not Found"); + assert_eq!( + message, + "Composio direct request failed with HTTP 404 Not Found" + ); }