diff --git a/extensions/ql-vscode/src/variant-analysis/custom-errors.ts b/extensions/ql-vscode/src/variant-analysis/custom-errors.ts new file mode 100644 index 00000000000..4e5cd8aacbd --- /dev/null +++ b/extensions/ql-vscode/src/variant-analysis/custom-errors.ts @@ -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" + ); +} diff --git a/extensions/ql-vscode/src/variant-analysis/variant-analysis-manager.ts b/extensions/ql-vscode/src/variant-analysis/variant-analysis-manager.ts index 8ff46901f33..d4b463700d3 100644 --- a/extensions/ql-vscode/src/variant-analysis/variant-analysis-manager.ts +++ b/extensions/ql-vscode/src/variant-analysis/variant-analysis-manager.ts @@ -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, @@ -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; @@ -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, diff --git a/extensions/ql-vscode/test/unit-tests/variant-analysis/custom-errors.test.ts b/extensions/ql-vscode/test/unit-tests/variant-analysis/custom-errors.test.ts new file mode 100644 index 00000000000..c81edb02152 --- /dev/null +++ b/extensions/ql-vscode/test/unit-tests/variant-analysis/custom-errors.test.ts @@ -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)}`; +}