fix: surface actionable error when OpenCode prompt returns no response data#28
Conversation
…e data On opencode CLI 1.17.x an invalid or unavailable provider/model ID (e.g. github-copilot/gpt-5 from #21) no longer produces an empty HTTP body; the prompt call succeeds but result.data is falsy, bypassing the actionable empty-JSON error from #24 and hitting the generic "No response data from OpenCode" error with no guidance. - doGenerate now throws an APICallError naming the model ID, suggesting `opencode models`, including the server error payload when available, and carrying errorType "EmptyResponseData" - doStream surfaces the same error as an error stream part and terminates the stream instead of hanging on the event subscription - wrapError returns already-wrapped AI SDK errors unchanged instead of re-wrapping them and losing their metadata - export createEmptyResponseDataError, with tests mirroring the #24 coverage in errors.test.ts and opencode-language-model.test.ts Verified against dist/ with opencode CLI 1.17.3: both generateText and streamText now produce the actionable message for github-copilot/gpt-5.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 61a33fd688
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Codex review flagged that the new no-data check in doStream assumed
fields-style SDK results. A client configured with responseStyle: "data"
(allowed by OpencodeClientOptions) resolves session.prompt to the payload
itself, or undefined on failure, so the check would flag every successful
prompt as missing data (terminating a healthy stream) or throw a TypeError
on undefined.
- Managed clients are now always created with responseStyle: "fields";
clientOptions.responseStyle "data" is ignored with a warning, mirroring
the existing baseUrl/directory normalization
- doGenerate/doStream normalize the prompt result shape, treating
fields-style results ({ data, error }) and data-style payloads
uniformly, so caller-supplied data-style clients get correct content
on success and the actionable EmptyResponseData error on failure
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9442c9e091
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Codex follow-up: a preconfigured responseStyle "data" client without settings.sessionId never reached the prompt-result normalization because getOrCreateSession() still read result.data from session.create, treating a successful data-style create payload as a failure. Generalize extractPromptResult to extractSdkResult and apply it to session.create as well, so data-style clients work end-to-end.
|
@codex review |
|
Codex Review: Didn't find any major issues. Swish! ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
Summary
Extends the actionable error guidance from #24 to the no-data branch. On opencode CLI 1.17.x, an invalid or unavailable
provider/modelID (e.g.github-copilot/gpt-5, the exact repro from #21) no longer produces an empty HTTP body — the SDK call succeeds butresult.datais falsy, bypassing #24'sEmptyJSONResponsewrapping and hitting the genericNo response data from OpenCodeerror (errorType"Error", no guidance).Changes
doGenerate: the no-data branch now throws anAPICallErrornaming the model ID, suggestingopencode models, including the server error payload (result.error) when available, and carryingerrorType: "EmptyResponseData"withisRetryable: false.doStream: a prompt that resolves with no data previously left the stream hanging forever on the event subscription. It now surfaces the same actionable error as anerrorstream part and terminates the stream. The error enqueue is also guarded against an already-closed controller (the prompt promise routinely settles aftersession.idlecloses the stream).wrapError: now returns already-wrapped AI SDK errors (APICallError,LoadAPIKeyError) unchanged instead of re-wrapping them and losing their metadata. Needed because the new error is thrown insidedoGenerate's try block whose catch funnels everything throughwrapError; also fixes double-wrapping for direct callers.createEmptyResponseDataErrorfactory exported alongsidecreateAPICallError/createTimeoutError.createEmptyResponseDataErrorunit tests and awrapErrorpass-through test inerrors.test.ts;doGenerateno-data and server-error-detail tests plus adoStreamhanging-stream test inopencode-language-model.test.ts.[3.0.5]section (no version bump here — main is already at 3.0.5 from fix: preconfigured-client example and singleton client manager dispose recovery #27; this work ships in that release).Verification
Tested against
dist/with opencode CLI 1.17.3 and the invalid modelgithub-copilot/gpt-5. BothgenerateTextandstreamTextnow produce:with
data.errorType: "EmptyResponseData", and the streaming call terminates instead of hanging.npm run format:checkandnpm run ci(typecheck, lint, 366 tests) pass locally.Refs #21, follows up on #24.