Skip to content

fix(observability): classify backend 4xx as BackendUserError (OPENHUMAN-TAURI-BC)#1676

Merged
senamakel merged 3 commits into
tinyhumansai:mainfrom
CodeGhost21:fix/integrations-skip-4xx-user-errors-bc
May 14, 2026
Merged

fix(observability): classify backend 4xx as BackendUserError (OPENHUMAN-TAURI-BC)#1676
senamakel merged 3 commits into
tinyhumansai:mainfrom
CodeGhost21:fix/integrations-skip-4xx-user-errors-bc

Conversation

@CodeGhost21
Copy link
Copy Markdown
Contributor

@CodeGhost21 CodeGhost21 commented May 13, 2026

Summary

  • Adds an ExpectedErrorKind::BackendUserError variant matching the canonical wire shape "Backend returned <status> ..." produced by IntegrationClient::post/get and ComposioClient::delete.
  • Classifies 4xx except 408/429 (those stay actionable — 408/429 are transient, and a sustained 429 rate-limit cliff still needs to surface).
  • Switches three call sites — src/openhuman/integrations/client.rs::post, ::get, src/openhuman/composio/client.rs::delete — from raw report_error to report_error_or_expected so the classifier suppresses user-input / auth-state shapes.
  • 5xx remains actionable (server bugs should still reach Sentry).

Why

OPENHUMAN-TAURI-BC: a user submitted SharePoint authorize without the required Tenant Name field. Composio correctly returned 400 ConnectedAccount_MissingRequiredFields, the UI surfaced the structured error to the user via toast, but the integrations client still fired a Sentry event with no remediation path — the request was malformed by the user's input, not by our code.

Per the established "surface the deeper fix" rule:

  • UI pre-flight validation of every Composio toolkit's required fields would prevent these requests entirely, but requires the UI to track Composio's per-toolkit field schemas (which vary and change over time) — that's a non-trivial product feature, not a bug fix.
  • The current UX (submit → backend validates → toast) is correct. Only the Sentry noise needs to stop.

Mirrors the existing NetworkUnreachable (OPENHUMAN-TAURI-32) and TransientUpstreamHttp (OPENHUMAN-TAURI-5Z) skips — both wire user-environment shapes through the same report_error_or_expected path.

Fixes OPENHUMAN-TAURI-BC

Test plan

  • `cargo check --tests` — clean (only pre-existing warnings unrelated to this change)
  • `cargo test --lib core::observability` — 19 passed (2 new tests)
  • `cargo test --lib openhuman::integrations::client` — 13 passed (2 new tests)
  • `cargo test --lib openhuman::composio::client` — 27 passed (call-site change compiles + existing behavior intact)
  • New tests:
    • `classifies_backend_user_error_responses` — 400/401/403/404/422/451 + the exact OPENHUMAN-TAURI-BC wire shape
    • `does_not_classify_transient_or_server_backend_errors_as_user_error` — 408/429/500/502/503/504 stay actionable
    • `post_400_user_input_failure_classifies_as_backend_user_error` — HTTP-driven regression pinning the bail format to the classifier
    • `post_500_remains_actionable` — counterpart guard

Summary by CodeRabbit

  • Bug Fixes

    • Improved backend error handling: certain non-transient 4xx backend responses are now treated as non-actionable user/authentication issues, reducing noisy error alerts and preserving actionable reporting for 5xx and transient errors.
  • Tests

    • Added regression tests to ensure correct classification of backend 4xx vs 5xx responses.

Review Change Stack

…AN-TAURI-BC)

The integrations / composio backend clients reported every non-2xx
response through bare `report_error`, so a 4xx user-input failure (here:
SharePoint authorize 400 because the user didn't fill in the required
Tenant Name field — Composio slug `ConnectedAccount_MissingRequiredFields`)
fired a Sentry event Sentry has no signal to act on — the request was
malformed by the user's input, not by our code; the UI already toasts
the structured backend error.

Adds an `ExpectedErrorKind::BackendUserError` variant matching the
canonical `"Backend returned <status>"` wire shape produced by
`IntegrationClient::post`/`get` and `ComposioClient::delete`. The
classifier covers 4xx (except 408/429, which are transient and stay
actionable via the existing path or sustained-rate-limit surfacing).
5xx remains actionable — server bugs should reach Sentry. The three
call sites switch from `report_error` to `report_error_or_expected`.

Tests pin both directions:
- `classifies_backend_user_error_responses`: 400/401/403/404/422/451
  and the exact OPENHUMAN-TAURI-BC wire shape classify as BackendUserError
- `does_not_classify_transient_or_server_backend_errors_as_user_error`:
  408/429/500/502/503/504 do **not** classify (preserves OPENHUMAN-TAURI-8M
  intent for sustained outages and rate-limit cliffs)
- `post_400_user_input_failure_classifies_as_backend_user_error`:
  HTTP-driven regression — bail message from `IntegrationClient::post`
  classifies, locking the format string to the classifier match
- `post_500_remains_actionable`: 5xx bail message does not classify

Fixes OPENHUMAN-TAURI-BC
@CodeGhost21 CodeGhost21 requested a review from a team May 13, 2026 18:48
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 13, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 747624c7-ec4c-4df9-957f-b324a2e75dd1

📥 Commits

Reviewing files that changed from the base of the PR and between 15bb8b2 and c8a96c2.

📒 Files selected for processing (1)
  • src/core/observability.rs
🚧 Files skipped from review as they are similar to previous changes (1)
  • src/core/observability.rs

📝 Walkthrough

Walkthrough

Adds ExpectedErrorKind::BackendUserError and is_backend_user_error_message to detect backend-formatted 4xx messages (excluding 408/429), routes client non-2xx responses through report_error_or_expected, and logs classified backend 4xx as tracing::warn! breadcrumbs; tests verify 4xx classification and 5xx retention.

Changes

Backend user-error classification and observability routing

Layer / File(s) Summary
Backend user error classification contract and detection
src/core/observability.rs
ExpectedErrorKind gains BackendUserError. is_backend_user_error_message parses Backend returned <status> ... and expected_error_kind maps non-transient 4xx (excluding 408/429) to BackendUserError.
Observability reporting for backend user errors
src/core/observability.rs
report_expected_message recognizes BackendUserError and emits a tracing::warn! breadcrumb, suppressing Sentry error events for these cases.
Client HTTP error path routing to classification
src/openhuman/composio/client.rs, src/openhuman/integrations/client.rs
ComposioClient::raw_delete, IntegrationClient::post, and IntegrationClient::get now call report_error_or_expected(...) instead of report_error(...) on non-2xx responses, enabling the new classification to demote certain 4xx into warn breadcrumbs while leaving 5xx actionable.
Regression and unit tests for classification behavior
src/openhuman/integrations/client_tests.rs, src/core/observability.rs
New integration tests verify a 400 composio response classifies as BackendUserError and a 500 remains actionable; unit tests cover multiple 4xx classifications and exclusion of transient 408/429 and 5xx, plus non-matching messages.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related issues

Possibly related PRs

Suggested labels

working

Suggested reviewers

  • senamakel

Poem

🐰 I hopped through logs with careful cheer,
Found four-hundred whispers, calm and clear,
Warn breadcrumbs placed where users erred,
Five-hundreds still get flagged and heard,
A tidy hop — observability, my dear.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately describes the primary change: adding BackendUserError classification for backend 4xx responses in observability, with a specific ticket reference.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.


Comment @coderabbitai help to get the list of available commands and usage tips.

coderabbitai[bot]
coderabbitai Bot previously approved these changes May 13, 2026
Copy link
Copy Markdown
Member

@senamakel senamakel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

resolve merge conflicts

@senamakel senamakel self-assigned this May 14, 2026
…PENHUMAN-TAURI-BC)

Resolved merge conflicts in src/core/observability.rs and
src/openhuman/integrations/client.rs, incorporating the LocalAiCapabilityUnavailable
classifier and TRANSIENT_HTTP_STATUSES/TRANSIENT_TRANSPORT_PHRASES constants from
main alongside the PR's BackendUserError classification.

All 466 related unit tests pass (30 observability, 69 integrations, 367 composio).
cargo check clean on both core and Tauri shell crates.
@senamakel
Copy link
Copy Markdown
Member

pr-manager update (automated)

Merge conflicts in src/core/observability.rs and src/openhuman/integrations/client.rs have been resolved and pushed to senamakel/openhuman:pr/1676 (commit c8a96c2).

Quality suite results:

  • cargo fmt — clean (no changes needed)
  • cargo check (core + Tauri shell) — clean (pre-existing warnings only, none from this PR)
  • cargo test --lib core::observability — 30/30 passed
  • cargo test --lib openhuman::integrations — 69/69 passed
  • cargo test --lib openhuman::composio — 367/367 passed
  • Pre-push hooks (Prettier, ESLint, rust format check, cargo check) — all passed

Note: This branch is on senamakel/openhuman:pr/1676 (the maintainer's fork copy), not CodeGhost21/openhuman. CodeRabbit has already APPROVED the PR. The senamakel CHANGES_REQUESTED review (resolve merge conflicts) is now addressed.

@coderabbitai coderabbitai Bot added the working A PR that is being worked on by the team. label May 14, 2026
@senamakel senamakel merged commit 59e469c into tinyhumansai:main May 14, 2026
25 of 28 checks passed
AusAgentSmith pushed a commit to AusAgentSmith/openhuman that referenced this pull request May 23, 2026
…AN-TAURI-BC) (tinyhumansai#1676)

Co-authored-by: Steven Enamakel <enamakel@tinyhumans.ai>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

working A PR that is being worked on by the team.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants