fix(observability): demote ollama daemon-unreachable to expected error (TAURI-RUST-B / #2921)#2923
Conversation
…r (TAURI-RUST-B) The ollama health-gate in memory_store/factories.rs called `report_error_message` directly, which bypasses the `expected_error_kind` classifier and fires `sentry::capture_message(..., Level::Error)` for every process restart where the user has Ollama opted-in but not running. Switch to `report_error_or_expected` so the classifier runs first. The wire shape "ollama embeddings opted-in but daemon unreachable at ..." already matches `is_ollama_user_config_rejection` → `ExpectedErrorKind::ProviderUserState`, so it is demoted to a warn breadcrumb and never captured as a Sentry error. Also adds a `tauri_rust_b_wire_shape_classifies_as_expected` regression test in the factories module to pin this classification for future refactors. Closes tinyhumansai#2921
|
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 (1)
📝 WalkthroughWalkthroughThe PR fixes the Ollama-unreachable health gate to report its fallback condition through ChangesOllama Health Gate Wire-Shape Classification
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 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. ✨ Finishing Touches⚔️ Resolve merge conflicts
Comment |
oxoxDev
left a comment
There was a problem hiding this comment.
LGTM — good root-cause fix. The gate was calling report_error_message directly, bypassing the classifier and firing capture_message(Level::Error) unconditionally. Routing through report_error_or_expected lets expected_error_kind demote the daemon-unreachable shape to ProviderUserState (warn breadcrumb). Test pins the exact wire shape. CI green. Approving.
…hable-sentry Resolve conflict in src/openhuman/memory_store/factories.rs — both sides used `report_error_or_expected`, only the comment wording differed; kept the PR's original comment text which references the TAURI-RUST-B root cause.
oxoxDev
left a comment
There was a problem hiding this comment.
Re-approve — my earlier approval was auto-dismissed when the branch was updated. The only change since is the chore: merge upstream/main commit (29aad60); the fix itself is unchanged.
Still LGTM: routing the ollama daemon-unreachable health-gate message through report_error_or_expected (instead of report_error_message directly) lets expected_error_kind demote it to a warn breadcrumb → no Sentry error event. Root-cause fix for TAURI-RUST-B, with a test pinning the exact wire shape to ProviderUserState.
|
Heads-up: the |
Summary
report_ollama_health_gate_onceinsrc/openhuman/memory_store/factories.rscalledreport_error_messagedirectly, bypassing theexpected_error_kindclassifier. This firedsentry::capture_message(..., Level::Error)unconditionally whenever a user has Ollama opted-in for embeddings but the daemon is not running — producing 472 events that are pure user-config noise, not bugs.report_error_messagewithreport_error_or_expected, which runs the classifier first. The wire shape"ollama embeddings opted-in but daemon unreachable at …"already matchesis_ollama_user_config_rejection→ExpectedErrorKind::ProviderUserState, so it is demoted to a warn breadcrumb and never fires as a Sentry error.tauri_rust_b_wire_shape_classifies_as_expectedinfactories.rstests to pin the classification and guard against future regressions.Sentry issue: TAURI-RUST-B (issue 65, 472 events, multi-release)
Test plan
GGML_NATIVE=OFF cargo check --manifest-path Cargo.tomlpassestauri_rust_b_wire_shape_classifies_as_expectedtest added and verified logically against the classifiergx_wire_shape_classifies_as_provider_user_statetest inollama_tests.rsalready covers the same wire shape from the Sentry sideSummary by CodeRabbit
Bug Fixes
Tests