composio: preserve Composio tool error semantics instead of bucketing as 502 (#1797)#1827
Merged
senamakel merged 9 commits intoMay 16, 2026
Conversation
…inyhumansai#1814) Bind payload JSON once and slice via floor_char_boundary so multi-byte characters straddling byte 500 no longer abort the core thread (OPENHUMAN-TAURI-KC). Co-authored-by: Cursor <cursoragent@cursor.com>
…inyhumansai#1797) Validate and normalize high-traffic Composio actions before dispatch, classify provider failures with grep-friendly error classes, and retry Slack history rate limits so agents see actionable messages instead of generic gateway errors. Co-authored-by: Cursor <cursoragent@cursor.com>
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds Composio error classification/formatting, argument normalization/validation, a shared execute dispatcher with retry/backoff (Slack-specific rate-limit retries), and wires these through client/ops/tools plus a frontend formatter and UTF‑8-safe socket logging. ChangesComposio Error Classification and Tool Validation
Sequence DiagramsequenceDiagram
participant Caller
participant Dispatcher as execute_composio_action
participant Prepare as prepare_execute_arguments
participant Retry as execute_with_retries
participant Client as ComposioClient
participant ErrorMap as error_mapping::remap_transport_error
Caller->>Dispatcher: call(tool, arguments)
Dispatcher->>Prepare: prepare_execute_arguments(tool, arguments)
Dispatcher->>Retry: execute_with_retries(client, tool, args)
Retry->>Client: execute_with_auth_retry_inner(...)
Client-->>Retry: provider response / transport error
Retry->>Dispatcher: response or error
alt transport failure
Dispatcher->>ErrorMap: remap_transport_error(tool, raw)
ErrorMap-->>Caller: "[composio:error:<class>] …"
else provider unsuccessful
Dispatcher->>ErrorMap: format_provider_error(tool, resp.error)
ErrorMap-->>Caller: "[composio:error:<class>] …"
else success
Dispatcher-->>Caller: ComposioExecuteResponse
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related issues
Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (6)
src/openhuman/composio/execute_dispatch.rs (1)
84-84: 💤 Low valueRate-limit retry is Slack-specific but uses generic predicate.
The retry logic at line 84 checks
tool == SLACK_HISTORYbefore retrying on rate-limits, but theis_rate_limitedpredicate (lines 102-109) is generic. If other tools need rate-limit retry in the future, this condition would need to be generalized.Consider whether the Slack-specific check should be documented or if a more general approach (e.g., a list of tools that support rate-limit retry) would be clearer for future maintainers.
🤖 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/execute_dispatch.rs` at line 84, The retry branch currently gates rate-limit retries with a Slack-specific check (tool == SLACK_HISTORY) while using the generic predicate is_rate_limited(err_text); replace that hard equality with a clearer, maintainable policy: define and use a named collection/constant (e.g., RATE_LIMIT_RETRY_TOOLS) and check membership (e.g., RATE_LIMIT_RETRY_TOOLS.contains(&tool)) or, if Slack must remain the only case, add a short comment documenting why only SLACK_HISTORY should retry; update the conditional using symbols tool, SLACK_HISTORY, is_rate_limited, attempt, and RATELIMIT_MAX_ATTEMPTS accordingly so future tools can be added without changing logic.src/openhuman/composio/execute_prepare.rs (2)
5-32: ⚡ Quick winAdd debug logging for validation rejections.
When
prepare_execute_argumentsreturns an error (lines 11-14, 28, 33), there's no diagnostic trace. Given that this is a new validation layer that may reject previously-accepted calls, logging the rejection reason would aid debugging.📊 Add validation failure logging
Some(other) => { + tracing::debug!( + tool = %tool, + got_type = ?other, + "[composio][prepare] arguments must be object, rejecting" + ); return Err(format!( "composio: `{tool}` arguments must be a JSON object, got {}", other )); }And similarly for the tool-specific validation failures at lines 25-29.
As per coding guidelines: "In Rust, use
log/tracingatdebugortracelevel for development-oriented diagnostics on new/changed flows, including logs at entry/exit points, branch decisions, external calls, retries/timeouts, state transitions, and error handling paths."🤖 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/execute_prepare.rs` around lines 5 - 32, prepare_execute_arguments currently returns errors without logging; update it to emit debug/tracing logs whenever it rejects arguments: log the incoming tool and arguments and the error message before returning Err in the object-type mismatch branch inside prepare_execute_arguments, and likewise log validation failures just before propagating errors from normalize_calendar_time_bounds, ensure_notion_fetch_type, validate_gmail_send_email, and validate_gmail_add_label (include the tool name and the specific validation error). Use the project's tracing/log macros at debug or trace level and keep the logs concise to aid debugging.
61-62: ⚖️ Poor tradeoffDate format heuristic relies on byte indexing.
The check
s.len() == 10 && s.as_bytes().get(4) == Some(&b'-') && s.as_bytes().get(7) == Some(&b'-')works for ASCII dates but won't catch invalid dates like2026-99-99. This is acceptable as a pre-flight check (the provider will reject invalid dates), but consider whether tighter validation via a date-parsing crate would catch user errors earlier.🤖 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/execute_prepare.rs` around lines 61 - 62, The current heuristic in the if branch that checks s.len() and byte positions (s.len() == 10 && s.as_bytes().get(4) == Some(&b'-') && s.as_bytes().get(7) == Some(&b'-')) only validates ASCII layout and can accept impossible dates; replace this pre-flight with an explicit date parse (e.g., using chrono::NaiveDate::parse_from_str(s, "%Y-%m-%d") or time::Date::parse) to validate the YYYY-MM-DD value: if parsing succeeds, produce the normalized RFC3339 midnight string (e.g., format "{s}T00:00:00Z" or build from the parsed date) and otherwise fall back to the existing behavior (or return None/propagate error) so invalid dates like "2026-99-99" are rejected earlier; update the block that currently inspects s and bytes accordingly.src/openhuman/composio/error_mapping.rs (2)
114-118: 💤 Low valueToolkit extraction assumes specific naming pattern.
The toolkit name is derived by taking the first segment before
_, which works forGMAIL_SEND_EMAIL→gmailbut would fail for tools that don't follow theTOOLKIT_ACTIONpattern (e.g., a tool named justSEARCHwould producesearch, which is correct, but an edge case likeGMAILalone would also work).This is defensive and unlikely to be an issue given Composio's naming conventions, but consider documenting the assumption or adding a fallback for unexpected patterns.
🤖 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/error_mapping.rs` around lines 114 - 118, The current toolkit extraction (variable toolkit computed from tool.split('_').next().unwrap_or("integration").to_ascii_lowercase()) assumes tools follow TOOLKIT_ACTION naming; update it to be defensive by using the whole tool string when no '_' is present (i.e., if tool.contains('_') take the segment before '_' otherwise use tool), convert to_ascii_lowercase(), and only fall back to "integration" if the resulting string is empty; also add a brief inline comment near this logic explaining the naming assumption so future readers know why this behavior exists (refer to the toolkit variable and the split('_') extraction).
33-58: ⚡ Quick winConsider adding debug logging for classification decisions.
The
classify_composio_errorfunction makes important classification decisions (especially the gateway-vs-provider distinction at line 44) but produces no diagnostics. When debugging misclassified errors, knowing why a particular class was chosen would be valuable.📊 Add classification logging
pub fn classify_composio_error(tool: &str, message: &str) -> ComposioErrorClass { let lower = message.to_ascii_lowercase(); if is_validation_shape(&lower) { + tracing::debug!(tool = %tool, class = "validation", "[composio][error_mapping] classified"); return ComposioErrorClass::Validation; } if is_insufficient_scope_shape(&lower) { + tracing::debug!(tool = %tool, class = "insufficient_scope", "[composio][error_mapping] classified"); return ComposioErrorClass::InsufficientScope; } if is_rate_limited_shape(&lower) { + tracing::debug!(tool = %tool, class = "rate_limited", "[composio][error_mapping] classified"); return ComposioErrorClass::RateLimited; } if is_gateway_transport_shape(&lower) && !is_embedded_provider_failure(&lower) { + tracing::debug!(tool = %tool, class = "gateway", "[composio][error_mapping] classified"); return ComposioErrorClass::Gateway; } if is_composio_platform_shape(&lower) { + tracing::debug!(tool = %tool, class = "composio_platform", "[composio][error_mapping] classified"); return ComposioErrorClass::ComposioPlatform; } if tool.starts_with("GMAIL_") || tool.starts_with("SLACK_") || tool.starts_with("NOTION_") || tool.starts_with("GOOGLECALENDAR_") { + tracing::debug!(tool = %tool, class = "upstream_provider", "[composio][error_mapping] classified"); return ComposioErrorClass::UpstreamProvider; } + tracing::debug!(tool = %tool, class = "other", "[composio][error_mapping] classified"); ComposioErrorClass::Other }As per coding guidelines: "In Rust, use
log/tracingatdebugortracelevel for development-oriented diagnostics on new/changed flows, including logs at entry/exit points, branch decisions, external calls, retries/timeouts, state transitions, and error handling paths."🤖 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/error_mapping.rs` around lines 33 - 58, Add debug/tracing diagnostics to classify_composio_error: log entry with the tool and original message, then emit a debug/trace for each branch decision (e.g., when is_validation_shape, is_insufficient_scope_shape, is_rate_limited_shape, is_gateway_transport_shape (and the nested !is_embedded_provider_failure), is_composio_platform_shape, and the provider-tool prefix checks like tool.starts_with("GMAIL_")/ "SLACK_"/"NOTION_"/"GOOGLECALENDAR_"). Ensure each log shows which predicate matched (or that none matched) and the resulting ComposioErrorClass so it’s easy to trace why the gateway-vs-provider path was chosen; use the project’s tracing/log facade (tracing::debug! or log::debug!) and keep messages concise.src/openhuman/composio/execute_dispatch_tests.rs (1)
12-26: ⚡ Quick win
drop(tx)immediately shuts down the test server.Line 24 drops the oneshot sender right after creation, which closes the receiver channel and triggers graceful shutdown. The test still passes because the async shutdown completes after the test assertions run, but this makes the mock server lifecycle unclear.
🔧 Return the shutdown handle for explicit cleanup
-async fn start_mock_backend(app: Router) -> String { +async fn start_mock_backend(app: Router) -> (String, oneshot::Sender<()>) { let listener = tokio::net::TcpListener::bind("127.0.0.1:0").await.unwrap(); let addr = listener.local_addr().unwrap(); let (tx, rx) = oneshot::channel::<()>(); tokio::spawn(async move { axum::serve(listener, app) .with_graceful_shutdown(async { let _ = rx.await; }) .await .unwrap(); }); - drop(tx); - format!("http://{addr}") + (format!("http://{addr}"), tx) } // In test: - let base = start_mock_backend(app).await; + let (base, _shutdown) = start_mock_backend(app).await;Or simply remove the graceful shutdown logic if tests don't need explicit cleanup.
🤖 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/execute_dispatch_tests.rs` around lines 12 - 26, The test server is being shut down immediately because the oneshot sender `tx` is dropped in `start_mock_backend`, which closes the receiver `rx` and triggers `with_graceful_shutdown`; fix by either returning the shutdown handle so the caller can close the server explicitly (change `start_mock_backend` to return e.g. `(String, oneshot::Sender<()>)` and do not drop `tx` inside the function) or remove the graceful shutdown machinery entirely (drop `rx` usage and `with_graceful_shutdown` and just spawn `axum::serve(listener, app).await`), updating call sites to use the returned sender for cleanup if you choose the first option.
🤖 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/composio/ops.rs`:
- Line 268: The current Err(format!("[composio] execute failed: {e}")) in ops.rs
wraps every dispatcher error and can strip or hide a leading classification tag;
change the logic in the execute/error-return site (where variable e is produced)
to detect if e.starts_with("[composio:error:") and if so return Err(e)
unchanged, otherwise return the formatted Err with the "[composio] execute
failed: " prefix; update the branch that produces Err(...) to perform this
conditional so consumers can still match the original dispatcher error tag.
In `@src/openhuman/socket/event_handlers.rs`:
- Around line 35-40: The debug log currently prints raw, truncated payload text
via payload_json and preview_end (using
crate::openhuman::util::floor_char_boundary), which can leak secrets; change the
logging to avoid raw body content and instead log safe metadata such as the
payload byte length and structural info: compute payload_len =
payload_json.len() (or data.as_bytes().len()), and in the log::debug call for
event_name replace &payload_json[..preview_end] with a redacted summary like
"<REDACTED_BODY length={payload_len} chars>" (or include inferred content
type/field keys if available) so the log shows size/structure but never raw body
text. Ensure you update any uses of preview_end/floor_char_boundary accordingly
to avoid slicing into the body when redacting.
---
Nitpick comments:
In `@src/openhuman/composio/error_mapping.rs`:
- Around line 114-118: The current toolkit extraction (variable toolkit computed
from tool.split('_').next().unwrap_or("integration").to_ascii_lowercase())
assumes tools follow TOOLKIT_ACTION naming; update it to be defensive by using
the whole tool string when no '_' is present (i.e., if tool.contains('_') take
the segment before '_' otherwise use tool), convert to_ascii_lowercase(), and
only fall back to "integration" if the resulting string is empty; also add a
brief inline comment near this logic explaining the naming assumption so future
readers know why this behavior exists (refer to the toolkit variable and the
split('_') extraction).
- Around line 33-58: Add debug/tracing diagnostics to classify_composio_error:
log entry with the tool and original message, then emit a debug/trace for each
branch decision (e.g., when is_validation_shape, is_insufficient_scope_shape,
is_rate_limited_shape, is_gateway_transport_shape (and the nested
!is_embedded_provider_failure), is_composio_platform_shape, and the
provider-tool prefix checks like tool.starts_with("GMAIL_")/
"SLACK_"/"NOTION_"/"GOOGLECALENDAR_"). Ensure each log shows which predicate
matched (or that none matched) and the resulting ComposioErrorClass so it’s easy
to trace why the gateway-vs-provider path was chosen; use the project’s
tracing/log facade (tracing::debug! or log::debug!) and keep messages concise.
In `@src/openhuman/composio/execute_dispatch_tests.rs`:
- Around line 12-26: The test server is being shut down immediately because the
oneshot sender `tx` is dropped in `start_mock_backend`, which closes the
receiver `rx` and triggers `with_graceful_shutdown`; fix by either returning the
shutdown handle so the caller can close the server explicitly (change
`start_mock_backend` to return e.g. `(String, oneshot::Sender<()>)` and do not
drop `tx` inside the function) or remove the graceful shutdown machinery
entirely (drop `rx` usage and `with_graceful_shutdown` and just spawn
`axum::serve(listener, app).await`), updating call sites to use the returned
sender for cleanup if you choose the first option.
In `@src/openhuman/composio/execute_dispatch.rs`:
- Line 84: The retry branch currently gates rate-limit retries with a
Slack-specific check (tool == SLACK_HISTORY) while using the generic predicate
is_rate_limited(err_text); replace that hard equality with a clearer,
maintainable policy: define and use a named collection/constant (e.g.,
RATE_LIMIT_RETRY_TOOLS) and check membership (e.g.,
RATE_LIMIT_RETRY_TOOLS.contains(&tool)) or, if Slack must remain the only case,
add a short comment documenting why only SLACK_HISTORY should retry; update the
conditional using symbols tool, SLACK_HISTORY, is_rate_limited, attempt, and
RATELIMIT_MAX_ATTEMPTS accordingly so future tools can be added without changing
logic.
In `@src/openhuman/composio/execute_prepare.rs`:
- Around line 5-32: prepare_execute_arguments currently returns errors without
logging; update it to emit debug/tracing logs whenever it rejects arguments: log
the incoming tool and arguments and the error message before returning Err in
the object-type mismatch branch inside prepare_execute_arguments, and likewise
log validation failures just before propagating errors from
normalize_calendar_time_bounds, ensure_notion_fetch_type,
validate_gmail_send_email, and validate_gmail_add_label (include the tool name
and the specific validation error). Use the project's tracing/log macros at
debug or trace level and keep the logs concise to aid debugging.
- Around line 61-62: The current heuristic in the if branch that checks s.len()
and byte positions (s.len() == 10 && s.as_bytes().get(4) == Some(&b'-') &&
s.as_bytes().get(7) == Some(&b'-')) only validates ASCII layout and can accept
impossible dates; replace this pre-flight with an explicit date parse (e.g.,
using chrono::NaiveDate::parse_from_str(s, "%Y-%m-%d") or time::Date::parse) to
validate the YYYY-MM-DD value: if parsing succeeds, produce the normalized
RFC3339 midnight string (e.g., format "{s}T00:00:00Z" or build from the parsed
date) and otherwise fall back to the existing behavior (or return None/propagate
error) so invalid dates like "2026-99-99" are rejected earlier; update the block
that currently inspects s and bytes accordingly.
🪄 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: b0293a95-1492-4b3f-b220-014e6b545cb3
📒 Files selected for processing (14)
app/src/lib/composio/formatters.test.tsapp/src/lib/composio/formatters.tssrc/openhuman/composio/action_tool.rssrc/openhuman/composio/client.rssrc/openhuman/composio/error_mapping.rssrc/openhuman/composio/error_mapping_tests.rssrc/openhuman/composio/execute_dispatch.rssrc/openhuman/composio/execute_dispatch_tests.rssrc/openhuman/composio/execute_prepare.rssrc/openhuman/composio/execute_prepare_tests.rssrc/openhuman/composio/mod.rssrc/openhuman/composio/ops.rssrc/openhuman/composio/tools.rssrc/openhuman/socket/event_handlers.rs
Comment on lines
+35
to
+40
| let payload_json = data.to_string(); | ||
| let preview_end = crate::openhuman::util::floor_char_boundary(&payload_json, 500); | ||
| log::debug!( | ||
| "[socket] event payload: name={} data={}", | ||
| event_name, | ||
| &data.to_string()[..data.to_string().len().min(500)] | ||
| &payload_json[..preview_end] |
Contributor
There was a problem hiding this comment.
Redact payload content from debug logging.
Line 38 currently logs raw payload content (truncated), which can still leak secrets/PII for short events. Keep byte length and structural metadata, but omit raw body text.
Proposed fix
let payload_json = data.to_string();
let preview_end = crate::openhuman::util::floor_char_boundary(&payload_json, 500);
+ let payload_shape = match &data {
+ serde_json::Value::Object(map) => format!("object_keys={}", map.len()),
+ serde_json::Value::Array(arr) => format!("array_len={}", arr.len()),
+ serde_json::Value::String(_) => "string".to_string(),
+ serde_json::Value::Number(_) => "number".to_string(),
+ serde_json::Value::Bool(_) => "bool".to_string(),
+ serde_json::Value::Null => "null".to_string(),
+ };
log::debug!(
- "[socket] event payload: name={} data={}",
+ "[socket] event payload: name={} data_bytes={} preview_bytes={} shape={} preview_omitted=true",
event_name,
- &payload_json[..preview_end]
+ payload_json.len(),
+ preview_end,
+ payload_shape
);As per coding guidelines: “Never log secrets, raw JWTs, API keys, credentials, or full PII in debug logs; redact or omit sensitive fields.”
🤖 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/socket/event_handlers.rs` around lines 35 - 40, The debug log
currently prints raw, truncated payload text via payload_json and preview_end
(using crate::openhuman::util::floor_char_boundary), which can leak secrets;
change the logging to avoid raw body content and instead log safe metadata such
as the payload byte length and structural info: compute payload_len =
payload_json.len() (or data.as_bytes().len()), and in the log::debug call for
event_name replace &payload_json[..preview_end] with a redacted summary like
"<REDACTED_BODY length={payload_len} chars>" (or include inferred content
type/field keys if available) so the log shows size/structure but never raw body
text. Ensure you update any uses of preview_end/floor_char_boundary accordingly
to avoid slicing into the body when redacting.
- ops.rs: preserve `[composio:error:<class>] …` tag from the dispatcher so the frontend formatter in app/src/lib/composio/formatters.ts can parse the class. Updated ops_tests::composio_execute_via_mock_propagates_backend_error to assert the new classified format. - error_mapping.rs: log classifier outcome once per call (tool + class) so the gateway-vs-provider decision is traceable. - execute_dispatch_tests.rs: drop the broken oneshot graceful-shutdown plumbing (sender was dropped immediately, signalling shutdown before the test could exercise the server). - execute_dispatch.rs: include the validation error message in the pre-flight rejection debug log; document why only SLACK_FETCH_CONVERSATION_HISTORY is allow-listed for transparent rate-limit retries. - execute_prepare.rs: validate bare YYYY-MM-DD with chrono::NaiveDate so impossible dates like 2026-99-99 are rejected up front instead of being passed through to Google Calendar. - client_tests.rs: fix execute_tool_surfaces_non_successful_provider_response to use a tool slug that bypasses local arg validation, so the test exercises the gateway-response path as intended.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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/composio/ops.rs`:
- Around line 268-275: Add branch-level debug tracing when mapping execute
errors in the error-handling block that checks
e.starts_with("[composio:error:"), so you log which branch was taken and include
the error payload; specifically, in the block inside ops.rs where the code
currently returns Err(e) or Err(format!("[composio] execute failed: {e}")), emit
a debug/trace call (use the project's tracing macro, e.g., tracing::debug!)
before returning — one message for the "classified" branch indicating a
preserved composio:error class and the error string, and another for the
"wrapped" branch indicating an execute failure and the wrapped message including
e.
🪄 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: 0ec6fa6f-70c3-4e06-8d25-ebef2cb17699
📒 Files selected for processing (7)
src/openhuman/composio/client_tests.rssrc/openhuman/composio/error_mapping.rssrc/openhuman/composio/execute_dispatch.rssrc/openhuman/composio/execute_dispatch_tests.rssrc/openhuman/composio/execute_prepare.rssrc/openhuman/composio/ops.rssrc/openhuman/composio/ops_tests.rs
🚧 Files skipped from review as they are similar to previous changes (3)
- src/openhuman/composio/execute_dispatch.rs
- src/openhuman/composio/execute_prepare.rs
- src/openhuman/composio/error_mapping.rs
…sio-error-mapping # Conflicts: # src/openhuman/composio/ops.rs
…oolError Bring diff coverage for formatters.ts above the 80% gate by exercising the empty-body fallback messages for each classified error case (validation, insufficient_scope, rate_limited, gateway) plus the unknown-class default and null/undefined/empty inputs.
# Conflicts: # src/openhuman/socket/event_handlers.rs
# Conflicts: # src/openhuman/composio/action_tool.rs # src/openhuman/composio/mod.rs # src/openhuman/composio/ops.rs # src/openhuman/composio/tools.rs
- composio/ops.rs: emit branch-level tracing when mapping execute errors (classified vs wrapped) so incident triage can grep the decision (CodeRabbit #3250474827). - socket/event_handlers.rs: stop logging raw payload bytes at debug level; emit byte-length + top-level shape only so PII / secrets / auth tokens in socket frames cannot leak through debug logs (CodeRabbit #3250222027). - composio/tools.rs: rustfmt collapse of the `super::client` import group left over after dropping `direct_execute`.
# Conflicts: # src/openhuman/composio/client.rs
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
2 participants
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
[composio:error:<class>]prefixes instead of generic gateway wording.timeMin/timeMaxbare dates → RFC 3339,NOTION_FETCH_DATAfetch_typeinference, Gmail send/label required-field checks, and Slack history rate-limit backoff.formatComposioToolErrorin the app for UI-friendly classified error copy.Problem
Composio support confirmed there were no platform 502s for our org — tool-level validation, missing OAuth scopes, and upstream provider 429/403 failures were being surfaced to agents/users as generic 502/gateway instability (#1797).
Solution
execute_prepare.rs— local validation + normalization before any HTTP call.error_mapping.rs— classify provider/transport failures; re-label embedded provider errors inside 502 bodies as validation/upstream_provider, not gateway.execute_dispatch.rs— single dispatch used bycomposio_execute, per-action tools, and RPC; includes SlackSLACK_FETCH_CONVERSATION_HISTORYrate-limit retries.formatComposioToolError— strips classified prefixes for UI.Submission Checklist
coverage.ymlwill enforce; fullpnpm test:coverage+pnpm test:rustnot run in this session.N/A: behaviour-only change in composio execute/error path## Related—N/AN/ACloses #NNNin the## RelatedsectionImpact
Related
AI Authored PR Metadata (required for Codex/Linear PRs)
Linear Issue
Commit & Branch
cursor/a03-1797-composio-error-mapping711e27e8c9badd99f47faed40a9e597f8347b378Validation Run
pnpm --filter openhuman-app format:checkpnpm typecheckcargo test --lib composio::execute+composio::error_mapping, Vitestformatters.test.tscargo fmt,cargo check --libValidation Blocked
command:pnpm test:coverage+pnpm test:rust(full diff-cover)error:Not run locally — rely on CI coverage workflow.impact:May need follow-up if diff-cover < 80%.Behavior Changes
Parity Contract
execute_with_auth_retry_inner.composio_executeand per-action tools shareexecute_composio_action.Duplicate / Superseded PR Handling
Summary by CodeRabbit
New Features
Bug Fixes
Tests