Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
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
6 changes: 4 additions & 2 deletions src/commands/flinkStatements.ts
Original file line number Diff line number Diff line change
Expand Up @@ -248,10 +248,12 @@ 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) {
const responseErrors: { errors: [{ detail: string }] } = objFromResponse;
const responseErrors: { errors: { detail: string }[] } = objFromResponse;
logger.error(JSON.stringify(responseErrors, null, 2));
errorMessages = responseErrors.errors.map((e: { detail: string }) => e.detail).join("\n");
} else {
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