Skip to content

fix(observability): classify list_models 404 as ProviderUserState (Sentry TAURI-RUST-YJ)#2873

Merged
M3gA-Mind merged 1 commit into
tinyhumansai:mainfrom
M3gA-Mind:fix/observability-list-models-404-user-config
May 28, 2026
Merged

fix(observability): classify list_models 404 as ProviderUserState (Sentry TAURI-RUST-YJ)#2873
M3gA-Mind merged 1 commit into
tinyhumansai:mainfrom
M3gA-Mind:fix/observability-list-models-404-user-config

Conversation

@M3gA-Mind
Copy link
Copy Markdown
Contributor

@M3gA-Mind M3gA-Mind commented May 28, 2026

Summary

Rebased version of #2793 by @CodeGhost21 on the latest upstream/main. The original PR's CI failed because a new test (classifies_embedding_api_invalid_token_401_as_session_expired) was added to main via #2869 after the branch was cut, and the stale base couldn't pass it.

This branch is identical in intent to #2793 — only rebased to include the session-expired ordering fix from main (PR #2786/#2869).

No functional changes from #2793: the single new branch in is_provider_user_state_message and its two tests are preserved exactly.

  • inference/provider/ops.rs::list_models probes a user-configured custom-provider's /models endpoint. When the upstream returns 404 (wrong base URL, model-only proxy, typo'd path), the wire shape "provider returned 404: {<body>}" was unclassified and filed a Sentry event.
  • New arm: lower.starts_with("provider returned 404")ProviderUserState.
  • 404 only — sibling 4xx/5xx from the same emit site stay actionable (401/403 BYO-key auth, 400 client-shape bugs, 429/5xx transient).

Closes #2793.

Test plan

  • classifies_list_models_404_as_provider_user_state — pins verbatim Sentry payload + 3 body-shape variants
  • does_not_classify_non_404_list_models_failures_as_user_state — exercises every sibling status code
  • All 9991 pre-existing core tests pass (including the new classifies_embedding_api_invalid_token_401_as_session_expired from main)
  • Diff coverage ≥ 80%

Summary by CodeRabbit

Bug Fixes

  • Provider 404 errors during model discovery are now correctly classified as expected behavior, reducing false error reporting in observability logs.

Review Change Stack

`inference/provider/ops.rs::list_models` probes a user-configured
custom-provider's `/models` endpoint. When the upstream returns 404
because the user pointed the base URL at something that doesn't host
a `/models` listing (wrong base, model-only proxy, typo'd path), the
client emits:

    "provider returned 404: {<upstream body>}"

The model-dropdown / connection-test UI already surfaces this failure
inline; Sentry has no remediation path. Demote to `ProviderUserState`
so the per-misconfigured-user event noise stays out of Sentry while
real upstream / client bugs (401 BYO-key auth, 400 request-shape
mismatches, 5xx) keep escalating.

Anchor: `lower.starts_with("provider returned 404")` — the
`"provider returned NNN: "` prefix is emitted ONLY from this one
site (verified via grep across `src/`), so the prefix alone is a
sufficient anchor. **404 only**:

  - 401 / 403 → BYO-key auth wall, must stay actionable (tinyhumansai#2286
    contract preserved).
  - 400      → typically a client-shape bug worth investigating.
  - 429 / 5xx → transient / server faults, retried at provider layer.

Targets Sentry OPENHUMAN-TAURI-YJ (issue 1207): 4 events between
2026-05-19 and 2026-05-27 across v0.54.0 and v0.56.0, all
`domain=rpc method=openhuman.inference_list_models`.

Tests pin the verbatim Sentry payload, three additional body-shape
variants (FastAPI `{"detail":...}`, bare HTML, post-`truncate_with_ellipsis`
clipped body), and a discrimination guard exercising every sibling
4xx / 5xx code from the same emit site to confirm only 404 demotes.
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 28, 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: d5618912-63e6-4b68-bde6-27b3ccdd8c89

📥 Commits

Reviewing files that changed from the base of the PR and between 94f8e4e and b272131.

📒 Files selected for processing (1)
  • src/core/observability.rs

📝 Walkthrough

Walkthrough

This PR adds a single classification rule to demote provider returned 404 messages from custom-provider /models probes to ExpectedErrorKind::ProviderUserState, preventing user-misconfiguration noise from escalating to Sentry while preserving alerts for actual upstream auth or server faults.

Changes

Provider 404 observability classification

Layer / File(s) Summary
Provider 404 classification and test coverage
src/core/observability.rs
New branch in is_provider_user_state_message matches messages starting with provider returned 404 and returns true. Unit tests verify that 404 variants classify as ProviderUserState and that other status codes from the same emit site (401, 403, 400, 429, 500, 503) do not demote.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

Possibly related PRs

  • tinyhumansai/openhuman#2090: Both update is_provider_user_state_message in observability.rs to classify provider error messages as ExpectedErrorKind::ProviderUserState.
  • tinyhumansai/openhuman#2783: Both adjust observability for list_models failures by demoting provider/user-state "unknown provider" conditions from error-level reporting.
  • tinyhumansai/openhuman#2239: Both extend expected-error classification/demotion in observability.rs for specific provider error message patterns.

Suggested labels

bug, rust-core, sentry-traced-bug

Suggested reviewers

  • graycyrus
  • senamakel

🐰 A 404 hops away, the user's config's astray,
We demote to logs, keep Sentry at bay,
But auth walls stay high—401's still fly,
Error-wise and user-kind—watch and try! 🔍

🚥 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 clearly and specifically describes the main change: classifying list_models 404 responses as ProviderUserState in observability code to reduce Sentry noise.
Linked Issues check ✅ Passed The PR fully implements all requirements from #2793: adds 404 classification branch, includes comprehensive tests for positive and negative cases, preserves non-404 status handling, maintains BYO-key 401 contract, and achieves ≥80% diff coverage.
Out of Scope Changes check ✅ Passed All changes are directly scoped to the stated objective: a single conditional branch in is_provider_user_state_message and two related unit tests, with no unrelated modifications.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

✏️ 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 May 28, 2026
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 28, 2026

Actionable comments posted: 0

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.

2 participants