improve(errors): accept Responses, type extractResponseBody - #3441
Draft
Dave Shoup (shouples) wants to merge 2 commits into
Draft
improve(errors): accept Responses, type extractResponseBody#3441Dave Shoup (shouples) wants to merge 2 commits into
Dave Shoup (shouples) wants to merge 2 commits into
Conversation
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
This PR refactors extractResponseBody() to be reusable for both OpenAPI-client ResponseErrors and raw Response objects, while improving type-safety by removing any and pushing callers to declare the response shape they expect.
Changes:
- Update
extractResponseBody()to acceptAnyResponseError | Responseand returnPromise<T>(defaultunknown), decoding JSON with a text fallback. - Update five call sites to provide explicit type arguments when decoding error response bodies.
- Add unit tests covering both
ResponseErrorand rawResponseinputs across JSON, text-fallback, and throw paths.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| src/errors.ts | Generalize extractResponseBody() to accept Response and add generic typing. |
| src/errors.test.ts | Expand unit coverage for extractResponseBody() across both accepted input kinds. |
| src/commands/flinkStatements.ts | Pass an explicit type argument when decoding 400 error bodies for statement submission. |
| src/commands/medusaCodeLens.ts | Pass an explicit type argument when decoding Medusa API error bodies. |
| src/commands/utils/uploadArtifactOrUDF.ts | Pass an explicit type argument when decoding upload error bodies and simplify error extraction. |
| src/flinkSql/flinkStatementResultsManager.ts | Pass an explicit type argument when decoding results-fetch error payloads. |
| src/viewProviders/flinkDatabase.ts | Pass an explicit type argument when decoding artifact-load error bodies for display. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
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
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 of Changes
extractResponseBody()(insrc/errors.ts) decodes the body of a failed HTTP response as JSON, falling back to raw text. It only accepted theResponseErrortypes our generated OpenAPI clients throw, and returnedPromise<any>- so every caller got an untyped blob and had to guess at its shape.Now it also accepts a raw
Response, so a directfetch()call (not through a generated client) can reuse the same decode logic. And it returns a genericPromise<T>(defaultunknown) instead ofany, so each caller passes the body shape it expects (e.g.extractResponseBody<{ message?: string }>(error)) and gets a checked result back.No runtime behavior change - same decoding, and non-
Response/non-ResponseErrorinput still throws (message now mentions both accepted types). Updated the five call sites to pass an explicit type argument, and added unit tests insrc/errors.test.tscovering both input kinds across the JSON, text-fallback, and throw paths.Closes #2683
Optional: Any additional details or context that should be provided?
This is a type-level refactor with no user-visible behavior change, so there's nothing to click-test. Run
npx gulp test -t "extractResponseBody"to see the new coverage.Call sites updated (each now declares the body shape it reads):
src/commands/flinkStatements.tssrc/commands/medusaCodeLens.tssrc/commands/utils/uploadArtifactOrUDF.tssrc/flinkSql/flinkStatementResultsManager.tssrc/viewProviders/flinkDatabase.tsPull request checklist
Please check if your PR fulfills the following (if applicable):
Tests
Release notes