Skip to content

Commit 31b2d24

Browse files
committed
Update custom errors to use GHEC-DR URL
1 parent 67aa216 commit 31b2d24

File tree

8 files changed

+91
-14
lines changed

8 files changed

+91
-14
lines changed

extensions/ql-vscode/src/config.ts

Lines changed: 38 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -108,12 +108,36 @@ export function hasEnterpriseUri(): boolean {
108108
return getEnterpriseUri() !== undefined;
109109
}
110110

111+
/**
112+
* Does the uri look like GHEC-DR?
113+
*/
114+
function isGhecDrUri(uri: Uri | undefined): boolean {
115+
return uri !== undefined && uri.authority.toLowerCase().endsWith(".ghe.com");
116+
}
117+
111118
/**
112119
* Is the GitHub Enterprise URI set to something that looks like GHEC-DR?
113120
*/
114121
export function hasGhecDrUri(): boolean {
115122
const uri = getEnterpriseUri();
116-
return uri !== undefined && uri.authority.toLowerCase().endsWith(".ghe.com");
123+
return isGhecDrUri(uri);
124+
}
125+
126+
/**
127+
* The URI for GitHub.com.
128+
*/
129+
export const GITHUB_URL = new URL("https://github.com");
130+
131+
/**
132+
* If the GitHub Enterprise URI is set to something that looks like GHEC-DR, return it.
133+
*/
134+
export function getGhecDrUri(): Uri | undefined {
135+
const uri = getEnterpriseUri();
136+
if (isGhecDrUri(uri)) {
137+
return uri;
138+
} else {
139+
return undefined;
140+
}
117141
}
118142

119143
const ROOT_SETTING = new Setting("codeQL");
@@ -570,6 +594,11 @@ export async function setRemoteControllerRepo(repo: string | undefined) {
570594
export interface VariantAnalysisConfig {
571595
controllerRepo: string | undefined;
572596
showSystemDefinedRepositoryLists: boolean;
597+
/**
598+
* This uses a URL instead of a URI because the URL class is available in
599+
* unit tests and is fully browser-compatible.
600+
*/
601+
githubUrl: URL;
573602
onDidChangeConfiguration?: Event<void>;
574603
}
575604

@@ -591,6 +620,14 @@ export class VariantAnalysisConfigListener
591620
public get showSystemDefinedRepositoryLists(): boolean {
592621
return !hasEnterpriseUri();
593622
}
623+
624+
public get githubUrl(): URL {
625+
const ghecDrUri = getGhecDrUri();
626+
if (ghecDrUri) {
627+
return new URL(ghecDrUri.toString());
628+
}
629+
return GITHUB_URL;
630+
}
594631
}
595632

596633
const VARIANT_ANALYSIS_FILTER_RESULTS = new Setting(

extensions/ql-vscode/src/extension.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@ import {
3131
joinOrderWarningThreshold,
3232
QueryHistoryConfigListener,
3333
QueryServerConfigListener,
34+
VariantAnalysisConfigListener,
3435
} from "./config";
3536
import {
3637
AstViewer,
@@ -875,6 +876,7 @@ async function activateWithInstalledDistribution(
875876
variantAnalysisStorageDir,
876877
variantAnalysisResultsManager,
877878
dbModule.dbManager,
879+
new VariantAnalysisConfigListener(),
878880
);
879881
ctx.subscriptions.push(variantAnalysisManager);
880882
ctx.subscriptions.push(variantAnalysisResultsManager);

extensions/ql-vscode/src/variant-analysis/custom-errors.ts

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ type ErrorResponse = {
1515

1616
export function handleRequestError(
1717
e: RequestError,
18+
githubUrl: URL,
1819
logger: NotificationLogger,
1920
): boolean {
2021
if (e.status !== 422) {
@@ -60,9 +61,12 @@ export function handleRequestError(
6061
return false;
6162
}
6263

63-
const createBranchURL = `https://github.com/${
64-
missingDefaultBranchError.repository
65-
}/new/${encodeURIComponent(missingDefaultBranchError.default_branch)}`;
64+
const createBranchURL = new URL(
65+
`/${
66+
missingDefaultBranchError.repository
67+
}/new/${encodeURIComponent(missingDefaultBranchError.default_branch)}`,
68+
githubUrl,
69+
).toString();
6670

6771
void showAndLogErrorMessage(
6872
logger,

extensions/ql-vscode/src/variant-analysis/variant-analysis-manager.ts

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -98,6 +98,7 @@ import { findVariantAnalysisQlPackRoot } from "./ql";
9898
import { resolveCodeScanningQueryPack } from "./code-scanning-pack";
9999
import { isSarifResultsQueryKind } from "../common/query-metadata";
100100
import { isVariantAnalysisEnabledForGitHubHost } from "./ghec-dr";
101+
import type { VariantAnalysisConfig } from "../config";
101102
import { getEnterpriseUri } from "../config";
102103

103104
const maxRetryCount = 3;
@@ -158,6 +159,7 @@ export class VariantAnalysisManager
158159
private readonly storagePath: string,
159160
private readonly variantAnalysisResultsManager: VariantAnalysisResultsManager,
160161
private readonly dbManager: DbManager,
162+
private readonly config: VariantAnalysisConfig,
161163
) {
162164
super();
163165
this.variantAnalysisMonitor = this.push(
@@ -426,7 +428,10 @@ export class VariantAnalysisManager
426428
);
427429
} catch (e: unknown) {
428430
// If the error is handled by the handleRequestError function, we don't need to throw
429-
if (e instanceof RequestError && handleRequestError(e, this.app.logger)) {
431+
if (
432+
e instanceof RequestError &&
433+
handleRequestError(e, this.config.githubUrl, this.app.logger)
434+
) {
430435
return undefined;
431436
}
432437

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,11 @@
11
import type { VariantAnalysisConfig } from "../../src/config";
2+
import { GITHUB_URL } from "../../src/config";
23

34
export function createMockVariantAnalysisConfig(): VariantAnalysisConfig {
45
return {
56
controllerRepo: "foo/bar",
67
showSystemDefinedRepositoryLists: true,
8+
githubUrl: GITHUB_URL,
79
onDidChangeConfiguration: jest.fn(),
810
};
911
}

extensions/ql-vscode/test/unit-tests/variant-analysis/custom-errors.test.ts

Lines changed: 32 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -4,13 +4,14 @@ import { handleRequestError } from "../../../src/variant-analysis/custom-errors"
44
import { faker } from "@faker-js/faker";
55

66
describe("handleRequestError", () => {
7+
const githubUrl = new URL("https://github.com");
78
const logger = createMockLogger();
89

910
it("returns false when handling a non-422 error", () => {
1011
const e = mockRequestError(404, {
1112
message: "Not Found",
1213
});
13-
expect(handleRequestError(e, logger)).toBe(false);
14+
expect(handleRequestError(e, githubUrl, logger)).toBe(false);
1415
expect(logger.showErrorMessage).not.toHaveBeenCalled();
1516
});
1617

@@ -19,13 +20,13 @@ describe("handleRequestError", () => {
1920
message:
2021
"Unable to trigger a variant analysis. None of the requested repositories could be found.",
2122
});
22-
expect(handleRequestError(e, logger)).toBe(false);
23+
expect(handleRequestError(e, githubUrl, logger)).toBe(false);
2324
expect(logger.showErrorMessage).not.toHaveBeenCalled();
2425
});
2526

2627
it("returns false when handling an error without response body", () => {
2728
const e = mockRequestError(422, undefined);
28-
expect(handleRequestError(e, logger)).toBe(false);
29+
expect(handleRequestError(e, githubUrl, logger)).toBe(false);
2930
expect(logger.showErrorMessage).not.toHaveBeenCalled();
3031
});
3132

@@ -42,7 +43,7 @@ describe("handleRequestError", () => {
4243
},
4344
},
4445
});
45-
expect(handleRequestError(e, logger)).toBe(false);
46+
expect(handleRequestError(e, githubUrl, logger)).toBe(false);
4647
expect(logger.showErrorMessage).not.toHaveBeenCalled();
4748
});
4849

@@ -58,7 +59,7 @@ describe("handleRequestError", () => {
5859
},
5960
],
6061
});
61-
expect(handleRequestError(e, logger)).toBe(false);
62+
expect(handleRequestError(e, githubUrl, logger)).toBe(false);
6263
expect(logger.showErrorMessage).not.toHaveBeenCalled();
6364
});
6465

@@ -75,7 +76,7 @@ describe("handleRequestError", () => {
7576
},
7677
],
7778
});
78-
expect(handleRequestError(e, logger)).toBe(false);
79+
expect(handleRequestError(e, githubUrl, logger)).toBe(false);
7980
expect(logger.showErrorMessage).not.toHaveBeenCalled();
8081
});
8182

@@ -92,11 +93,11 @@ describe("handleRequestError", () => {
9293
},
9394
],
9495
});
95-
expect(handleRequestError(e, logger)).toBe(false);
96+
expect(handleRequestError(e, githubUrl, logger)).toBe(false);
9697
expect(logger.showErrorMessage).not.toHaveBeenCalled();
9798
});
9899

99-
it("shows notification when handling a missing default branch error", () => {
100+
it("shows notification when handling a missing default branch error with github.com URL", () => {
100101
const e = mockRequestError(422, {
101102
message:
102103
"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", () => {
110111
},
111112
],
112113
});
113-
expect(handleRequestError(e, logger)).toBe(true);
114+
expect(handleRequestError(e, githubUrl, logger)).toBe(true);
114115
expect(logger.showErrorMessage).toHaveBeenCalledWith(
115116
"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.",
116117
);
117118
});
119+
120+
it("shows notification when handling a missing default branch error with GHEC-DR URL", () => {
121+
const e = mockRequestError(422, {
122+
message:
123+
"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.",
124+
errors: [
125+
{
126+
resource: "Repository",
127+
field: "default_branch",
128+
code: "missing",
129+
repository: "github/pickles",
130+
default_branch: "main",
131+
},
132+
],
133+
});
134+
expect(
135+
handleRequestError(e, new URL("https://tenant.ghe.com"), logger),
136+
).toBe(true);
137+
expect(logger.showErrorMessage).toHaveBeenCalledWith(
138+
"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.",
139+
);
140+
});
118141
});
119142

120143
function mockRequestError(status: number, body: any): RequestError {

extensions/ql-vscode/test/vscode-tests/activated-extension/variant-analysis/variant-analysis-manager.test.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -82,12 +82,14 @@ describe("Variant Analysis Manager", () => {
8282
cli,
8383
extLogger,
8484
);
85+
const variantAnalysisConfig = createMockVariantAnalysisConfig();
8586
variantAnalysisManager = new VariantAnalysisManager(
8687
app,
8788
cli,
8889
storagePath,
8990
variantAnalysisResultsManager,
9091
dbManager,
92+
variantAnalysisConfig,
9193
);
9294
});
9395

extensions/ql-vscode/test/vscode-tests/cli-integration/variant-analysis/variant-analysis-manager.test.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -60,12 +60,14 @@ describe("Variant Analysis Manager", () => {
6060
cli,
6161
extLogger,
6262
);
63+
const variantAnalysisConfig = createMockVariantAnalysisConfig();
6364
variantAnalysisManager = new VariantAnalysisManager(
6465
app,
6566
cli,
6667
storagePath,
6768
variantAnalysisResultsManager,
6869
dbManager,
70+
variantAnalysisConfig,
6971
);
7072
});
7173

0 commit comments

Comments
 (0)