Update custom errors to use GHEC-DR URL
This commit is contained in:
@@ -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<void>;
|
||||
}
|
||||
|
||||
@@ -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(
|
||||
|
||||
@@ -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);
|
||||
|
||||
@@ -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,
|
||||
|
||||
@@ -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;
|
||||
}
|
||||
|
||||
|
||||
@@ -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(),
|
||||
};
|
||||
}
|
||||
|
||||
@@ -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 {
|
||||
|
||||
@@ -82,12 +82,14 @@ describe("Variant Analysis Manager", () => {
|
||||
cli,
|
||||
extLogger,
|
||||
);
|
||||
const variantAnalysisConfig = createMockVariantAnalysisConfig();
|
||||
variantAnalysisManager = new VariantAnalysisManager(
|
||||
app,
|
||||
cli,
|
||||
storagePath,
|
||||
variantAnalysisResultsManager,
|
||||
dbManager,
|
||||
variantAnalysisConfig,
|
||||
);
|
||||
});
|
||||
|
||||
|
||||
@@ -60,12 +60,14 @@ describe("Variant Analysis Manager", () => {
|
||||
cli,
|
||||
extLogger,
|
||||
);
|
||||
const variantAnalysisConfig = createMockVariantAnalysisConfig();
|
||||
variantAnalysisManager = new VariantAnalysisManager(
|
||||
app,
|
||||
cli,
|
||||
storagePath,
|
||||
variantAnalysisResultsManager,
|
||||
dbManager,
|
||||
variantAnalysisConfig,
|
||||
);
|
||||
});
|
||||
|
||||
|
||||
Reference in New Issue
Block a user