Add custom error handler for missing default branch
When the GitHub API returns an error for a missing default branch, we will now show a custom error message. This custom error message includes a link to the page to create the branch. The error is detected using the `errors` field on the response that is now being returned.
This commit is contained in:
85
extensions/ql-vscode/src/variant-analysis/custom-errors.ts
Normal file
85
extensions/ql-vscode/src/variant-analysis/custom-errors.ts
Normal file
@@ -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"
|
||||
);
|
||||
}
|
||||
@@ -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,
|
||||
|
||||
@@ -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)}`;
|
||||
}
|
||||
Reference in New Issue
Block a user