From 31b2d24ca94340b345d33df974aa3f349d7389a9 Mon Sep 17 00:00:00 2001 From: Koen Vlaswinkel Date: Wed, 24 Apr 2024 13:17:46 +0200 Subject: [PATCH] Update custom errors to use GHEC-DR URL --- extensions/ql-vscode/src/config.ts | 39 +++++++++++++++++- extensions/ql-vscode/src/extension.ts | 2 + .../src/variant-analysis/custom-errors.ts | 10 +++-- .../variant-analysis-manager.ts | 7 +++- extensions/ql-vscode/test/factories/config.ts | 2 + .../variant-analysis/custom-errors.test.ts | 41 +++++++++++++++---- .../variant-analysis-manager.test.ts | 2 + .../variant-analysis-manager.test.ts | 2 + 8 files changed, 91 insertions(+), 14 deletions(-) diff --git a/extensions/ql-vscode/src/config.ts b/extensions/ql-vscode/src/config.ts index 2217cb397..ed305ccee 100644 --- a/extensions/ql-vscode/src/config.ts +++ b/extensions/ql-vscode/src/config.ts @@ -108,12 +108,36 @@ export function hasEnterpriseUri(): boolean { return getEnterpriseUri() !== undefined; } +/** + * Does the uri look like GHEC-DR? + */ +function isGhecDrUri(uri: Uri | undefined): boolean { + return uri !== undefined && uri.authority.toLowerCase().endsWith(".ghe.com"); +} + /** * Is the GitHub Enterprise URI set to something that looks like GHEC-DR? */ export function hasGhecDrUri(): boolean { const uri = getEnterpriseUri(); - return uri !== undefined && uri.authority.toLowerCase().endsWith(".ghe.com"); + return isGhecDrUri(uri); +} + +/** + * The URI for GitHub.com. + */ +export const GITHUB_URL = new URL("https://github.com"); + +/** + * If the GitHub Enterprise URI is set to something that looks like GHEC-DR, return it. + */ +export function getGhecDrUri(): Uri | undefined { + const uri = getEnterpriseUri(); + if (isGhecDrUri(uri)) { + return uri; + } else { + return undefined; + } } const ROOT_SETTING = new Setting("codeQL"); @@ -570,6 +594,11 @@ export async function setRemoteControllerRepo(repo: string | undefined) { export interface VariantAnalysisConfig { controllerRepo: string | undefined; showSystemDefinedRepositoryLists: boolean; + /** + * This uses a URL instead of a URI because the URL class is available in + * unit tests and is fully browser-compatible. + */ + githubUrl: URL; onDidChangeConfiguration?: Event; } @@ -591,6 +620,14 @@ export class VariantAnalysisConfigListener public get showSystemDefinedRepositoryLists(): boolean { return !hasEnterpriseUri(); } + + public get githubUrl(): URL { + const ghecDrUri = getGhecDrUri(); + if (ghecDrUri) { + return new URL(ghecDrUri.toString()); + } + return GITHUB_URL; + } } const VARIANT_ANALYSIS_FILTER_RESULTS = new Setting( diff --git a/extensions/ql-vscode/src/extension.ts b/extensions/ql-vscode/src/extension.ts index a4849ddc2..eef7993f2 100644 --- a/extensions/ql-vscode/src/extension.ts +++ b/extensions/ql-vscode/src/extension.ts @@ -31,6 +31,7 @@ import { joinOrderWarningThreshold, QueryHistoryConfigListener, QueryServerConfigListener, + VariantAnalysisConfigListener, } from "./config"; import { AstViewer, @@ -875,6 +876,7 @@ async function activateWithInstalledDistribution( variantAnalysisStorageDir, variantAnalysisResultsManager, dbModule.dbManager, + new VariantAnalysisConfigListener(), ); ctx.subscriptions.push(variantAnalysisManager); ctx.subscriptions.push(variantAnalysisResultsManager); diff --git a/extensions/ql-vscode/src/variant-analysis/custom-errors.ts b/extensions/ql-vscode/src/variant-analysis/custom-errors.ts index e2846e0ae..246ec5dba 100644 --- a/extensions/ql-vscode/src/variant-analysis/custom-errors.ts +++ b/extensions/ql-vscode/src/variant-analysis/custom-errors.ts @@ -15,6 +15,7 @@ type ErrorResponse = { export function handleRequestError( e: RequestError, + githubUrl: URL, logger: NotificationLogger, ): boolean { if (e.status !== 422) { @@ -60,9 +61,12 @@ export function handleRequestError( return false; } - const createBranchURL = `https://github.com/${ - missingDefaultBranchError.repository - }/new/${encodeURIComponent(missingDefaultBranchError.default_branch)}`; + const createBranchURL = new URL( + `/${ + missingDefaultBranchError.repository + }/new/${encodeURIComponent(missingDefaultBranchError.default_branch)}`, + githubUrl, + ).toString(); void showAndLogErrorMessage( logger, 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 fdf8abd7c..4bfe3274c 100644 --- a/extensions/ql-vscode/src/variant-analysis/variant-analysis-manager.ts +++ b/extensions/ql-vscode/src/variant-analysis/variant-analysis-manager.ts @@ -98,6 +98,7 @@ import { findVariantAnalysisQlPackRoot } from "./ql"; import { resolveCodeScanningQueryPack } from "./code-scanning-pack"; import { isSarifResultsQueryKind } from "../common/query-metadata"; import { isVariantAnalysisEnabledForGitHubHost } from "./ghec-dr"; +import type { VariantAnalysisConfig } from "../config"; import { getEnterpriseUri } from "../config"; const maxRetryCount = 3; @@ -158,6 +159,7 @@ export class VariantAnalysisManager private readonly storagePath: string, private readonly variantAnalysisResultsManager: VariantAnalysisResultsManager, private readonly dbManager: DbManager, + private readonly config: VariantAnalysisConfig, ) { super(); this.variantAnalysisMonitor = this.push( @@ -426,7 +428,10 @@ export class VariantAnalysisManager ); } 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)) { + if ( + e instanceof RequestError && + handleRequestError(e, this.config.githubUrl, this.app.logger) + ) { return undefined; } diff --git a/extensions/ql-vscode/test/factories/config.ts b/extensions/ql-vscode/test/factories/config.ts index 9a7060871..ef24f6617 100644 --- a/extensions/ql-vscode/test/factories/config.ts +++ b/extensions/ql-vscode/test/factories/config.ts @@ -1,9 +1,11 @@ import type { VariantAnalysisConfig } from "../../src/config"; +import { GITHUB_URL } from "../../src/config"; export function createMockVariantAnalysisConfig(): VariantAnalysisConfig { return { controllerRepo: "foo/bar", showSystemDefinedRepositoryLists: true, + githubUrl: GITHUB_URL, onDidChangeConfiguration: jest.fn(), }; } 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 index 017324baf..91bb43311 100644 --- 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 @@ -4,13 +4,14 @@ import { handleRequestError } from "../../../src/variant-analysis/custom-errors" import { faker } from "@faker-js/faker"; describe("handleRequestError", () => { + const githubUrl = new URL("https://github.com"); 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(handleRequestError(e, githubUrl, logger)).toBe(false); expect(logger.showErrorMessage).not.toHaveBeenCalled(); }); @@ -19,13 +20,13 @@ describe("handleRequestError", () => { message: "Unable to trigger a variant analysis. None of the requested repositories could be found.", }); - expect(handleRequestError(e, logger)).toBe(false); + expect(handleRequestError(e, githubUrl, 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(handleRequestError(e, githubUrl, logger)).toBe(false); expect(logger.showErrorMessage).not.toHaveBeenCalled(); }); @@ -42,7 +43,7 @@ describe("handleRequestError", () => { }, }, }); - expect(handleRequestError(e, logger)).toBe(false); + expect(handleRequestError(e, githubUrl, logger)).toBe(false); expect(logger.showErrorMessage).not.toHaveBeenCalled(); }); @@ -58,7 +59,7 @@ describe("handleRequestError", () => { }, ], }); - expect(handleRequestError(e, logger)).toBe(false); + expect(handleRequestError(e, githubUrl, logger)).toBe(false); expect(logger.showErrorMessage).not.toHaveBeenCalled(); }); @@ -75,7 +76,7 @@ describe("handleRequestError", () => { }, ], }); - expect(handleRequestError(e, logger)).toBe(false); + expect(handleRequestError(e, githubUrl, logger)).toBe(false); expect(logger.showErrorMessage).not.toHaveBeenCalled(); }); @@ -92,11 +93,11 @@ describe("handleRequestError", () => { }, ], }); - expect(handleRequestError(e, logger)).toBe(false); + expect(handleRequestError(e, githubUrl, logger)).toBe(false); expect(logger.showErrorMessage).not.toHaveBeenCalled(); }); - it("shows notification when handling a missing default branch error", () => { + it("shows notification when handling a missing default branch error with github.com URL", () => { 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.", @@ -110,11 +111,33 @@ describe("handleRequestError", () => { }, ], }); - expect(handleRequestError(e, logger)).toBe(true); + expect(handleRequestError(e, githubUrl, 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.", ); }); + + it("shows notification when handling a missing default branch error with GHEC-DR URL", () => { + 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, new URL("https://tenant.ghe.com"), 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://tenant.ghe.com/github/pickles/new/main) and re-run the variant analysis query.", + ); + }); }); function mockRequestError(status: number, body: any): RequestError { diff --git a/extensions/ql-vscode/test/vscode-tests/activated-extension/variant-analysis/variant-analysis-manager.test.ts b/extensions/ql-vscode/test/vscode-tests/activated-extension/variant-analysis/variant-analysis-manager.test.ts index 79a9c201b..c0173dbe9 100644 --- a/extensions/ql-vscode/test/vscode-tests/activated-extension/variant-analysis/variant-analysis-manager.test.ts +++ b/extensions/ql-vscode/test/vscode-tests/activated-extension/variant-analysis/variant-analysis-manager.test.ts @@ -82,12 +82,14 @@ describe("Variant Analysis Manager", () => { cli, extLogger, ); + const variantAnalysisConfig = createMockVariantAnalysisConfig(); variantAnalysisManager = new VariantAnalysisManager( app, cli, storagePath, variantAnalysisResultsManager, dbManager, + variantAnalysisConfig, ); }); diff --git a/extensions/ql-vscode/test/vscode-tests/cli-integration/variant-analysis/variant-analysis-manager.test.ts b/extensions/ql-vscode/test/vscode-tests/cli-integration/variant-analysis/variant-analysis-manager.test.ts index dbbc60023..5721a67d0 100644 --- a/extensions/ql-vscode/test/vscode-tests/cli-integration/variant-analysis/variant-analysis-manager.test.ts +++ b/extensions/ql-vscode/test/vscode-tests/cli-integration/variant-analysis/variant-analysis-manager.test.ts @@ -60,12 +60,14 @@ describe("Variant Analysis Manager", () => { cli, extLogger, ); + const variantAnalysisConfig = createMockVariantAnalysisConfig(); variantAnalysisManager = new VariantAnalysisManager( app, cli, storagePath, variantAnalysisResultsManager, dbManager, + variantAnalysisConfig, ); });