fix(integrations): propagate backend error body in non-2xx responses (#1296)#1330
Conversation
…ponses (tinyhumansai#1296) Pre-fix the post/get error paths read the response body then discarded it (`let _body_text = …`), so callers only saw `Backend returned 400 …` with no detail — masking the actual cause (Insufficient balance, Toolkit not enabled, etc.). Now extract the `error` field from the standard `{success:false,error:"…"}` envelope, fall back to truncated raw text, and include it in both the `report_error` payload and the bail message. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…yhumansai#1296) Same diagnostic gap as integrations/client post/get — non-2xx DELETE responses bailed with a generic `Backend returned 4xx for DELETE …` that hid the actual reason. Use the shared `extract_error_detail` helper so DELETE failures (delete_connection, disable_trigger) carry the same detail as POST/GET failures. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…inyhumansai#1296) Tightens `disable_trigger_surfaces_non_2xx_status` to also assert the envelope error string lands in the bail message, and adds `delete_connection_surfaces_envelope_error_detail` covering the delete_connection path. Closes the asymmetry where post/get had explicit envelope-detail tests but DELETE only asserted the status code. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
📝 WalkthroughWalkthroughThis PR improves HTTP error reporting across Composio and integrations clients by extracting backend error details from response bodies, bounding their length for safety, and propagating them in error messages instead of reporting only HTTP status codes. ChangesBackend Error Detail Extraction Across HTTP Clients
Sequence Diagram(s)sequenceDiagram
participant Backend
participant Client
participant Extractor
participant Observability
participant Caller
Backend->>Client: HTTP non-2xx + response body
Client->>Client: read body text
Client->>Extractor: extract_error_detail(body, max_len)
alt JSON with error field
Extractor->>Extractor: parse JSON, trim whitespace
Extractor-->>Client: error field value
else Non-JSON or missing field
Extractor->>Extractor: truncate at UTF-8 boundary
Extractor-->>Client: truncated raw body + ellipsis
end
Client->>Observability: report with enriched detail
Client-->>Caller: Err("{status} for {method} {url}: {detail}")
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/openhuman/composio/client.rs (1)
319-329:⚠️ Potential issue | 🟠 Major | ⚡ Quick winAvoid byte-slicing the DELETE error body in the debug log.
&body_text[..body_text.len().min(300)]can panic when the 300-byte cutoff lands inside a multibyte UTF-8 character, so some non-ASCII backend errors will crash this failure path instead of returning the enrichedErr. Reuse the shared truncation helper here as well.Proposed fix
if !status.is_success() { let body_text = resp.text().await.unwrap_or_default(); let detail = crate::openhuman::integrations::client::extract_error_detail( &body_text, crate::openhuman::integrations::client::MAX_ERROR_BODY_LEN, ); + let logged_body = crate::openhuman::integrations::client::extract_error_detail( + &body_text, + 300, + ); tracing::debug!( "[composio] DELETE {} → {} body={}", url, status, - &body_text[..body_text.len().min(300)] + logged_body );🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/openhuman/composio/client.rs` around lines 319 - 329, The debug log is slicing body_text with a byte index which can panic on multibyte UTF-8; instead reuse the already-computed safe representation and truncation: log the safe `detail` produced by crate::openhuman::integrations::client::extract_error_detail (or call the module's safe truncation helper) rather than using `&body_text[..body_text.len().min(300)]`, e.g. replace that slice in the tracing::debug call with `detail` so the log uses a UTF-8-safe, truncated string.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/openhuman/integrations/client.rs`:
- Around line 40-48: The function truncate_at_char_boundary currently slices to
max bytes then appends an ellipsis, which can exceed the advertised cap; update
truncate_at_char_boundary to reserve space for the ellipsis before choosing the
slice point: compute the byte length of the ellipsis (e.g. '…') and set
available = max - ellipsis_len, then find the largest char boundary <= available
(same char-boundary loop currently using end) and slice to that boundary,
finally append the ellipsis; also handle the edge case when max is less than or
equal to the ellipsis byte length by returning a truncated ellipsis or empty
string appropriate for MAX_ERROR_BODY_LEN usage (ensure final result never
exceeds max). Mentioned symbols: truncate_at_char_boundary,
extract_error_detail, MAX_ERROR_BODY_LEN.
---
Outside diff comments:
In `@src/openhuman/composio/client.rs`:
- Around line 319-329: The debug log is slicing body_text with a byte index
which can panic on multibyte UTF-8; instead reuse the already-computed safe
representation and truncation: log the safe `detail` produced by
crate::openhuman::integrations::client::extract_error_detail (or call the
module's safe truncation helper) rather than using
`&body_text[..body_text.len().min(300)]`, e.g. replace that slice in the
tracing::debug call with `detail` so the log uses a UTF-8-safe, truncated
string.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: b1d8eb0e-81b6-4b52-913a-2b8376a5095d
📒 Files selected for processing (4)
src/openhuman/composio/client.rssrc/openhuman/composio/client_tests.rssrc/openhuman/integrations/client.rssrc/openhuman/integrations/client_tests.rs
…r_detail (tinyhumansai#1296) CodeRabbit Minor: `truncate_at_char_boundary` sliced to `max` then appended `…`, so `extract_error_detail(body, 500)` could return up to 503 bytes — leaking past the advertised hard cap that Sentry tag values + user-facing toasts depend on. Reserve `'…'.len_utf8()` (3 bytes) before choosing the slice end. Edge case: when `max < ellipsis_len`, return empty string rather than panic. Tightens the existing UTF-8 truncation test to assert `out.len() <= 50` strictly, and adds `extract_error_detail_with_max_below_ellipsis_returns_empty` covering the new edge case. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…og (tinyhumansai#1296) CodeRabbit Major: the existing `&body_text[..body_text.len().min(300)]` in the `[composio] DELETE` debug log panics when the 300-byte cutoff lands inside a multibyte codepoint, so non-ASCII backend errors crash the failure path instead of returning the enriched `Err`. Reuse `extract_error_detail(&body_text, 300)` so the log preview is bounded by the same safe truncation as the bail message. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
Addressed both CodeRabbit findings on dcf70d2:
Targeted tests still green (11/11 integrations + 24/24 composio). Both commits GPG-signed. |
…inyhumansai#1296) (tinyhumansai#1330) Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Summary
errorfield (or truncated raw body) in the bail / Sentry message wheneverIntegrationClient::post,IntegrationClient::get, orComposioClient::raw_deletereceives a non-2xx response.let _body_text = …), soBackend returned 400 …had no detail; now the same path yieldsBackend returned 400 … : Insufficient balance(orToolkit "X" is not enabled, etc.).Problem
Onboarding's
ContextGatheringStep(Building your profile) always fails for fresh users with Gmail connected. The pipeline dies at Stage 1 withcomposio execute_tool failed: Backend returned 400 Bad Requestand we cannot tell which 400 it is — backendexecuteTool.tsraisesBadRequestErrorfrom three sites (tool is required, toolkit-not-allowlisted,Insufficient balance) and the openhuman client throws away the body that would name the cause.integrations/client.rs:82-96was explicitly flagged in the issue as the diagnostic gap.Solution
extract_error_detail(body, max_bytes)insrc/openhuman/integrations/client.rs. When the body parses as the standard{success:false, error:"<msg>"}envelope, return the inner string verbatim (the authoritative failure message); otherwise truncate the raw body at a UTF-8 char boundary so the diagnostic stays bounded.MAX_ERROR_BODY_LEN) so the new path can't bloat tracing/Sentry payloads or user-facing toasts.IntegrationClient::post,IntegrationClient::get, andComposioClient::raw_delete. Both thereport_errorpayload and theanyhow::bail!message now include the detail.extract_error_detail(envelope happy path, whitespace trim, missing/blankerror, non-JSON fallback, empty body, multi-byte UTF-8 truncation), 3 axum-backed HTTP tests forpost 400 envelope/post 500 raw body/get 403 envelope, and 2 raw_delete envelope-detail tests on the composio side. All 10 + 24 pass locally.Submission Checklist
docs/TESTING-STRATEGY.mdextract_error_detail, the modified non-2xx branches, and theraw_deletechange is exercised by the new tests.cargo test openhuman::integrations::client→ 10/10;cargo test openhuman::composio::client→ 24/24.docs/TEST-COVERAGE-MATRIX.mdis unchanged.docs/TESTING-STRATEGY.md)Refs #1296, notCloses, because the user-visible failure (onboarding still dead-ends at "Almost there!") is not yet fixed. Closing bug(onboarding): Composio GMAIL_FETCH_EMAILS returns 400 — profile building always fails #1296 requires Phase B/C follow-ups (see ## Related).Impact
serde_json::from_str+ at-most-500-byte truncation on the error path. Happy path unchanged.extract_error_detailtruncates at 500 bytes at a UTF-8 char boundary so an unbounded backend body can't blow up tracing/Sentry. Backend error messages are written by the backend and the body was already being read into memory; no new sensitive data flows.Backend returned X for Y URL→Backend returned X for Y URL: <detail>); any external grep-on-bail-message consumer would need to extend its match. None known.post_400_propagates_backend_error_envelope_messageagainst an axum mock that mirrors the realerrorHandler.tsshape ({success:false, error:"…"}, status 400). Live verification of the new message landing in Sentry is deferred to Phase B post-merge.Related
integrations.post.non_2xxevent tagged withpath=/agent-integrations/composio/execute. The new message body will name the actual cause (Insufficient balance/Toolkit "x" is not enabled/ etc.).tinyhumansai/backendto grant signup credits OR exempt onboarding tool calls from balance check; AND/OR a small openhuman frontend follow-up that on a 400 inContextGatheringStepskips the gmail stage gracefully (mark failed, advance pipeline to LinkedIn) instead of dead-ending with "Almost there!".Closes #1296keyword will live on whichever follow-up actually flips onboarding back to working for fresh users.AI Authored PR Metadata (required for Codex/Linear PRs)
Linear Issue
Commit & Branch
fix/1296-composio-error-bodyValidation Run
pnpm --filter openhuman-app format:check— no frontend code changedpnpm typecheck— no TypeScript code changedcargo test --lib openhuman::integrations::client(11/11),cargo test --lib openhuman::composio::client(24/24)cargo fmt --checkclean on changed files;cargo check --manifest-path Cargo.tomlclean (warnings pre-existing main, unrelated)Validation Blocked
command:N/Aerror:N/Aimpact:N/ABehavior Changes
errorenvelope field (or truncated raw body) in the propagated error message + Sentry tag. No happy-path change.Parity Contract
Result<T, anyhow::Error>shape on success and on failure. Only the failure message string is enriched.extract_error_detailfalls back to truncated raw body when the envelope is malformed or missing theerrorfield, so non-conforming backends still produce a usable diagnostic instead of an empty string.