fix(observability): classify OpenHuman/Embedding/streaming backend 'Invalid token' 401 as SessionExpired (TAURI-RUST-4P0 + 4K5 + 1EE)#2786
Conversation
… SessionExpired
The OpenHuman backend rejects an expired/revoked JWT with the envelope
`{"success":false,"error":"Invalid token"}` (vs. the explicit
`"Session expired. Please log in again."` body that the existing
classifier already catches). Same emit site
(`providers::ops::api_error` → `web_channel.run_chat_task`), same
wrapping, same expected user state — just a different body substring
chosen by the backend's JWT-validity branch.
Issue tinyhumansai#2286 deliberately stopped matching bare `"Invalid token"` as
session-expired because that string also surfaces from Discord /
OAuth provider rejections, which are actionable scoped errors that
must reach Sentry. We preserve that contract with a conjunctive
matcher: BOTH the OpenHuman-scoped `"OpenHuman API error (401"`
prefix AND the envelope-shaped `"\"error\":\"Invalid token\""` must
be present.
tinyhumansai#2286 cases still route to Sentry (verified by the existing
`does_not_classify_byo_key_provider_401_as_session_expired` test
staying green):
- `"Invalid token"` → None ✓
- `"got an invalid token here"` → None ✓
- `"OpenAI API error (401 Unauthorized): invalid_api_key"` → None ✓
- `"Anthropic API error (401 Unauthorized): ..."` → None ✓
Targets Sentry OPENHUMAN-TAURI-4P0 (issue 5332): low volume so far
(1 event) but the wire shape is durable — every OpenHuman user with
a stale JWT will hit this on the next agent turn, so quietly
demoting it to a `warn!` log keeps the noise from compounding.
📝 WalkthroughWalkthroughAdds conjunctive matching and docs in ChangesOpenHuman 401 Invalid Token Classification
Sequence Diagram(s)(omitted — change is a targeted classifier update and tests; no multi-component sequential flow requires visualization) Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
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 |
graycyrus
left a comment
There was a problem hiding this comment.
@CodeGhost21 hey! the code looks good to me — the conjunctive anchor approach is exactly right here. Requiring both the "OpenHuman API error (401" prefix and the envelope-shaped "\"error\":\"Invalid token\"" body together is the correct way to preserve the #2286 contract while catching this specific backend branch, and the two new test cases pin both the verbatim Sentry wire shape and the unwrapped emit shape cleanly.
CI still has a few checks pending (Windows E2E, core image build, Rust core coverage). Once those come back green I'll come back and approve this. Let me know if anything comes up in the meantime.
…sionExpired (TAURI-RUST-4K5)
TAURI-RUST-4K5 (~118 events, escalating on 0.56.0,
domain=embeddings operation=openai_embed status=401) carries the same
OpenHuman backend `{"success":false,"error":"Invalid token"}` envelope
as 4P0, but the embedding client at
`src/openhuman/embeddings/openai.rs:139` wraps it with the
`"Embedding API error"` prefix instead of `"OpenHuman API error"`. The
breadcrumb `[scheduler_gate] signed_out false -> true` immediately
preceding the 401 in the event payload confirms it's the same
session-expired cause, just emitted at the embedding layer.
The conjunctive `"OpenHuman API error (401"` anchor added in the
previous commit catches the chat-API path; this commit adds a parallel
`"Embedding API error (401"` anchor so the embedding path also routes
to SessionExpired. The envelope-shaped `"\"error\":\"Invalid token\""`
gate stays the same, so third-party BYO-key embedding 401s (OpenAI /
Voyage / Cohere rejecting the user's own API key) continue to escalate
as actionable misconfiguration — covered by the new
`does_not_classify_embedding_byo_key_401_as_session_expired`
polarity guard.
## Test plan
- [x] `cargo test classifies_embedding_api_invalid_token` — passes (new)
- [x] `cargo test does_not_classify_embedding_byo_key` — passes (new polarity guard)
- [x] `cargo test classifies_openhuman_invalid_token` — passes (4P0, unchanged)
- [x] `cargo test does_not_classify_byo_key_provider` — passes (tinyhumansai#2286 BYO-key contract preserved)
- [x] `cargo test core::observability` — 91 tests pass, 0 regressions
- [x] `cargo check --bin openhuman-core` — passes
- [x] `cargo fmt --check` — clean
…as SessionExpired (TAURI-RUST-1EE)
Third emit-site prefix for the same OpenHuman backend
`{"success":false,"error":"Invalid token"}` 401 envelope this PR
already classifies for non-streaming chat (4P0) and embeddings (4K5).
TAURI-RUST-1EE (Sentry issue 1807, 110 events, 109 on
openhuman@0.56.0, domain=llm_provider operation=streaming_chat
status=401 provider=OpenHuman) is the streaming-chat path: the body is
wrapped at `inference/provider/compatible.rs:949` with the
`"OpenHuman streaming API error"` prefix. The `streaming` token between
`OpenHuman` and `API error` means the 4P0 anchor
(`"OpenHuman API error (401"`) does not match it, so it needs its own
prefix arm.
Same conjunctive-anchor pattern as the existing arms — the
OpenHuman-scoped streaming prefix gates the match so a third-party
BYO-key streaming 401 (`"OpenAI streaming API error (401):
invalid_api_key"`) stays actionable in Sentry.
Tests:
- `classifies_openhuman_streaming_invalid_token_401_as_session_expired`
— verbatim 1EE wire shape (direct + caller-wrapped).
- `does_not_classify_streaming_byo_key_401_as_session_expired` —
polarity guard for the streaming prefix.
## Test plan
- [x] `cargo test classifies_openhuman_streaming_invalid_token` — passes
- [x] `cargo test does_not_classify_streaming_byo_key` — passes (polarity)
- [x] `cargo test core::observability` — 93 tests pass, 0 regressions
- [x] `cargo check --bin openhuman-core` — passes
- [x] `cargo fmt --check` — clean
graycyrus
left a comment
There was a problem hiding this comment.
@CodeGhost21 the two follow-on commits look clean — the 4K5 (Embedding API) and 1EE (streaming) arms follow the same conjunctive-anchor pattern as the original 4P0 arm, the polarity guards for BYO-key embedding and streaming 401s are solid, and the test coverage (direct + caller-wrapped shapes for each arm) is thorough.
One CI check is still pending (Windows / Appium Chromium E2E). Once that clears, I'll come back and approve this.
`reliable::format_failure_aggregate` (no-configured-fallbacks branch) wraps every exhausted `reliable_chat_with_system` turn with: "The model `<name>` may not be available on your provider. Configure a fallback chain via `reliability.model_fallbacks` in your OpenHuman config, or change your default model in Settings → AI.\n\nAll providers/models failed. Attempts:\n…" The aggregate fires once per turn regardless of the underlying per- attempt cause (401 auth wall, unknown model, region block, rate- limit cliff). All of those are user-actionable: pick a different model, fix the credential, or configure fallbacks — the message literally tells the user how. Sentry has no remediation path that the per-attempt body classifiers haven't already covered at the lower layer (`SessionExpired`, `BudgetExhausted`, config_rejection siblings, etc.). Adds `"reliability.model_fallbacks"` to the `is_provider_config_rejection_message` PHRASES list. The string is uniquely OpenHuman — that config path is rendered into an error message only from `reliable.rs:332-334`, verified via grep across `src/`. A stray "may not be available" log line elsewhere will not collide. The configured-fallbacks aggregate branch (just `"All providers/models failed. Attempts:\n…"`) is intentionally NOT matched — the user has already engaged with the knob, so per- attempt classifiers should drive the per-body decision. Targets Sentry OPENHUMAN-TAURI-4JS (issue 5215): 25 events on v0.56.0 in 5h, `domain=llm_provider operation=reliable_chat_with_system failure=all_exhausted`. The current 25-event sample carries an "Invalid token" 401 underlying cause (body-equivalent to the already-open PR tinyhumansai#2786, which would also demote this aggregate via the body substring match). This PR catches the aggregate at the emit-site level so future all_exhausted scenarios with non-401 underlying causes (model name typo, region block, …) demote the same way. Tests pin the verbatim 4JS payload + three underlying-cause variants (unknown-model upstream, region block, bare aggregate) + a negative guard confirming the configured-fallbacks branch does NOT classify on the aggregate phrase alone.
oxoxDev
left a comment
There was a problem hiding this comment.
Walkthrough
Extends is_session_expired_message in src/core/observability.rs with 3 new conjunctive-anchor arms — each gating on a distinct emit-site prefix ("OpenHuman API error (401" / "Embedding API error (401" / "OpenHuman streaming API error (401") AND the envelope-shaped "\"error\":\"Invalid token\"". Drops self-hosted Sentry issues TAURI-RUST-4P0 (non-streaming chat) + 4K5 (~118 events, embedding openai.rs:139) + 1EE (~110 events, streaming compatible.rs:949). +209/-0, 1 file. 5 new tests. All CI green.
Verified
- Three-prefix split is real:
"streaming"token betweenOpenHumanandAPI errorbreaks the 4P0 anchor on 1EE, so each path genuinely needs its own arm — confirmed in author doc + test ✓ - Polarity arithmetic: BYO 401s carry
invalid_api_key/authentication_errorpayloads — they hit the prefix anchor but the envelope anchor"error":"Invalid token"fails → no match → still reach Sentry. Confirmed for OpenAI/Anthropic/Voyage/Cohere shapes ✓ - Pre-existing
does_not_classify_byo_key_provider_401_as_session_expired(#2286 contract) still in place; both new polarity guards (embedding+streamingBYO) extend the same contract per emit-site ✓ - Wire-shape envelope tolerant of optional whitespace:
{"success":false,"error":"Invalid token"}and{"success":false, "error":"Invalid token"}both substring-match"error":"Invalid token"✓
Nits
- The 3 arms share the same
(<prefix> && envelope)shape. When a 4th emit-site (e.g. tool-call backend, embeddings rerank path) lands, worth hoisting to a small&[(prefix, envelope)]table +.iter().any(...)so each new arm is one tuple line, not 4 lines of boilerplate. Don't block on it. - Envelope match is case-sensitive on
"Invalid token". If backend ever swaps toINVALID_TOKEN/ camelCase / etc., these arms silently fall through. Cheap defense: anchor on the lowercase variant vialower.contains(...)like the existing arms above. Worth a follow-up. - User-pasted JSON in chat that contains exactly
OpenHuman API error (401+"error":"Invalid token"would get silenced. Vanishingly unlikely (the prefix is emit-site-only string), but the prefix-as-gate is the only guard. Flagging only.
Questions
- Did you check whether the
expected_error_kinddispatch path that callsis_session_expired_messageproduces anExpectedErrorKind::SessionExpiredwith the same Breadcrumb-vs-Ignore tier as the existing arms? (Want to make sure the new shapes drop to Ignore at TRACE / DEBUG vs Breadcrumb at INFO/WARN — leak risk perfeedback_devtools_smoke_void_promisestyle breadcrumbs-stick-to-events pattern.) - Self-hosted Sentry — confirm
Closes TAURI-RUST-4P0,Closes TAURI-RUST-4K5,Closes TAURI-RUST-1EEare in the PR body so Phase 7 Step 4.5 self-hosted sweep flips them on merge.
|
@oxoxDev thanks for the thorough review and the approval! Answering your two questions: Q1 — tier / breadcrumb-leak risk. No divergence: classification routes on the kind, not the message shape. Q2 — self-hosted Sentry close directives. Added to the PR body under a new One heads-up for the Phase 7 Step 4.5 sweep: PR #2692's body flagged that the post-merge resolve regex is anchored to Nits — all noted as follow-ups (not in this PR): the |
…adiction on Embedding 401 + Invalid token PRs tinyhumansai#2830 and tinyhumansai#2786 both shipped on main and made contradictory assertions for the SAME wire shape: Embedding API error (401 Unauthorized): {"success":false,"error":"Invalid token"} - tinyhumansai#2830 added `is_embedding_backend_auth_failure` and a test asserting `BackendUserError`. - tinyhumansai#2786 added `classifies_embedding_api_invalid_token_401_as_session_expired` asserting `SessionExpired`. The tinyhumansai#2830 arm runs first in `expected_error_kind`, so the tinyhumansai#2786 test fails in CI on every PR that rebases onto current main (verified on upstream/main @ e83bfd6). Per the doc evidence and breadcrumb context (`[scheduler_gate] signed_out false -> true` immediately preceding the 401), the SessionExpired routing is the correct one — the OpenHuman backend envelope `{"success":false, "error":"Invalid token"}` is the JWT-invalidity branch of the same session-renewal flow as TAURI-RUST-4P0. Disable `is_embedding_backend_auth_failure` (keep the function as a doc breadcrumb so the regression is traceable) and remove the contradicting `classifies_embedding_backend_auth_failure` test. The SessionExpired arm in `is_session_expired_message` (added by tinyhumansai#2786) now catches the wire shape correctly. BYO-key embedding 401s (no OpenHuman envelope) still escalate to Sentry — guarded by `does_not_classify_embedding_byo_key_401_as_session_expired`. Local tests: cargo test core::observability::tests → 117/117 pass. Local repro: `classifies_embedding_api_invalid_token_401_as_session_expired` panicked on pure upstream/main before this commit; passes after. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…sionExpired (TAURI-RUST-4K5 regression) PR tinyhumansai#2786 (commit 14ff92b) added an `is_embedding_backend_auth_failure` matcher that was supposed to classify the TAURI-RUST-4K5 wire shape (`Embedding API error (401 Unauthorized): {"error":"Invalid token"}`) as `SessionExpired` — to align with the 4P0 OpenHuman-backend variant and surface a re-login prompt instead of a Sentry noise bucket. A subsequent merge (tinyhumansai#2830 commit d578b57, 'demote expected-error Sentry buckets across embeddings, provider, memory-store, FS, and thinking-mode wire shapes') landed on a pre-tinyhumansai#2786 base and clobbered the SessionExpired return back to BackendUserError. The `classifies_embedding_api_invalid_token_401_as_session_expired` test that tinyhumansai#2786 shipped therefore fails on every PR rebased onto current `upstream/main`: assertion `left == right` failed: TAURI-RUST-4K5 verbatim wire shape must classify as SessionExpired left: Some(BackendUserError) right: Some(SessionExpired) Restore the intended return value. One-line fix to the `is_embedding_backend_auth_failure` arm in `expected_error_kind`. All observability tests pass. Co-Authored-By: Claude <noreply@anthropic.com>
|
Posted #2869 with the root-cause fix: #2830's merge inadvertently reverted #2786's SessionExpired classification back to BackendUserError, so the |
… so embedding 401 'Invalid token' classifies as SessionExpired (TAURI-RUST-4K5) The merge surfaced a pre-existing main contradiction (tinyhumansai#2786 vs tinyhumansai#2830): the embedding 401 "Invalid token" envelope was shadowed by the broader is_embedding_backend_auth_failure matcher (BackendUserError) before reaching is_session_expired_message. Move the narrowly-anchored session-expired check ahead of the embedding-auth matcher so the parenthesised `Embedding API error (401 …): {"error":"Invalid token"}` shape classifies as SessionExpired; the bare-status shape still falls through to BackendUserError. Mirrors the authoritative fix in tinyhumansai#2867 to unblock this PR's Rust Core Tests.
…ssionExpired
The TAURI-RUST-4K5 wire-shape (`Embedding API error (401 Unauthorized):
{"success":false,"error":"Invalid token"}`) is the OpenHuman backend's
session-expired envelope reaching the embeddings worker, not a backend
4xx bug. PR tinyhumansai#2786 added a SessionExpired classification + test for this
shape but left the obsolete TAURI-RUST-T BackendUserError test in place,
producing a contradiction: two tests assert different classifications for
identical input. On main, `is_embedding_backend_auth_failure` claims the
envelope first (line 392) and short-circuits before SessionExpired (line
335) can match, failing the newer test.
Three coordinated edits:
1. `is_embedding_backend_auth_failure` now skips the OpenHuman-backend
envelope `"\"error\":\"invalid token\""` so BYO-key 401s (no envelope)
still classify as BackendUserError but the 4K5 envelope falls through
to SessionExpired. The function's narrative (third-party BYO-key
rejection) is preserved.
2. `is_session_expired_message` now matches both the parenthesized
(`Embedding API error (401`) and bare-status (`Embedding API error
401`) wire shapes. Both are observed in production per the older
TAURI-RUST-T test.
3. The obsolete `classifies_embedding_backend_auth_failure` test now
asserts SessionExpired for both wire shapes — kept as a regression
guard against `is_embedding_backend_auth_failure` re-claiming the
envelope.
121 observability tests green.
…ssionExpired
The TAURI-RUST-4K5 wire-shape (`Embedding API error (401 Unauthorized):
{"success":false,"error":"Invalid token"}`) is the OpenHuman backend's
session-expired envelope reaching the embeddings worker, not a backend
4xx bug. PR tinyhumansai#2786 added a SessionExpired classification + test for this
shape but left the obsolete TAURI-RUST-T BackendUserError test in place,
producing a contradiction: two tests assert different classifications for
identical input. On main, `is_embedding_backend_auth_failure` claims the
envelope first (line 392) and short-circuits before SessionExpired (line
335) can match, failing the newer test.
Three coordinated edits:
1. `is_embedding_backend_auth_failure` now skips the OpenHuman-backend
envelope `"\"error\":\"invalid token\""` so BYO-key 401s (no envelope)
still classify as BackendUserError but the 4K5 envelope falls through
to SessionExpired. The function's narrative (third-party BYO-key
rejection) is preserved.
2. `is_session_expired_message` now matches both the parenthesized
(`Embedding API error (401`) and bare-status (`Embedding API error
401`) wire shapes. Both are observed in production per the older
TAURI-RUST-T test.
3. The obsolete `classifies_embedding_backend_auth_failure` test now
asserts SessionExpired for both wire shapes — kept as a regression
guard against `is_embedding_backend_auth_failure` re-claiming the
envelope.
121 observability tests green.
Summary
Extend
is_session_expired_messageinsrc/core/observability.rsto recognise the OpenHuman backend's{"success":false,"error":"Invalid token"}401 envelope as a session-expired condition (previously only the explicit"Session expired. Please log in again."body was recognised). The same upstream cause — backend rejecting the bearer JWT as invalid — surfaces under three emit-site prefixes depending on the call path, each producing its own Sentry fingerprint:OpenHuman API error (401 …)run_chat_task)Embedding API error (401 …)embeddings/openai.rs:139OpenHuman streaming API error (401 …)compatible.rs:949)All three are typically preceded by a
[scheduler_gate] signed_out false -> truebreadcrumb. The UI already drives reauth via theSessionExpiredevent-domain path; this stops the noise leaking into Sentry as a code bug.Why three arms, not one
The matcher uses conjunctive anchors per arm —
"<emit-site prefix> (401"AND the envelope-shaped"\"error\":\"Invalid token\"". Anchoring on each OpenHuman-scoped prefix is what preserves the #2286 BYO-key contract:"OpenAI API error (401 Unauthorized): invalid_api_key"(user's own OpenAI key revoked) must NOT match."OpenAI streaming API error (401): invalid_api_key"and"Embedding API error (401): invalid_api_key"(BYO embedding/streaming key revoked) must NOT match either.Each of those is pinned by a dedicated
does_not_classify_*_byo_key_401_as_session_expiredpolarity guard. A single broad "any 401 + invalid token" matcher would silence all of them, so each OpenHuman-backend emit-site prefix gets its own prefix-gated arm. Thestreamingtoken in 1EE specifically means the 4P0 anchor can't cover it.Tests added (
observability::tests)classifies_openhuman_invalid_token_401_as_session_expired— 4P0 (wrapped + unwrapped).classifies_embedding_api_invalid_token_401_as_session_expired— 4K5 (direct + wrapped).classifies_openhuman_streaming_invalid_token_401_as_session_expired— 1EE (direct + wrapped).does_not_classify_embedding_byo_key_401_as_session_expired— embedding polarity guard.does_not_classify_streaming_byo_key_401_as_session_expired— streaming polarity guard.does_not_classify_byo_key_provider_401_as_session_expired— SessionExpired clears the session for unrelated backend 401s #2286 chat-path contract, still green.Test plan
cargo test classifies_openhuman_invalid_token— passes (4P0)cargo test classifies_embedding_api_invalid_token— passes (4K5)cargo test classifies_openhuman_streaming_invalid_token— passes (1EE)cargo test does_not_classify_embedding_byo_key/does_not_classify_streaming_byo_key/does_not_classify_byo_key_provider— pass (polarity)cargo test core::observability— 93 tests pass, 0 regressionscargo check --bin openhuman-core— passescargo fmt --check— cleanRelated
Self-hosted Sentry (
sentry.tinyhumans.ai,tauri-rustproject) — no GitHub issue:Closes TAURI-RUST-4P0
Closes TAURI-RUST-4K5
Closes TAURI-RUST-1EE