Skip to content

fix(api): suppress expected backend 401s from Sentry via typed-error flatten (#3297)#3336

Closed
oxoxDev wants to merge 4 commits into
tinyhumansai:mainfrom
oxoxDev:fix/3297-backend-401-sentry-suppress
Closed

fix(api): suppress expected backend 401s from Sentry via typed-error flatten (#3297)#3336
oxoxDev wants to merge 4 commits into
tinyhumansai:mainfrom
oxoxDev:fix/3297-backend-401-sentry-suppress

Conversation

@oxoxDev
Copy link
Copy Markdown
Contributor

@oxoxDev oxoxDev commented Jun 4, 2026

Summary

  • After fix(backend_api): suppress Sentry on 401 via typed BackendApiError::Unauthorized #2781 the backend 401 became a typed BackendApiError::Unauthorized (Display "backend rejected session token on {method} {path}"), but the team/billing ops flatten it to a String before the JSON-RPC dispatcher — and that string matches none of the session-expiry classifiers, so expected session-lapse 401s leak to Sentry.
  • Add api::flatten_authed_error, which downcasts the typed error and maps Unauthorized onto the existing SESSION_EXPIRED sentinel (every other error keeps its full {e:#} chain).
  • Route team::ops and billing::ops authed_json errors through the helper.
  • Suppresses TAURI-RUST-8WY (/teams/me/usage, 32 users) and TAURI-RUST-8WZ (/payments/stripe/currentPlan, 27 users), and the rest of the authed-endpoint family, without a brittle global string match.

Problem

BackendApiError::Unauthorized is an expected user-session state (token expired / revoked / rotated server-side), not a code bug — the REST layer intentionally skips report_error for it. But callers flatten it to a String (format!("{e:#}") / e.to_string()) before it reaches the JSON-RPC dispatcher (jsonrpc.rs). There, all three session-expiry classifiers — is_session_expired_error (jsonrpc.rs), is_session_expired_message (observability.rs), and the before_send net is_session_expired_event (observability.rs) — still match the old "401" + "unauthorized" + "GET /" wording. The new Display string matches none, so expected session-lapse 401s flow to Sentry on every authed_json caller once a session lapses.

Background: #2924 added a matching arm for "backend rejected session token", but #2959 deliberately reverted all string-based Sentry suppression. So re-adding a global prose arm is the wrong direction.

Solution

Fix it at the ops boundary — the last place the typed error still exists and is downcastable:

  • New api::flatten_authed_error(err: anyhow::Error) -> String:
    • downcast_ref::<BackendApiError>(); on Unauthorized { method, path } returns "SESSION_EXPIRED: backend rejected session token on {method} {path}".
    • everything else (including MessageNotFound) → unchanged format!("{e:#}").
  • The SESSION_EXPIRED sentinel is already recognised by is_session_expired_message, so the dispatcher both suppresses the Sentry report and publishes DomainEvent::SessionExpired (auth domain drives re-sign-in) — no new behavior wiring needed.
  • team::ops and billing::ops route their authed_json errors through the helper (single seam → endpoint-agnostic for current and future callers).

Design decision: the mapping keys off the typed downcast, not the Display wording, so it stays correct if the #[error(...)] text changes. The sentinel is minted deterministically from the type — not a match on upstream prose — which is consistent with #2959's intent (kill brittle string matches), unlike the reverted #2924 arm. A fully-typed end-to-end channel (changing the RPC Result<Value, String> error type) was considered and rejected as too large for this fix.

Submission Checklist

If a section does not apply to this change, mark the item as N/A with a one-line reason. Do not delete items.

  • Tests added or updated (happy path + at least one failure / edge case) per Testing Strategy
  • Diff coverage ≥ 80% — changed logic covered by 4 new focused tests (the two one-line .map_err(flatten_authed_error) seams in team/billing ops are exercised through the helper's own unit tests); enforced by the Rust Core Coverage CI gate.
  • Coverage matrix updated — N/A: behaviour-only change (no feature added/removed/renamed; internal error-routing refinement).
  • All affected feature IDs from the matrix are listed under ## RelatedN/A: behaviour-only change.
  • No new external network dependencies introduced (mock backend used per Testing Strategy)
  • Manual smoke checklist updated if this touches release-cut surfaces — N/A: no release-cut surface touched (internal Sentry error routing).
  • Linked issue closed via Closes #NNN in the ## Related section

Impact

  • Desktop (staging/production) Rust core only. No UI, no schema, no migration.
  • Reduces Sentry noise: expected session-lapse 401s on authed_json endpoints stop reporting as errors and instead drive the existing SessionExpired re-auth path.
  • No change to genuine failures (timeouts, DNS, non-401 status, MessageNotFound) — they keep their full anyhow chain and still reach Sentry.
  • Security: no secrets logged; sentinel carries only method + path (already non-sensitive).

Related

Sentry-Issue: TAURI-RUST-8WY
Sentry-Issue: TAURI-RUST-8WZ


AI Authored PR Metadata (required for Codex/Linear PRs)

Linear Issue

  • Key: N/A
  • URL: N/A

Commit & Branch

  • Branch: fix/3297-backend-401-sentry-suppress
  • Commit SHA: c585a3239bcc932a96238e58e3e488d3f60e1ccc

Validation Run

  • N/A: no frontend changespnpm --filter openhuman-app format:check
  • N/A: no frontend changespnpm typecheck
  • Focused tests: cargo test --lib -- api::rest::key_bytes_from_string_tests::flatten core::jsonrpc::tests::is_session_expired_error_matches_flattened_backend_unauthorized → 4 passed
  • Rust fmt/check (if changed): cargo fmt + cargo check (exit 0) + cargo clippy --lib (exit 0, no new warnings)
  • N/A: Tauri shell untouched — Tauri fmt/check

Validation Blocked

  • command: N/A
  • error: N/A
  • impact: N/A

Behavior Changes

  • Intended behavior change: expected backend 401s on team/billing authed_json calls are classified as session expiry (suppressed from Sentry, drive SessionExpired).
  • User-visible effect: none directly; fewer false-error reports, consistent re-auth on session lapse.

Parity Contract

  • Legacy behavior preserved: non-401 errors keep full {e:#} chain and still report; MessageNotFound not swallowed; 403 still reported.
  • Guard/fallback/dispatch parity checks: covered by flatten_authed_error_preserves_non_unauthorized_chain and flatten_authed_error_does_not_swallow_message_not_found.

Duplicate / Superseded PR Handling

  • Duplicate PR(s): none
  • Canonical PR: this
  • Resolution (closed/superseded/updated): N/A

Summary by CodeRabbit

  • Bug Fixes

    • Improved handling of session expiration errors to properly detect and classify unauthorized backend responses as session expirations, enabling more accurate error messaging and session recovery.
  • Tests

    • Added comprehensive test coverage for session expiration error classification and flattening logic.

@oxoxDev oxoxDev requested a review from a team June 4, 2026 08:21
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Jun 4, 2026

Review Change Stack

📝 Walkthrough

Walkthrough

This PR adds a flatten_authed_error helper that converts typed BackendApiError::Unauthorized responses into a SESSION_EXPIRED sentinel string, enabling JSON-RPC classifiers to recognize expected 401 backend errors. The helper is applied at call sites in billing and team ops, with comprehensive unit and regression test coverage.

Changes

Session-Expired Error Flattening

Layer / File(s) Summary
Helper function definition and re-export
src/api/rest.rs, src/api/mod.rs
flatten_authed_error downcasts anyhow::Error to BackendApiError, maps Unauthorized to a SESSION_EXPIRED: sentinel string (preserving method and path), and preserves all other errors in full {err:#} format. The function is added to rest.rs and re-exported via mod.rs.
Consumer call-site integration
src/openhuman/billing/ops.rs, src/openhuman/team/ops.rs
get_authed_value in both files now pipe BackendOAuthClient::authed_json errors through flatten_authed_error instead of formatting directly; inline comments document how Unauthorized is mapped to the SESSION_EXPIRED sentinel for JSON-RPC classification.
Unit and regression test coverage
src/api/rest_tests.rs, src/core/jsonrpc_tests.rs
Three unit tests in rest_tests.rs validate flatten_authed_error behavior: Unauthorized maps to SESSION_EXPIRED while preserving method/path and remaining classifiable as session-expiry; non-Unauthorized errors keep full anyhow chains; MessageNotFound does not collapse into session-expiry. A regression test in jsonrpc_tests.rs confirms is_session_expired_error recognizes the flattened unauthorized payload.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

  • tinyhumansai/openhuman#2781: Introduced the typed BackendApiError::Unauthorized in src/api/rest.rs for 401 handling; this PR adds flatten_authed_error plus call-site/test updates that map that Unauthorized into the SESSION_EXPIRED JSON-RPC classification.
  • tinyhumansai/openhuman#2292: Tightens is_session_expired_* logic and tests to only treat strict session-expired messages as DomainEvent::SessionExpired, complementing this PR's SESSION_EXPIRED sentinel mapping.
  • tinyhumansai/openhuman#2544: Adds regression coverage around is_session_expired_error behavior in src/core/jsonrpc_tests.rs, similar to the session-expiry classification test added here.

Suggested labels

rust-core, sentry-traced-bug, bug

Suggested reviewers

  • graycyrus

Poem

🐰 A sentinel string hops through the pipe,
SESSION_EXPIRED marks the 401 ripe.
No more Sentry surprises from sessions that fade—
Classifiers now see the backend cascade! 🎉

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The PR title clearly and concisely summarizes the main change: suppressing expected backend 401s from Sentry via a typed-error flatten helper function.
Linked Issues check ✅ Passed The PR fully implements the structured signal approach requested in #3297: it adds flatten_authed_error to downcast typed BackendApiError::Unauthorized to a SESSION_EXPIRED sentinel, wires team::ops and billing::ops through the helper, and adds comprehensive tests verifying the mapping and dispatcher recognition.
Out of Scope Changes check ✅ Passed All changes are directly scoped to the objectives: the helper function, its re-export, its integration into authed_json error paths, and comprehensive tests covering the new logic. No unrelated modifications detected.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.

✏️ 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 coderabbitai Bot added rust-core Core Rust runtime in src/: CLI, core_server, shared infrastructure. sentry-traced-bug Bug identified via Sentry triage bug labels Jun 4, 2026
@senamakel
Copy link
Copy Markdown
Member

no this is not the way to handle bugs. we need to RCA and patch it.

@oxoxDev
Copy link
Copy Markdown
Contributor Author

oxoxDev commented Jun 4, 2026

Superseded by #3384 — the root-cause fix (reject expired session locally before any backend call + gate usage/plan polling on auth). This PR's flatten_authed_error classifier is carried into #3384 as defense-in-depth.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug rust-core Core Rust runtime in src/: CLI, core_server, shared infrastructure. sentry-traced-bug Bug identified via Sentry triage

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Expected backend 401s leaking to Sentry after #2781 typed-error change (TAURI-RUST-8WY / 8WZ)

2 participants