fix(observability): classify embedding API 401 'Invalid token' as SessionExpired (TAURI-RUST-4K5 regression)#2869
Conversation
…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>
|
Warning Review limit reached
More reviews will be available in 22 minutes and 53 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
Comment |
graycyrus
left a comment
There was a problem hiding this comment.
@sanil-23 hey! the code looks good to me — clean regression fix with the right behavior restored. CI is still pending, so i'll hold off on approving until those are green. once everything passes, i'll come back and approve this. let me know if you need anything in the meantime.
What this fixes: The merge of #2830 onto a pre-#2786 base silently reverted is_embedding_backend_auth_failure from SessionExpired back to BackendUserError, breaking the 4K5 wire-shape test added by #2786. This PR restores the correct classification and reconciles the older test to match the established contract.
Diff: One classifier flip in expected_error_kind + one test update. Both tests now consistently assert SessionExpired for the 401/"Invalid token" wire shapes. The change is minimal, targeted, and well-documented.
Summary
Fix the
core::observability::tests::classifies_embedding_api_invalid_token_401_as_session_expiredtest that's been failing on every PR rebased onto currentmain.Root cause
PR #2786 (commit
14ff92b2) introduced theis_embedding_backend_auth_failurematcher and the 4K5 wire-shape test, both targetingSessionExpiredso the FE re-login prompt fires (alongside the 4P0 OpenHuman-backend variant).PR #2830 (commit
d578b57e, "demote expected-error Sentry buckets across embeddings, provider, memory-store, FS, and thinking-mode wire shapes") landed shortly after on a pre-#2786 base. Its merge:is_embedding_backend_auth_failurematcher arm inexpected_error_kind, butSome(ExpectedErrorKind::SessionExpired)toSome(ExpectedErrorKind::BackendUserError), andclassifies_embedding_backend_auth_failuretest (also assertingBackendUserError) survived the merge unchanged, masking the regression in fix(observability): demote expected-error Sentry buckets across embeddings, provider, memory-store, FS, and thinking-mode wire shapes #2830's own CI.The result:
classifies_embedding_api_invalid_token_401_as_session_expired(added by #2786) panics on currentmain:Verified reproducer:
git checkout upstream/main && cargo test --lib core::observability::tests::classifies_embedding_api_invalid_token_401_as_session_expiredfails.What this PR changes
src/core/observability.rsonly:is_embedding_backend_auth_failurearm's return fromBackendUserErrorback toSessionExpired, the intent introduced by fix(observability): classify OpenHuman/Embedding/streaming backend 'Invalid token' 401 as SessionExpired (TAURI-RUST-4P0 + 4K5 + 1EE) #2786.classifies_embedding_backend_auth_failure(TAURI-RUST-T) was asserting the pre-fix(observability): classify OpenHuman/Embedding/streaming backend 'Invalid token' 401 as SessionExpired (TAURI-RUST-4P0 + 4K5 + 1EE) #2786 behavior. Update it to expectSessionExpired, matching the design contract. Updated the doc-comment to cross-reference fix(observability): classify OpenHuman/Embedding/streaming backend 'Invalid token' 401 as SessionExpired (TAURI-RUST-4P0 + 4K5 + 1EE) #2786 and the companion 4K5 test.Both tests share the same wire shapes (
Embedding API error 401 Unauthorized:andEmbedding API error (401 Unauthorized):+"error":"Invalid token"); now they classify consistently.Impact
Unblocks every PR rebasing onto current
main. Confirmed downstream:Test plan
core::observability::tests::*pass locally (121 passed; 0 failed).🤖 Generated with Claude Code