Skip to content

fix: [#788] surface OpenAI nested error.message instead of Bad Request: {?:?}#789

Merged
anandgupta42 merged 1 commit intomainfrom
fix/openai-error-message-extraction
May 4, 2026
Merged

fix: [#788] surface OpenAI nested error.message instead of Bad Request: {?:?}#789
anandgupta42 merged 1 commit intomainfrom
fix/openai-error-message-extraction

Conversation

@anandgupta42
Copy link
Copy Markdown
Contributor

@anandgupta42 anandgupta42 commented May 4, 2026

What does this PR do?

Fixes provider/error.ts so OpenAI 4xx errors with the standard {error: {message, type, code}} shape surface their inner message to the user, instead of the previous fallthrough behavior that produced APIError: Bad Request: {?:?} (the {?:?} is the App Insights pipeline scrubbing string values from the raw structured body).

The original OR-chain body.message || body.error || body.error?.message short-circuited on body.error (an object), failed the typeof === "string" guard, and dumped the raw responseBody. Replaced with an explicit-typeof ternary that handles the three known shapes cleanly and matches parseStreamError's defensive pattern at line 153.

Wrapped in // altimate_change start — upstream_fix: markers per project policy. Upstream has the same bug; a parallel upstream PR is being filed so we can drop our marker if upstream takes the fix.

Type of change

  • Bug fix (non-breaking change which fixes an issue)

Issue for this PR

Closes #788

How did you verify your code works?

  • bun test packages/opencode/test/provider/error.test.ts24/24 pass (18 existing + 6 new)
  • bun test packages/opencode/test/provider/386/386 pass (full provider suite)
  • bunx tsgo --noEmit — clean (exit 0)
  • bun run script/upstream/analyze.ts --markers --base main --strict — passes (changes wrapped in markers)
  • Reviewed by GPT 5.4, Gemini 3.1 Pro, Kimi K2.5 via /consensus:code-review — all approved with the test coverage and ternary improvements applied before commit

Tests added

  • OpenAI nested shape extracts the inner message and does not leak the structured body (regression test for the bug)
  • Top-level body.message still works (non-regression)
  • Plain-string body.error still works (non-regression)
  • Object body.error without message key falls through to top-level body.message
  • Non-string body.error.message (array/object) does not block a valid body.message (defensive guard for the same class of bug)
  • All-non-string-fields body falls through to raw responseBody (existing last-resort behavior preserved)

Checklist

  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • I have made corresponding changes to the documentation (n/a — internal error parser)
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective
  • New and existing unit tests pass locally with my changes

Summary by cubic

Fixes error message extraction for OpenAI 4xx responses by reading nested error.message instead of dumping the raw body. Users now see actionable errors (e.g., model not found) instead of "Bad Request: {?:?}", meeting Linear #788.

  • Bug Fixes
    • Extract error via body.error.messagebody.message → string body.error, avoiding object short-circuits and matching stream parser behavior.
    • Avoids leaking raw responseBody when a valid message exists; covered by new tests.

Written for commit 3a7ec72. Summary will update on new commits.

Summary by CodeRabbit

Release Notes

  • Bug Fixes

    • Improved error message extraction from API responses, ensuring clearer and more accurate error details are displayed to users even when APIs return complex error structures.
  • Tests

    • Added comprehensive test coverage for error message extraction across multiple response formats to ensure consistent error reporting.

…st: {?:?}`

`provider/error.ts` extracted `errMsg` via `body.message || body.error || body.error?.message`. For OpenAI's standard `{error: {message, type, code}}` response shape, `body.error` short-circuits as a truthy object, the `typeof errMsg === "string"` guard fails, and the parser falls through to dumping the raw response body. Telemetry (2026-05-04) caught users seeing `APIError: Bad Request: {?:?}` (App Insights redacts string values) and retrying with the same broken model selection because the surfaced error gave no clue about the underlying cause.

Switch to an explicit-typeof ternary matching `parseStreamError`'s pattern (line 153). Now handles all three shapes safely:
- `{error: {message: "..."}}` (OpenAI) — extracts nested message
- `{message: "..."}` (Anthropic et al.) — extracts top-level message
- `{error: "..."}` (some others) — extracts string error

Wrapped in `altimate_change start — upstream_fix:` markers; upstream has the same bug. Filing upstream PR in parallel.

## Tests

Six tests added to `error.test.ts` covering:
- OpenAI nested shape extracts the inner message and does not leak the structured body
- Top-level message field still works
- Plain-string `body.error` still works
- Object `body.error` without a `message` key falls through to top-level `body.message`
- Non-string `body.error.message` (array/object) does not block a valid `body.message` (regression guard for the same class of bug)
- All-non-string-fields body falls through to raw responseBody dump (existing last-resort behavior preserved)

Suite: 386/386 provider tests pass. Typecheck clean. Marker check passes.

Reviewed by GPT 5.4, Gemini 3.1 Pro, Kimi K2.5 via /consensus:code-review — all approved with the test coverage and ternary improvements that landed before commit.

Closes #788
Copy link
Copy Markdown

@claude claude Bot left a comment

Choose a reason for hiding this comment

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

Claude Code Review

This repository is configured for manual code reviews. Comment @claude review to trigger a review and subscribe this PR to future pushes, or @claude review once for a one-time review.

Tip: disable this comment in your organization's Code Review settings.

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 4, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: c1543697-9639-480d-b8bb-97ca5bcb9b5e

📥 Commits

Reviewing files that changed from the base of the PR and between 10381b0 and 3a7ec72.

📒 Files selected for processing (2)
  • packages/opencode/src/provider/error.ts
  • packages/opencode/test/provider/error.test.ts

📝 Walkthrough

Walkthrough

This PR fixes error message extraction in OpenAI API response handling. The ProviderError.message() method now safely extracts messages from nested error payloads via explicit string-type checks instead of short-circuiting on truthy objects, resolving cases where raw JSON was included in error messages.

Changes

Error Message Extraction Robustness

Layer / File(s) Summary
Core Logic
packages/opencode/src/provider/error.ts
ProviderError.message() replaces OR-chain short-circuit with nested ternary checks for body.error?.message, then body.message, then body.error (if string), preventing truthy objects from blocking access to nested message fields.
Test Coverage
packages/opencode/test/provider/error.test.ts
New test cases validate extraction from OpenAI-shaped {error: {message}}, top-level {message}, string {error}, fallback chains, non-string intermediate values, and raw body dumps when no string message exists.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

🐰 A rabbit hops through error trails,
Where nested messages once failed,
Now deep-dives through the JSON lore,
No more {?:?} obscuring scores!

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (2 warnings)

Check name Status Explanation Resolution
Description check ⚠️ Warning The PR description is comprehensive, including what changed and why, detailed test results, and verification methods. However, it does not include the required 'PINEAPPLE' marker at the top for AI-generated contributions per the template. Add 'PINEAPPLE' at the very top of the PR description as required by the template for AI-generated contributions before any other content.
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and specifically describes the main fix: surfacing OpenAI's nested error.message instead of opaque '{?:?}' output, directly addressing issue #788.
Linked Issues check ✅ Passed The PR successfully implements the fix specified in #788: replacing the OR-chain with an explicit-typeof ternary that correctly extracts nested body.error.message, top-level body.message, and plain-string body.error, with comprehensive test coverage for all message shapes.
Out of Scope Changes check ✅ Passed All changes are directly scoped to fixing the error message extraction in provider/error.ts and adding corresponding tests; no unrelated modifications or scope creep detected.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/openai-error-message-extraction

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share
Review rate limit: 7/8 reviews remaining, refill in 7 minutes and 30 seconds.

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

Copy link
Copy Markdown

@cubic-dev-ai cubic-dev-ai Bot left a comment

Choose a reason for hiding this comment

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

No issues found across 2 files

@anandgupta42 anandgupta42 merged commit 95b7b90 into main May 4, 2026
20 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

OpenAI errors render as 'Bad Request: {?:?}' — provider/error.ts drops nested error.message

1 participant