Merge pull request #2988 from github/koesie10/error-default-branch

Add custom error handler for missing default branch

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"
+  );
+}

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(
+    let variantAnalysisResponse: ApiVariantAnalysis;
-      this.app.credentials,
+    try {
-      variantAnalysisSubmission,
+      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,

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)}`;
+}