Skip to content

Fix/embeddings OpenAI expected error reporting#2190

Merged
senamakel merged 3 commits into
tinyhumansai:mainfrom
YellowSnnowmann:fix/embeddings-openai-expected-error-reporting
May 19, 2026
Merged

Fix/embeddings OpenAI expected error reporting#2190
senamakel merged 3 commits into
tinyhumansai:mainfrom
YellowSnnowmann:fix/embeddings-openai-expected-error-reporting

Conversation

@YellowSnnowmann
Copy link
Copy Markdown
Contributor

@YellowSnnowmann YellowSnnowmann commented May 19, 2026

Summary

  • Demote the budget-exhausted HTTP 400 response from the OpenAI-compatible embedding provider to a breadcrumb at the call site instead of unconditionally emitting a Sentry error event.
  • Route openai_embed non-2xx reports through report_error_or_expected so the existing ExpectedErrorKind::BudgetExhausted classifier runs at emit time.
  • Caller behavior unchanged: the provider still returns Err so the embedding pipeline short-circuits exactly as before; only the Sentry signal changes.
  • Add a regression test that exercises the budget-exhausted 400 path end-to-end through a mocked /v1/embeddings endpoint.

Problem

OPENHUMAN-TAURI-JM — 136 events on openhuman@0.53.43+2b64ea8ac567 with the message:

Embedding API error 400 Bad Request: {"success":false,"error":"Budget exceeded — add credits to continue"}

This is a deterministic user-state condition (the user is out of credits) that the UI already surfaces as an actionable toast — Sentry has no remediation path, and one event fires per failed embed call across the entire memory-embedding pipeline. The phrasing was already recognized by is_budget_exhausted_message and bucketed as ExpectedErrorKind::BudgetExhausted, and PR #1633 added the is_budget_event before_send filter as defense-in-depth — but:

  • The embedding call site at ../src/openhuman/embeddings/openai.rs called report_error directly (skipping the classifier), so the Sentry event was still synthesized before being dropped at before_send.
  • Pre-fix(providers): drop budget-exhausted 400s from Sentry (#3M, #12, #13) #1633 releases (v0.53.43, which is what every event on this issue is tagged with) have no before_send filter at all, so the events flowed straight to Sentry.
  • Other embedding/inference call sites already use report_error_or_expected; this one was an outlier.

Solution

src/openhuman/embeddings/openai.rs — Switch the non-2xx diagnostic emit from report_error → report_error_or_expected. The classifier inspects the formatted message, matches the budget-exhausted phrase ("budget exceeded", "add credits", "insufficient balance"), and demotes to tracing::info! with kind = "budget" — no Sentry event is created. Other expected variants (API-key-missing, transient upstream HTTP, network-unreachable) are handled by the same classifier as a free bonus.

src/openhuman/embeddings/openai_tests.rs — Add embed_budget_exhausted_400_still_errors, which spins up a mock returning HTTP 400 with the canonical backend body and asserts the propagated error preserves both the status code and the "Budget exceeded" phrase so upstream short-circuit logic can still pattern-match on it.

Design notes

  • Kept the change scoped to the openai-compatible provider; ollama.rs was deliberately left alone because Ollama runs locally and does not proxy through the OpenHuman billing backend, so it cannot produce a budget-exhausted body.
  • The fix is intentionally at the call site rather than expanding the before_send filter — emitting then dropping is wasteful (event allocation, scope tagging, sentry hub work) and the demotion path is the documented pattern for every other expected-error site in the codebase.

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.

  • done Tests added or updated (happy path + at least one failure / edge case) per Testing Strategy — new embed_budget_exhausted_400_still_errors exercises the budget-exhausted 400 failure path; existing embed_server_error covers the generic non-2xx fallthrough.
  • not done 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.
  • done Coverage matrix updated — N/A: behaviour-only change (no user-visible feature row added/removed/renamed; the observability classification is internal-only).
  • done All affected feature IDs from the matrix are listed in the PR description under ## Related — N/A: no feature row impact, see above.
  • done No new external network dependencies introduced (mock backend used per Testing Strategy) — new test uses the existing in-process axum::Router mock pattern shared by every other test in this file.
  • done Manual smoke checklist updated if this touches release-cut surfaces (../docs/RELEASE-MANUAL-SMOKE.md) — N/A: observability-only change, no user-visible surface.
  • not done Linked issue closed via Closes #NNN in the ## Related section — see ## Related below.

Impact

  • Platform: Desktop only (the only host that links openhuman-core and emits to the openhuman-tauri Sentry project). No frontend, mobile, web, or CLI behavior change.
  • Runtime: Negligible — one extra string-match pass over the formatted error message before the Sentry capture decision. Hot path is unchanged on the success branch.
  • User-visible: None. The provider still returns Err with the same message; the embed pipeline short-circuits identically. UI continues to render the existing budget-exhausted toast.
  • Sentry signal: OPENHUMAN-TAURI-JM should stop accruing new events on builds containing this fix (combined with the v0.53.44+ before_send filter, current v0.53.43 deployments will tail off as users update).
  • Security / migration / compatibility: None. No schema, RPC, persistence, or wire-format change.

Related

Summary by CodeRabbit

  • Bug Fixes

    • Improved error handling for OpenAI embedding API responses, properly distinguishing between expected user-state conditions and unexpected errors in observability reporting.
  • Tests

    • Added test coverage for budget exhaustion scenarios in embeddings API.

Review Change Stack

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 19, 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: 1a0d7907-8bb0-4942-a1a2-a227f01f4879

📥 Commits

Reviewing files that changed from the base of the PR and between f285b80 and 81ee0ef.

📒 Files selected for processing (2)
  • src/openhuman/embeddings/openai.rs
  • src/openhuman/embeddings/openai_tests.rs

📝 Walkthrough

Walkthrough

OpenAiEmbedding::embed now routes non-success HTTP responses through report_error_or_expected instead of report_error, demoting expected user-state errors to lower-severity observability breadcrumbs. A new test validates that HTTP 400 budget-exhausted responses still return proper error formatting.

Changes

Error observability classification

Layer / File(s) Summary
Error classification demotion and validation
src/openhuman/embeddings/openai.rs, src/openhuman/embeddings/openai_tests.rs
Non-2xx responses now use report_error_or_expected to demote expected conditions (budget exhausted, rate limits) from Sentry errors to breadcrumbs. New integration test verifies HTTP 400 budget-exhausted responses still return errors with status code and message details.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

  • tinyhumansai/openhuman#1795: Both PRs route non-success "expected" error responses through report_error_or_expected to apply observability demotion logic across embeddings and provider integration clients.
  • tinyhumansai/openhuman#1763: Main PR switches embedding error handling to call report_error_or_expected, which integrates with the retrieved PR's core observability updates for classifying and demoting expected errors via ExpectedErrorKind.

Suggested labels

working

Suggested reviewers

  • senamakel
  • graycyrus

Poem

🐰 A little hop through error streams so bright,
Where expected states are demoted right,
No Sentry cries for budget's end of day,
Just breadcrumbs left to light the way! 🥕✨

🚥 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 directly and clearly summarizes the main change: routing OpenAI embedding errors through expected-error reporting instead of generic error reporting, which aligns perfectly with the changeset's primary objective.
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.

Warning

Review ran into problems

🔥 Problems

Git: Failed to clone repository. Please run the @coderabbitai full review command to re-trigger a full review. If the issue persists, set path_filters to include or exclude specific files.


Comment @coderabbitai help to get the list of available commands and usage tips.

@YellowSnnowmann YellowSnnowmann marked this pull request as ready for review May 19, 2026 09:41
@YellowSnnowmann YellowSnnowmann requested a review from a team May 19, 2026 09:41
@coderabbitai coderabbitai Bot added the working A PR that is being worked on by the team. label May 19, 2026
Copy link
Copy Markdown
Contributor

@graycyrus graycyrus left a comment

Choose a reason for hiding this comment

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

Walkthrough

Clean, well-scoped fix. Switches the openai embedding non-2xx diagnostic emit from report_errorreport_error_or_expected so the existing ExpectedErrorKind::BudgetExhausted classifier runs at the call site instead of synthesizing a Sentry event that gets dropped by before_send (or, on pre-#1633 builds, flows straight through). This aligns the openai embedding provider with every other expected-error call site in the codebase. Regression test is solid.

File Change Description
src/openhuman/embeddings/openai.rs Bug fix report_errorreport_error_or_expected for non-2xx responses
src/openhuman/embeddings/openai_tests.rs Test New embed_budget_exhausted_400_still_errors — mocked 400 with budget-exhausted body, asserts error propagation preserved

No findings — LGTM. Nice job keeping the scope tight and adding the regression test.

@senamakel senamakel merged commit c37f459 into tinyhumansai:main May 19, 2026
28 of 31 checks passed
CodeGhost21 pushed a commit to CodeGhost21/openhuman that referenced this pull request May 22, 2026
AusAgentSmith pushed a commit to AusAgentSmith/openhuman that referenced this pull request May 23, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants