Skip to content
Draft
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion src/commands/flinkStatements.ts
Original file line number Diff line number Diff line change
Expand Up @@ -248,7 +248,9 @@ export async function submitFlinkStatementCommand(
if (isResponseError(err) && err.response.status === 400) {
// Usually a bad SQL statement.
// The error string should be JSON, have 'errors' as an array of objs with 'details' human readable messages.
const objFromResponse = await extractResponseBody(err);
const objFromResponse = await extractResponseBody<{ errors: [{ detail: string }] } | string>(
err,
);
let errorMessages: string;
if (objFromResponse && typeof objFromResponse === "object" && "errors" in objFromResponse) {
Comment thread
shouples marked this conversation as resolved.
Outdated
const responseErrors: { errors: [{ detail: string }] } = objFromResponse;
Expand Down
2 changes: 1 addition & 1 deletion src/commands/medusaCodeLens.ts
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,7 @@ async function convertAvroSchemaToDataset(avroSchemaContent: string): Promise<Da

// Extract better error message from ResponseError if available
if (isResponseError(error)) {
const responseBody = await extractResponseBody(error);
const responseBody = await extractResponseBody<{ message?: string }>(error);
const errorMessage = responseBody?.message || responseBody;
throw new Error(`Medusa API error: ${errorMessage}`);
}
Expand Down
4 changes: 2 additions & 2 deletions src/commands/utils/uploadArtifactOrUDF.ts
Original file line number Diff line number Diff line change
Expand Up @@ -239,13 +239,13 @@ export function validateUdfInput(
export async function buildUploadErrorMessage(err: unknown, base: string): Promise<string> {
let errorMessage = base;
if (isResponseError(err)) {
const resp = await extractResponseBody(err);
const resp = await extractResponseBody<{ errors?: { detail: string }[] }>(err);

if (err.response.status === 400) {
// Bad request - a validation error we couldn't prevent. Only log to Sentry if unparseable.
if (resp && typeof resp === "object" && "errors" in resp) {
// Gather the detail(s) from all error(s)
const errors: Array<{ detail: string }> = resp.errors;
const errors = resp.errors ?? [];
errorMessage = `${errorMessage} ${errors.map((e) => e.detail).join("\n")}`;
} else {
// Unexpected - log to Sentry for investigation
Expand Down
35 changes: 29 additions & 6 deletions src/errors.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -192,29 +192,52 @@ describe("errors.ts isTransientResponseError()", () => {
});

describe("errors.ts extractResponseBody()", () => {
it("should return the response body as JSON if it is valid JSON", async () => {
it("should return a ResponseError's body as JSON if it is valid JSON", async () => {
const embeddedObject = { message: "test" };
const error = createResponseError(400, "Bad Request", JSON.stringify(embeddedObject));
const body = await extractResponseBody(error);

const body = await extractResponseBody<{ message: string }>(error);

assert.deepStrictEqual(embeddedObject, body);
});

it("should return the response body as string if it is not valid JSON", async () => {
it("should return a ResponseError's body as a string if it is not valid JSON", async () => {
const textResponse = "test random not-json {";
const error = createResponseError(400, "Bad Request", textResponse);

const body = await extractResponseBody(error);

assert.strictEqual(body, textResponse);
});

it("should decode a raw Response passed directly, as JSON if it is valid JSON", async () => {
const embeddedObject = { message: "test" };
const response = new Response(JSON.stringify(embeddedObject));

const body = await extractResponseBody<{ message: string }>(response);

assert.deepStrictEqual(embeddedObject, body);
});

it("should decode a raw Response passed directly, as a string if it is not valid JSON", async () => {
const textResponse = "test random not-json {";
const response = new Response(textResponse);

const body = await extractResponseBody(response);

assert.strictEqual(body, textResponse);
});

it("should throw if the error is not a ResponseError", async () => {
it("should throw if given neither a ResponseError nor a Response", async () => {
const error = new Error("test");

await assert.rejects(
async () => {
await extractResponseBody(error as any);
await extractResponseBody(error as unknown as Response);
},
{
name: "Error",
message: "extractResponseBody() called with non-ResponseError",
message: "extractResponseBody() called with neither a ResponseError nor a Response",
},
);
});
Expand Down
30 changes: 21 additions & 9 deletions src/errors.ts
Original file line number Diff line number Diff line change
Expand Up @@ -64,23 +64,35 @@
}

/**
* If error is a response error, try to decode its response body
* from JSON and return the resulting object.
* Decode the body of a {@link Response}, whether passed directly (e.g. from a raw `fetch()`) or
* wrapped in one of our client {@link AnyResponseError}s.
*
* If the response body is not JSON, return the text instead.
* The body is decoded as JSON; if it is not valid JSON, the raw text is returned instead.
*
* If the error is not a response error, raise an error.
* The caller supplies the expected body shape via the type parameter (defaulting to `unknown`), so
* downstream code has some idea what to expect rather than an untyped `any`.
*
* @throws if given something that is neither a {@link Response} nor an {@link AnyResponseError}.
*/
export async function extractResponseBody(error: AnyResponseError): Promise<any> {
if (!isResponseError(error)) {
throw new Error("extractResponseBody() called with non-ResponseError");
export async function extractResponseBody<T = unknown>(
errorOrResponse: AnyResponseError | Response,
): Promise<T> {
let response: Response;
if (isResponseError(errorOrResponse)) {
response = errorOrResponse.response;
} else if (errorOrResponse instanceof Response) {
response = errorOrResponse;
} else {
throw new Error("extractResponseBody() called with neither a ResponseError nor a Response");

Check warning on line 86 in src/errors.ts

View check run for this annotation

SonarQube-Confluent / SonarQube Code Analysis

`new Error()` is too unspecific for a type check. Use `new TypeError()` instead.

[S7786] Generic "Error" should be "TypeError" when thrown after type checking See more on https://sonarqube.confluent.io/project/issues?id=vscode&pullRequest=3441&issues=a04b5df9-29b6-4d36-a384-ca9d6d537fb5&open=a04b5df9-29b6-4d36-a384-ca9d6d537fb5
}

// Attempt to parse the response body as JSON, falling back to text if it fails
try {
return await error.response.clone().json();
const json: unknown = await response.clone().json();
return json as T;
} catch {
return await error.response.clone().text();
const text: unknown = await response.clone().text();
return text as T;
Comment thread
shouples marked this conversation as resolved.
}
}

Expand Down
2 changes: 1 addition & 1 deletion src/flinkSql/flinkStatementResultsManager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -354,7 +354,7 @@ export class FlinkStatementResultsManager {

if (isResponseError(error)) {
// clone before reading, so logError() below can still read the body for Sentry
const payload = await extractResponseBody(error);
const payload = await extractResponseBody<{ aborted?: boolean }>(error);
if (!payload?.aborted) {
const status = error.response.status;
shouldComplete = status >= 400;
Expand Down
2 changes: 1 addition & 1 deletion src/viewProviders/flinkDatabase.ts
Original file line number Diff line number Diff line change
Expand Up @@ -389,7 +389,7 @@ export class FlinkDatabaseViewProvider extends ParentedBaseViewProvider<
// only applies to loading artifacts, since all others are loaded via background statements
// and won't throw HTTP response errors
if (isResponseError(error)) {
const responseBody = await extractResponseBody(error);
const responseBody = await extractResponseBody<{ message?: string }>(error);
errorMsg = responseBody?.message || JSON.stringify(responseBody, null, 2);
errorLanguage = "json";
}
Expand Down