Skip to content

fix(agent): handle config rejection in streaming_chat path#2346

Merged
M3gA-Mind merged 4 commits into
tinyhumansai:mainfrom
YellowSnnowmann:fix/sentry-nj-config-rejection-streaming-paths
May 21, 2026
Merged

fix(agent): handle config rejection in streaming_chat path#2346
M3gA-Mind merged 4 commits into
tinyhumansai:mainfrom
YellowSnnowmann:fix/sentry-nj-config-rejection-streaming-paths

Conversation

@YellowSnnowmann
Copy link
Copy Markdown
Contributor

@YellowSnnowmann YellowSnnowmann commented May 20, 2026

Summary

  • In OpenAiCompatibleProvider, route HTTP errors that match a known provider-config rejection (e.g. invalid/unauthorized API key, disabled model, billing block) to log_provider_config_rejection instead of observability::report_error.
  • Applies the new branch consistently across all five request paths in the provider: responses_api, streaming_chat, chat_completions, native_chat, and stream_chat.
  • Suppresses the recurring Sentry issue OPENHUMAN-TAURI-NJ, which was being driven by user-config rejections being misclassified as provider bugs.
  • No behavior change for genuine provider failures — those still flow through should_report_provider_http_failurereport_error as before.
  • Second commit is a tiny formatting touch-up to the same file.

Problem

  • OpenAiCompatibleProvider was funneling every non-401/429 HTTP error into observability::report_error, including HTTP responses that are expected when the user's provider config is wrong (bad key, wrong base URL, unsupported model, account suspended, etc.).
  • Result: Sentry issue OPENHUMAN-TAURI-NJ accumulated a high-volume stream of "errors" that were actually user-actionable config problems, not bugs. Real provider regressions got buried in the noise.
  • The condition was already detectable — is_provider_config_rejection_http exists in the parent module — but only one arm (or none, depending on the path) was consulting it. The five request entry points had drifted out of sync.

Solution

  • Add a new else if super::is_provider_config_rejection_http(status, self.name.as_str(), &error_body) branch to each of the five request paths in compatible.rs, sitting between the auth/rate-limit handling and the generic should_report_provider_http_failure fallback.
  • On match, call super::log_provider_config_rejection(<arm_name>, provider, model, status) so the rejection is recorded as a structured log line (with the originating arm as the first argument, e.g. "responses_api", "streaming_chat", …) instead of being uploaded to Sentry.
  • All five arms now share the same classification ladder: auth → rate-limit → config rejection → generic reportable failure. This keeps the Sentry signal clean and makes the per-arm log prefix useful for triage.
  • Tradeoff: rejections no longer surface in Sentry. That is the intent — they belong in logs/UX, not in the bug tracker — but it does mean we rely on the logger for visibility going forward.

Submission Checklist

If a section does not apply to this change, mark the item as N/A with a one-line reason. Do not delete items.

  • Tests added or updated (happy path + at least one failure / edge case) per Testing Strategy
  • Diff coverage ≥ 80% — changed lines (Vitest + cargo-llvm-cov merged via diff-cover) meet the gate enforced by .github/workflows/coverage.yml. Run pnpm test:coverage and pnpm test:rust locally; PRs below 80% on changed lines will not merge.
  • Coverage matrix updated — N/A: error-classification refinement, no feature row added/removed/renamed.
  • All affected feature IDs from the matrix are listed in the PR description under ## RelatedN/A: no matrix row touched.
  • No new external network dependencies introduced (mock backend used per Testing Strategy)
  • Manual smoke checklist updated if this touches release-cut surfaces (docs/RELEASE-MANUAL-SMOKE.md) — N/A: routing fix on an existing error path; no new smoke step.
  • Linked issue closed via Closes #NNN in the ## Related section

Impact

  • Runtime/platform: desktop (Tauri host + in-process core). Affects every code path that talks to an OpenAI-compatible provider — i.e. all five request shapes in OpenAiCompatibleProvider. No mobile/web/CLI surface touched directly.
  • Performance: negligible — one extra match arm + helper call on the error path. Hot path (success) is unchanged.
  • Security: no change. log_provider_config_rejection follows the same redaction rules as the existing log helpers; no new fields, no PII leakage.
  • Observability: Sentry volume on OPENHUMAN-TAURI-NJ should drop materially. In exchange, expect a corresponding rise in structured [inference][config_rejection] log lines, tagged with the originating arm so triage can tell which request shape tripped.
  • Migration / compatibility: none. Provider trait signatures unchanged; RPC surface unchanged.

Related

Summary by CodeRabbit

  • Bug Fixes
    • Improved handling and logging for provider configuration rejection HTTP responses to better surface and distinguish configuration-related failures.
  • Tests
    • Added a test ensuring streaming requests propagate provider config rejection errors and do not report events to error-monitoring.

Review Change Stack

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 20, 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: bcf561b1-b8b2-496d-b40f-423d3137be6c

📥 Commits

Reviewing files that changed from the base of the PR and between 41d3612 and 72dda7d.

📒 Files selected for processing (1)
  • src/openhuman/inference/provider/compatible_tests.rs

📝 Walkthrough

Walkthrough

Adds detection and logging for provider configuration rejection HTTP responses across five OpenAI-compatible request/stream paths and a test that verifies a 400 provider-config-rejection from a streaming provider is returned as an error without producing Sentry events.

Changes

Provider Config Rejection Error Classification

Layer / File(s) Summary
Provider config rejection detection and logging across request paths
src/openhuman/inference/provider/compatible.rs
Inserted is_provider_config_rejection_http checks and log_provider_config_rejection(provider, model, status) branches into five non-success response handlers (chat_via_responses, stream_native_chat, chat_with_system, chat, stream_chat_with_system).
Sentry assertion test for streaming provider config rejection
src/openhuman/inference/provider/compatible_tests.rs
Adds test-only imports and a #[tokio::test] that uses WireMock and Sentry TestTransport to assert a 400 provider-config-rejection from stream_native_chat returns a streaming API error and yields no Sentry events.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

Suggested labels

working

Suggested reviewers

  • senamakel
  • graycyrus

Poem

A rabbit peeks at error skies, 🐇
Sniffs a 400 where silence lies.
It logs the cause with gentle tap,
Lets Sentry sleep — no urgent clap.
Hooray for quiet, tidy maps!

🚥 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 'fix(agent): handle config rejection in streaming_chat path' accurately reflects the main change—adding config rejection handling to the streaming chat request path in the OpenAI-compatible provider implementation.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ 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.

@YellowSnnowmann YellowSnnowmann marked this pull request as ready for review May 20, 2026 13:16
@YellowSnnowmann YellowSnnowmann requested a review from a team May 20, 2026 13:16
@coderabbitai coderabbitai Bot added the bug label May 20, 2026
coderabbitai[bot]
coderabbitai Bot previously approved these changes May 20, 2026
Copy link
Copy Markdown
Contributor

@M3gA-Mind M3gA-Mind left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Walkthrough

This PR closes a genuine gap: all five request arms in OpenAiCompatibleProvider (responses_api, streaming_chat, chat_completions, native_chat, stream_chat) were missing the is_provider_config_rejection_http branch that api_error in ops.rs already had. The fix is mechanically correct — each arm gets an identical else if inserted between the existing access-policy check and the generic should_report_provider_http_failure fallback, and anyhow::bail!(message) still fires below so the error is still propagated to callers. The provider_name.as_str() in the stream_chat arm (vs self.name.as_str() elsewhere) is the right choice for that closure context.

Area Files Result
Rust core — error classification compatible.rs 5 arms corrected, logic consistent
Tests compatible_tests.rs Missing — new branches uncovered

One blocking issue before this can merge.

Comment thread src/openhuman/inference/provider/compatible.rs
@M3gA-Mind M3gA-Mind self-assigned this May 20, 2026
@coderabbitai coderabbitai Bot added the working A PR that is being worked on by the team. label May 21, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug working A PR that is being worked on by the team.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants