fix(channels/discord): convert upstream 401/403 to domain-scoped error so card click can't sign user out (#2285)#2376
Merged
graycyrus merged 1 commit intoMay 22, 2026
Conversation
…r so card click can't sign user out (tinyhumansai#2285) The Channels page's Discord card opens a setup modal that calls `channels_discord_list_guilds` → `Discord REST /users/@me/guilds`. When the bot token is stale/revoked, Discord returns 401 and the client `bail!`s the literal "Discord list guilds failed (401 Unauthorized): {body}". That string flows up through JSON-RPC as `Err(String)` and trips `src/core/jsonrpc.rs::is_session_expired_error`, which classifies ANY `lower.contains("401") && lower.contains("unauthorized")` as backend session expiry — publishing `DomainEvent::SessionExpired` and signing the user out of OpenHuman over a *Discord* credentials problem. Three in-flight PRs (tinyhumansai#2292, tinyhumansai#2302, tinyhumansai#2356) all attempt to narrow `is_session_expired_error` itself. This PR takes the orthogonal defense-in-depth approach: a Discord-domain rewrap so even if none of those broader fixes land, the Channels page is safe. Discord API client (src/openhuman/channels/providers/discord/api.rs) - New helper `format_discord_http_error(endpoint, status, body)` that detects 401/403 and emits a user-facing message that: 1. does NOT contain "401" or "unauthorized" as substrings (so `is_session_expired_error` returns false), 2. names the failed endpoint (`list_guilds` / `list_channels` / `get_member_info` / `get_guild_roles` / `get_bot_user` / `get_channel`) so triage can read it from logs, 3. ends with the actionable `Settings → Channels → Discord` remediation path. Spells the HTTP code as "four-oh-one"/"four-oh-three" and uses "rejected"/"forbidden" — both are user-visible equivalents that don't match the classifier. - All 6 `anyhow::bail!` sites in `api.rs` route through the helper: list_bot_guilds_at_base, list_guild_channels_at_base, check_channel_permissions_at_base (3 call sites: get_bot_user, get_member_info, get_guild_roles), get_channel. - 401/403 path deliberately omits the upstream body from the user-facing message — Discord's auth-error bodies often include the literal words "401" and "Unauthorized", which would re-introduce the cascade trigger. The body is still in `tracing::debug!` logs above each call site for triage. Tests - `discord/api_tests.rs`: * Replaced `list_bot_guilds_errors_on_non_success_status` with `list_bot_guilds_rewraps_401_so_global_session_cascade_does_not_fire` — drives a mock Discord that returns the canonical `{"message":"401: Unauthorized","code":0}` body and asserts: - the rewrapped error contains neither "401" nor "unauthorized" - it preserves the `list_guilds` endpoint identifier - it carries the `Settings → Channels → Discord` remediation * Added `list_bot_guilds_5xx_still_carries_raw_status` — non-auth errors keep the verbose status format (they don't trip the session classifier anyway). * Replaced `list_guild_channels_errors_on_non_success_status` with `list_guild_channels_rewraps_403_with_remediation_and_no_session_keywords`. * Updated `check_channel_permissions_errors_on_member_lookup_failure` to assert the new endpoint identifier `get_member_info` + the absence of "401" / "unauthorized" substrings. - `core/jsonrpc_tests.rs`: * Added `is_session_expired_error_skips_discord_rewrap_for_2285` — pins the canonical 401 AND 403 rewrap message bodies and asserts `is_session_expired_error` returns `false` for both, so either-side drift (Discord wording change OR classifier broadening) fails loudly in CI. `cargo test --lib discord::api is_session_expired_error` → 31 + 8 = 39 passed, 0 failed (4 new). `cargo check` clean. `cargo fmt` applied. Out of scope (separate / complementary PRs) - No FE change: `DiscordConfig` already surfaces RPC errors via its existing `setError` path; the new domain-specific message bubbles through unchanged. - No change to `is_session_expired_error` itself: that's owned by the in-flight root-cause PRs (tinyhumansai#2292 / tinyhumansai#2302 / tinyhumansai#2356). This PR is defense-in-depth and remains useful even if all three of those land.
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughThis PR fixes issue ChangesDiscord auth-error rewrap for session-expired fix
Sequence DiagramsequenceDiagram
participant DiscordCardClick as Discord Card Click
participant DiscordAPI as Discord API Call
participant FormatHelper as format_discord_http_error
participant ErrorClassifier as is_session_expired_error
participant UIHandler as UI Error Handler
DiscordCardClick->>DiscordAPI: list_guilds / check_channel_permissions
DiscordAPI-->>FormatHelper: HTTP 401/403 response
FormatHelper->>FormatHelper: Omit "401"/"unauthorized" substrings
FormatHelper->>FormatHelper: Include remediation path
FormatHelper-->>DiscordAPI: Rewrapped error message
DiscordAPI-->>ErrorClassifier: Is this session-expired?
ErrorClassifier-->>ErrorClassifier: Check for 401/unauthorized keywords
ErrorClassifier-->>UIHandler: No match → show Discord settings prompt
UIHandler->>UIHandler: Display recoverable error (not logout)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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}
graycyrus
approved these changes
May 22, 2026
Contributor
graycyrus
left a comment
graycyrus
left a comment
There was a problem hiding this comment.
Looks good, nice work!
fzamel3333-ai
approved these changes
May 22, 2026
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
3 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
Problem
Repro from the issue: a user with a Discord connection clicks the Discord card → modal opens → `DiscordServerChannelPicker` calls `channelConnectionsApi.listDiscordGuilds()` → `openhuman.channels_discord_list_guilds` RPC → Rust `discord_list_guilds` → `api::list_bot_guilds` → Discord REST returns 401 (token rotated / app disabled) → `anyhow::bail!` with the literal "Discord list guilds failed (401 Unauthorized): {body}" → JSON-RPC error result → `is_session_expired_error` matches → `DomainEvent::SessionExpired` published → `SessionExpiredSubscriber` clears the OpenHuman session → user bounced back to Welcome / Google sign-in.
Acceptance criterion 1 ("triggering RPC identified") → `openhuman.channels_discord_list_guilds`, plus its siblings (`channels_discord_list_channels`, `channels_discord_check_permissions`) which call `get_member_info`, `get_guild_roles`, and `get_channel` and share the same failure mode.
Solution
Discord API client
New helper `format_discord_http_error(endpoint, status, body)` in `src/openhuman/channels/providers/discord/api.rs`. On 401 / 403 it emits:
The substrings `"401"` and `"unauthorized"` are both absent; the global `is_session_expired_error` classifier returns `false` for this string. Other 4xx/5xx statuses pass through with the legacy verbose format — they don't match the classifier even verbatim.
All six `anyhow::bail!` sites in `api.rs` route through the helper: `list_bot_guilds_at_base`, `list_guild_channels_at_base`, plus the three inner calls inside `check_channel_permissions_at_base` (`get_bot_user`, `get_member_info`, `get_guild_roles`) and `get_channel`. The 401/403 path deliberately omits the upstream body from the user-facing message — Discord's auth-error bodies typically contain the literal words "401" and "Unauthorized", which would smuggle the cascade trigger back in. The body is still in `tracing::debug!` logs immediately above each call site for triage.
Scope coordination
PRs #2292, #2302, #2356 each take a different swing at narrowing `is_session_expired_error` itself (the root cause). This PR is defense-in-depth at the Discord domain — useful even if all three of those land, because:
Submission Checklist
Impact
Related
AI Authored PR Metadata (required for Codex/Linear PRs)
Linear Issue
Commit & Branch
Validation Run
Validation Blocked
Behavior Changes
Parity Contract
Duplicate / Superseded PR Handling
Summary by CodeRabbit
Release Notes
Bug Fixes
Tests