revert: remove all Sentry error suppression (12 PRs)#2959
Open
graycyrus wants to merge 1 commit into
Open
Conversation
…tinyhumansai#2850 tinyhumansai#2899 tinyhumansai#2906 tinyhumansai#2915 tinyhumansai#2923 tinyhumansai#2924 tinyhumansai#2928 tinyhumansai#2930 tinyhumansai#2934 tinyhumansai#2937 tinyhumansai#2939 These PRs suppressed/demoted errors instead of fixing root causes. Errors will now properly fire Sentry events again so they can be tracked and fixed with proper root-cause solutions.
Contributor
📝 WalkthroughWalkthroughThis PR removes several observability error classification pathways and Sentry suppression filters. Core changes: ChangesError classification pathway removal and Sentry filter simplification
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 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
added
rust-core
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/openhuman/inference/ops.rs (1)
182-182: ⚡ Quick winAdd correlation fields to the error log.
Include
workloadandprovideron this error event to keep failure logs queryable with the same context as the start event.As per coding guidelines: “Prefix debug logs with stable, grep-friendly identifiers … and include correlation fields such as request IDs, method names, or entity IDs.”Suggested diff
- Err(err) => error!(error = %err, "{LOG_PREFIX} test_provider_model:error"), + Err(err) => error!( + workload, + provider, + error = %err, + "{LOG_PREFIX} test_provider_model:error" + ),🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/openhuman/inference/ops.rs` at line 182, The error log at the Err branch currently only logs the error; update the error! invocation in the Err(err) arm to include the correlation fields workload and provider (e.g. error!(workload = %workload, provider = %provider, error = %err, "{LOG_PREFIX} test_provider_model:error")), keeping the existing LOG_PREFIX message and error formatting; ensure workload and provider are in scope or passed into the function containing this Err(err) match arm (refer to the Err branch where error!(...) is called).
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/openhuman/inference/provider/config_rejection.rs`:
- Around line 170-192: Run the project's formatter and commit the changes so CI
passes: execute pnpm format (which runs rustfmt/prettier) and ensure cargo fmt
has formatted Rust files; then add and commit the updated file containing the
string entries "thinking mode must be passed back" and "requires a subscription,
upgrade for access" so pnpm format:check and cargo fmt --check succeed in CI.
---
Nitpick comments:
In `@src/openhuman/inference/ops.rs`:
- Line 182: The error log at the Err branch currently only logs the error;
update the error! invocation in the Err(err) arm to include the correlation
fields workload and provider (e.g. error!(workload = %workload, provider =
%provider, error = %err, "{LOG_PREFIX} test_provider_model:error")), keeping the
existing LOG_PREFIX message and error formatting; ensure workload and provider
are in scope or passed into the function containing this Err(err) match arm
(refer to the Err branch where error!(...) is called).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 07dd0604-9948-4940-a388-c4c8b3baedc0
📒 Files selected for processing (8)
src/core/observability.rssrc/main.rssrc/openhuman/inference/ops.rssrc/openhuman/inference/ops_tests.rssrc/openhuman/inference/provider/config_rejection.rssrc/openhuman/inference/provider/ops.rssrc/openhuman/memory_store/factories.rssrc/openhuman/subconscious/store.rs
💤 Files with no reviewable changes (2)
- src/openhuman/inference/ops_tests.rs
- src/main.rs
Comment on lines
+170
to
192
| // thinking-mode model (DeepSeek-R1 / Moonshot K2-thinking on | ||
| // `provider=cloud` custom_openai) rejects a follow-up turn that | ||
| // doesn't echo the prior assistant's `reasoning_content` field. | ||
| // Body shape (backtick-quoted JSON literal in the upstream body): | ||
| // `{"error":{"message":"The `reasoning_content` in the thinking | ||
| // mode must be passed back to the API.",...}}`. The | ||
| // provider-contract gap is on our side, but until the thinking- | ||
| // mode round-tripping ships in the inference layer, every affected | ||
| // turn fires a fresh Sentry event — and the UI already surfaces | ||
| // the actionable error to the user. Anchor on the unique | ||
| // `thinking mode must be passed back` substring so the match | ||
| // doesn't depend on the upstream's backtick-quoting around | ||
| // `reasoning_content` (some provider versions ship without them). | ||
| "thinking mode must be passed back", | ||
| // TAURI-RUST-4XK (~649 events) — Ollama Cloud subscription gate. | ||
| // Body: `{"error":"this model requires a subscription, upgrade for | ||
| // access: https://ollama.com/upgrade (ref: <uuid>)"}` on a 403 | ||
| // Forbidden from `compatible::OpenAiCompatibleProvider` with | ||
| // `name = "ollama"`. User-state: the model picked in Settings is | ||
| // a paid-tier Ollama Cloud model the user's account doesn't | ||
| // cover. The UI surfaces an actionable upgrade link in the | ||
| // remediation message itself. | ||
| "requires a subscription, upgrade for access", |
Contributor
There was a problem hiding this comment.
Run formatter to unblock CI.
cargo fmt --check and pnpm format:check are failing for this file, so this PR is currently blocked. Please run pnpm format and commit the formatter output.
As per coding guidelines **/*.{ts,tsx,rs,json,js}: “Run pnpm format … before committing; pnpm format:check … must pass in CI.”
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/openhuman/inference/provider/config_rejection.rs` around lines 170 - 192,
Run the project's formatter and commit the changes so CI passes: execute pnpm
format (which runs rustfmt/prettier) and ensure cargo fmt has formatted Rust
files; then add and commit the updated file containing the string entries
"thinking mode must be passed back" and "requires a subscription, upgrade for
access" so pnpm format:check and cargo fmt --check succeed in CI.
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
2 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
Reverts all Sentry error suppression code from 12 PRs that hid bugs instead of fixing root causes.
Affected PRs: #2813, #2850, #2899, #2906, #2915, #2923, #2924, #2928, #2930, #2934, #2937, #2939
Errors will resume firing Sentry events. Each will get a proper root-cause fix in separate PRs.
Why
Suppressing errors hides bugs. The correct approach is to fix the root cause — add backoff, refresh tokens, handle gracefully — not silence the symptom.
Test plan
cargo checkpassesSummary by CodeRabbit
Bug Fixes
Chores