Skip to content
Merged
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
85 changes: 85 additions & 0 deletions extensions/ql-vscode/src/variant-analysis/custom-errors.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,85 @@
import { RequestError } from "@octokit/request-error";
import { NotificationLogger, showAndLogErrorMessage } from "../common/logging";

type ApiError = {
resource: string;
field: string;
code: string;
};

type ErrorResponse = {
message: string;
errors?: ApiError[];
};

export function handleRequestError(
e: RequestError,
logger: NotificationLogger,
): boolean {
if (e.status !== 422) {
return false;
}

if (!e.response?.data) {
return false;
}

const data = e.response.data;
if (!isErrorResponse(data)) {
return false;
}

if (!data.errors) {
return false;
}

// This is the only custom error message we have
const missingDefaultBranchError = data.errors.find(
(error) =>
error.resource === "Repository" &&
error.field === "default_branch" &&
error.code === "missing",
);

if (!missingDefaultBranchError) {
return false;
}

if (
!("repository" in missingDefaultBranchError) ||
typeof missingDefaultBranchError.repository !== "string"
) {
return false;
}

if (
!("default_branch" in missingDefaultBranchError) ||
typeof missingDefaultBranchError.default_branch !== "string"
) {
return false;
}

const createBranchURL = `https://github.com/${
missingDefaultBranchError.repository
}/new/${encodeURIComponent(missingDefaultBranchError.default_branch)}`;

void showAndLogErrorMessage(
logger,
`Variant analysis failed because the controller repository ${missingDefaultBranchError.repository} does not have a branch '${missingDefaultBranchError.default_branch}'. ` +
`Please create a '${missingDefaultBranchError.default_branch}' branch by clicking [here](${createBranchURL}) and re-run the variant analysis query.`,
{
fullMessage: e.message,
},
);

return true;
}

function isErrorResponse(obj: unknown): obj is ErrorResponse {
return (
typeof obj === "object" &&
obj !== null &&
"message" in obj &&
typeof obj.message === "string"
);
}
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import {
submitVariantAnalysis,
getVariantAnalysisRepo,
} from "./gh-api/gh-api-client";
import { VariantAnalysis as ApiVariantAnalysis } from "./gh-api/variant-analysis";
import {
authentication,
AuthenticationSessionsChangeEvent,
Expand Down Expand Up @@ -76,6 +77,8 @@ import {
showAndLogWarningMessage,
} from "../common/logging";
import { QueryTreeViewItem } from "../queries-panel/query-tree-view-item";
import { RequestError } from "@octokit/request-error";
import { handleRequestError } from "./custom-errors";

const maxRetryCount = 3;

Expand Down Expand Up @@ -254,10 +257,20 @@ export class VariantAnalysisManager
},
};

const variantAnalysisResponse = await submitVariantAnalysis(
this.app.credentials,
variantAnalysisSubmission,
);
let variantAnalysisResponse: ApiVariantAnalysis;
try {
variantAnalysisResponse = await submitVariantAnalysis(
this.app.credentials,
variantAnalysisSubmission,
);
} catch (e: unknown) {
// If the error is handled by the handleRequestError function, we don't need to throw
if (e instanceof RequestError && handleRequestError(e, this.app.logger)) {
return;
}

throw e;
}

const processedVariantAnalysis = processVariantAnalysis(
variantAnalysisSubmission,
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,159 @@
import { RequestError } from "@octokit/request-error";
import { createMockLogger } from "../../__mocks__/loggerMock";
import { handleRequestError } from "../../../src/variant-analysis/custom-errors";
import { faker } from "@faker-js/faker";

describe("handleRequestError", () => {
const logger = createMockLogger();

it("returns false when handling a non-422 error", () => {
const e = mockRequestError(404, {
message: "Not Found",
});
expect(handleRequestError(e, logger)).toBe(false);
expect(logger.showErrorMessage).not.toHaveBeenCalled();
});

it("returns false when handling a different error without errors", () => {
const e = mockRequestError(422, {
message:
"Unable to trigger a variant analysis. None of the requested repositories could be found.",
});
expect(handleRequestError(e, logger)).toBe(false);
expect(logger.showErrorMessage).not.toHaveBeenCalled();
});

it("returns false when handling an error without response body", () => {
const e = mockRequestError(422, undefined);
expect(handleRequestError(e, logger)).toBe(false);
expect(logger.showErrorMessage).not.toHaveBeenCalled();
});

it("returns false when handling an error without response", () => {
const e = new RequestError("Timeout", 500, {
headers: {
"Content-Type": "application/json",
},
request: {
method: "POST",
url: faker.internet.url(),
headers: {
"Content-Type": "application/json",
},
},
});
expect(handleRequestError(e, logger)).toBe(false);
expect(logger.showErrorMessage).not.toHaveBeenCalled();
});

it("returns false when handling a different error with errors", () => {
const e = mockRequestError(422, {
message:
"Unable to trigger a variant analysis. None of the requested repositories could be found.",
errors: [
{
resource: "VariantAnalysis",
field: "repositories",
code: "not_found",
},
],
});
expect(handleRequestError(e, logger)).toBe(false);
expect(logger.showErrorMessage).not.toHaveBeenCalled();
});

it("returns false when handling without repository field", () => {
const e = mockRequestError(422, {
message:
"Variant analysis failed because controller repository github/pickles does not have a branch 'main'. Please create a 'main' branch in the repository and re-run the variant analysis.",
errors: [
{
resource: "Repository",
field: "default_branch",
code: "missing",
default_branch: "main",
},
],
});
expect(handleRequestError(e, logger)).toBe(false);
expect(logger.showErrorMessage).not.toHaveBeenCalled();
});

it("returns false when handling without default_branch field", () => {
const e = mockRequestError(422, {
message:
"Variant analysis failed because controller repository github/pickles does not have a branch 'main'. Please create a 'main' branch in the repository and re-run the variant analysis.",
errors: [
{
resource: "Repository",
field: "default_branch",
code: "missing",
repository: "github/pickles",
},
],
});
expect(handleRequestError(e, logger)).toBe(false);
expect(logger.showErrorMessage).not.toHaveBeenCalled();
});

it("shows notification when handling a missing default branch error", () => {
const e = mockRequestError(422, {
message:
"Variant analysis failed because controller repository github/pickles does not have a branch 'main'. Please create a 'main' branch in the repository and re-run the variant analysis.",
errors: [
{
resource: "Repository",
field: "default_branch",
code: "missing",
repository: "github/pickles",
default_branch: "main",
},
],
});
expect(handleRequestError(e, logger)).toBe(true);
expect(logger.showErrorMessage).toHaveBeenCalledWith(
"Variant analysis failed because the controller repository github/pickles does not have a branch 'main'. Please create a 'main' branch by clicking [here](https://github.com/github/pickles/new/main) and re-run the variant analysis query.",
);
});
});

function mockRequestError(status: number, body: any): RequestError {
return new RequestError(
body ? toErrorMessage(body) : "Unknown error",
status,
{
request: {
method: "POST",
url: faker.internet.url(),
headers: {
"Content-Type": "application/json",
},
},
response: {
url: faker.internet.url(),
status,
headers: {
"Content-Type": "application/json",
},
data: body,
},
},
);
}

// Copied from https://github.com/octokit/request.js/blob/c67f902350384846f88d91196e7066daadc08357/src/fetch-wrapper.ts#L166 to have a
// somewhat realistic error message
function toErrorMessage(data: any) {
if (typeof data === "string") return data;

if ("message" in data) {
if (Array.isArray(data.errors)) {
return `${data.message}: ${data.errors.map(JSON.stringify).join(", ")}`;
}

return data.message;
}

// istanbul ignore next - just in case
return `Unknown error: ${JSON.stringify(data)}`;
}