Skip to content

Commit 978e48f

Browse files
koesie10koesie10
authored
Merge pull request #3259 from github/koesie10/improve-error-message
Improve CLI error messages
2 parents dc8062b + ac5ed7b commit 978e48f

File tree

5 files changed

+223
-13
lines changed

5 files changed

+223
-13
lines changed

‎extensions/ql-vscode/CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
- Enable collection of telemetry for the `codeQL.addingDatabases.addDatabaseSourceToWorkspace` setting. [#3238](https://github.com/github/vscode-codeql/pull/3238)
66
- In the CodeQL model editor, you can now select individual method rows and save changes to only the selected rows, instead of having to save the entire library model. [#3156](https://github.com/github/vscode-codeql/pull/3156)
77
- If you run a query without having selected a database, we show a more intuitive prompt to help you select a database. [#3214](https://github.com/github/vscode-codeql/pull/3214)
8+
- Error messages returned from the CodeQL CLI are now less verbose and more user-friendly. [#3259](https://github.com/github/vscode-codeql/pull/3259)
89
- The UI for browsing and running CodeQL tests has moved to use VS Code's built-in test UI. This makes the CodeQL test UI more consistent with the test UIs for other languages.
910
This change means that this extension no longer depends on the "Test Explorer UI" and "Test Adapter Converter" extensions. You can uninstall those two extensions if they are
1011
not being used by any other extensions you may have installed.

‎extensions/ql-vscode/src/codeql-cli/cli-errors.ts

Lines changed: 102 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,102 @@
1+
import { asError, getErrorMessage } from "../common/helpers-pure";
2+
3+
// https://docs.github.com/en/code-security/codeql-cli/using-the-advanced-functionality-of-the-codeql-cli/exit-codes
4+
const EXIT_CODE_USER_ERROR = 2;
5+
const EXIT_CODE_CANCELLED = 98;
6+
7+
export class ExitCodeError extends Error {
8+
constructor(public readonly exitCode: number | null) {
9+
super(`Process exited with code ${exitCode}`);
10+
}
11+
}
12+
13+
export class CliError extends Error {
14+
constructor(
15+
message: string,
16+
public readonly stderr: string | undefined,
17+
public readonly cause: Error,
18+
public readonly commandDescription: string,
19+
public readonly commandArgs: string[],
20+
) {
21+
super(message);
22+
}
23+
}
24+
25+
export function getCliError(
26+
e: unknown,
27+
stderr: string | undefined,
28+
commandDescription: string,
29+
commandArgs: string[],
30+
): CliError {
31+
const error = asError(e);
32+
33+
if (!(error instanceof ExitCodeError) || !stderr) {
34+
return formatCliErrorFallback(
35+
error,
36+
stderr,
37+
commandDescription,
38+
commandArgs,
39+
);
40+
}
41+
42+
switch (error.exitCode) {
43+
case EXIT_CODE_USER_ERROR: {
44+
// This is an error that we should try to format nicely
45+
const fatalErrorIndex = stderr.lastIndexOf("A fatal error occurred: ");
46+
if (fatalErrorIndex !== -1) {
47+
return new CliError(
48+
stderr.slice(fatalErrorIndex),
49+
stderr,
50+
error,
51+
commandDescription,
52+
commandArgs,
53+
);
54+
}
55+
56+
break;
57+
}
58+
case EXIT_CODE_CANCELLED: {
59+
const cancellationIndex = stderr.lastIndexOf(
60+
"Computation was cancelled: ",
61+
);
62+
if (cancellationIndex !== -1) {
63+
return new CliError(
64+
stderr.slice(cancellationIndex),
65+
stderr,
66+
error,
67+
commandDescription,
68+
commandArgs,
69+
);
70+
}
71+
72+
break;
73+
}
74+
}
75+
76+
return formatCliErrorFallback(error, stderr, commandDescription, commandArgs);
77+
}
78+
79+
function formatCliErrorFallback(
80+
error: Error,
81+
stderr: string | undefined,
82+
commandDescription: string,
83+
commandArgs: string[],
84+
): CliError {
85+
if (stderr) {
86+
return new CliError(
87+
stderr,
88+
undefined,
89+
error,
90+
commandDescription,
91+
commandArgs,
92+
);
93+
}
94+
95+
return new CliError(
96+
getErrorMessage(error),
97+
undefined,
98+
error,
99+
commandDescription,
100+
commandArgs,
101+
);
102+
}

‎extensions/ql-vscode/src/codeql-cli/cli.ts

Lines changed: 15 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@ import { LINE_ENDINGS, splitStreamAtSeparators } from "../common/split-stream";
3535
import type { Position } from "../query-server/messages";
3636
import { LOGGING_FLAGS } from "./cli-command";
3737
import type { CliFeatures, VersionAndFeatures } from "./cli-version";
38+
import { ExitCodeError, getCliError } from "./cli-errors";
3839

3940
/**
4041
* The version of the SARIF format that we are using.
@@ -420,7 +421,9 @@ export class CodeQLCliServer implements Disposable {
420421
stderrBuffers.push(newData);
421422
});
422423
// Listen for process exit.
423-
process.addListener("close", (code) => reject(code));
424+
process.addListener("close", (code) =>
425+
reject(new ExitCodeError(code)),
426+
);
424427
// Write the command followed by a null terminator.
425428
process.stdin.write(JSON.stringify(args), "utf8");
426429
process.stdin.write(this.nullBuffer);
@@ -436,19 +439,18 @@ export class CodeQLCliServer implements Disposable {
436439
} catch (err) {
437440
// Kill the process if it isn't already dead.
438441
this.killProcessIfRunning();
442+
439443
// Report the error (if there is a stderr then use that otherwise just report the error code or nodejs error)
440-
const newError =
441-
stderrBuffers.length === 0
442-
? new Error(
443-
`${description} failed with args:${EOL} ${argsString}${EOL}${err}`,
444-
)
445-
: new Error(
446-
`${description} failed with args:${EOL} ${argsString}${EOL}${Buffer.concat(
447-
stderrBuffers,
448-
).toString("utf8")}`,
449-
);
450-
newError.stack += getErrorStack(err);
451-
throw newError;
444+
const cliError = getCliError(
445+
err,
446+
stderrBuffers.length > 0
447+
? Buffer.concat(stderrBuffers).toString("utf8")
448+
: undefined,
449+
description,
450+
args,
451+
);
452+
cliError.stack += getErrorStack(err);
453+
throw cliError;
452454
} finally {
453455
if (!silent) {
454456
void this.logger.log(Buffer.concat(stderrBuffers).toString("utf8"));

‎extensions/ql-vscode/src/common/vscode/commands.ts

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,8 @@ import { redactableError } from "../../common/errors";
1313
import { UserCancellationException } from "./progress";
1414
import { telemetryListener } from "./telemetry";
1515
import type { AppTelemetry } from "../telemetry";
16+
import { CliError } from "../../codeql-cli/cli-errors";
17+
import { EOL } from "os";
1618

1719
/**
1820
* Create a command manager for VSCode, wrapping registerCommandWithErrorHandling
@@ -62,6 +64,16 @@ export function registerCommandWithErrorHandling(
6264
} else {
6365
void showAndLogWarningMessage(logger, errorMessage.fullMessage);
6466
}
67+
} else if (e instanceof CliError) {
68+
const fullMessage = `${e.commandDescription} failed with args:${EOL} ${e.commandArgs.join(" ")}${EOL}${
69+
e.stderr ?? e.cause
70+
}`;
71+
void showAndLogExceptionWithTelemetry(logger, telemetry, errorMessage, {
72+
fullMessage,
73+
extraTelemetryProperties: {
74+
command: commandId,
75+
},
76+
});
6577
} else {
6678
// Include the full stack in the error log only.
6779
const fullMessage = errorMessage.fullMessageWithStack;

‎extensions/ql-vscode/test/unit-tests/codeql-cli/cli-errors.test.ts

Lines changed: 93 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,93 @@
1+
import {
2+
CliError,
3+
ExitCodeError,
4+
getCliError,
5+
} from "../../../src/codeql-cli/cli-errors";
6+
import { EOL } from "os";
7+
8+
describe("getCliError", () => {
9+
it("returns an error with an unknown error", () => {
10+
const error = new Error("foo");
11+
12+
expect(getCliError(error, undefined, "bar", ["baz"])).toEqual(
13+
new CliError("foo", undefined, error, "bar", ["baz"]),
14+
);
15+
});
16+
17+
it("returns an error with an unknown error with stderr", () => {
18+
const error = new Error("foo");
19+
20+
expect(getCliError(error, "Something failed", "bar", ["baz"])).toEqual(
21+
new CliError("Something failed", "Something failed", error, "bar", [
22+
"baz",
23+
]),
24+
);
25+
});
26+
27+
it("returns an error with an unknown error with stderr", () => {
28+
const error = new Error("foo");
29+
30+
expect(getCliError(error, "Something failed", "bar", ["baz"])).toEqual(
31+
new CliError("Something failed", "Something failed", error, "bar", [
32+
"baz",
33+
]),
34+
);
35+
});
36+
37+
it("returns an error with an exit code error with unhandled exit code", () => {
38+
const error = new ExitCodeError(99); // OOM
39+
40+
expect(getCliError(error, "OOM!", "bar", ["baz"])).toEqual(
41+
new CliError("OOM!", "OOM!", error, "bar", ["baz"]),
42+
);
43+
});
44+
45+
it("returns an error with an exit code error with handled exit code without string", () => {
46+
const error = new ExitCodeError(2);
47+
48+
expect(getCliError(error, "Something happened!", "bar", ["baz"])).toEqual(
49+
new CliError("Something happened!", "Something happened!", error, "bar", [
50+
"baz",
51+
]),
52+
);
53+
});
54+
55+
it("returns an error with a user code error with identifying string", () => {
56+
const error = new ExitCodeError(2);
57+
const stderr = `Something happened!${EOL}A fatal error occurred: The query did not run successfully.${EOL}The correct columns were not present.`;
58+
59+
expect(getCliError(error, stderr, "bar", ["baz"])).toEqual(
60+
new CliError(
61+
`A fatal error occurred: The query did not run successfully.${EOL}The correct columns were not present.`,
62+
stderr,
63+
error,
64+
"bar",
65+
["baz"],
66+
),
67+
);
68+
});
69+
70+
it("returns an error with a user code error with cancelled string", () => {
71+
const error = new ExitCodeError(2);
72+
const stderr = `Running query...${EOL}Something is happening...${EOL}Computation was cancelled: Cancelled by user`;
73+
74+
expect(getCliError(error, stderr, "bar", ["baz"])).toEqual(
75+
new CliError(stderr, stderr, error, "bar", ["baz"]),
76+
);
77+
});
78+
79+
it("returns an error with a cancelled error with identifying string", () => {
80+
const error = new ExitCodeError(98);
81+
const stderr = `Running query...${EOL}Something is happening...${EOL}Computation was cancelled: Cancelled by user`;
82+
83+
expect(getCliError(error, stderr, "bar", ["baz"])).toEqual(
84+
new CliError(
85+
"Computation was cancelled: Cancelled by user",
86+
stderr,
87+
error,
88+
"bar",
89+
["baz"],
90+
),
91+
);
92+
});
93+
});

0 commit comments

Comments
 (0)