Improve CLI error messages

This commit is contained in:
Koen Vlaswinkel
2024-01-19 12:07:42 +01:00
parent f1a915e46c
commit a85a4b41f5
4 changed files with 222 additions and 13 deletions

View File

@@ -0,0 +1,102 @@
import { asError, getErrorMessage } from "../common/helpers-pure";
// https://docs.github.com/en/code-security/codeql-cli/using-the-advanced-functionality-of-the-codeql-cli/exit-codes
const EXIT_CODE_USER_ERROR = 2;
const EXIT_CODE_CANCELLED = 98;
export class ExitCodeError extends Error {
constructor(public readonly exitCode: number | null) {
super(`Process exited with code ${exitCode}`);
}
}
export class CliError extends Error {
constructor(
message: string,
public readonly stderr: string | undefined,
public readonly cause: Error,
public readonly commandDescription: string,
public readonly commandArgs: string[],
) {
super(message);
}
}
export function getCliError(
e: unknown,
stderr: string | undefined,
commandDescription: string,
commandArgs: string[],
): CliError {
const error = asError(e);
if (!(error instanceof ExitCodeError) || !stderr) {
return formatCliErrorFallback(
error,
stderr,
commandDescription,
commandArgs,
);
}
switch (error.exitCode) {
case EXIT_CODE_USER_ERROR: {
// This is an error that we should try to format nicely
const fatalErrorIndex = stderr.lastIndexOf("A fatal error occurred: ");
if (fatalErrorIndex !== -1) {
return new CliError(
stderr.slice(fatalErrorIndex),
stderr,
error,
commandDescription,
commandArgs,
);
}
break;
}
case EXIT_CODE_CANCELLED: {
const cancellationIndex = stderr.lastIndexOf(
"Computation was cancelled: ",
);
if (cancellationIndex !== -1) {
return new CliError(
stderr.slice(cancellationIndex),
stderr,
error,
commandDescription,
commandArgs,
);
}
break;
}
}
return formatCliErrorFallback(error, stderr, commandDescription, commandArgs);
}
function formatCliErrorFallback(
error: Error,
stderr: string | undefined,
commandDescription: string,
commandArgs: string[],
): CliError {
if (stderr) {
return new CliError(
stderr,
undefined,
error,
commandDescription,
commandArgs,
);
}
return new CliError(
getErrorMessage(error),
undefined,
error,
commandDescription,
commandArgs,
);
}

View File

@@ -35,6 +35,7 @@ import { LINE_ENDINGS, splitStreamAtSeparators } from "../common/split-stream";
import type { Position } from "../query-server/messages";
import { LOGGING_FLAGS } from "./cli-command";
import type { CliFeatures, VersionAndFeatures } from "./cli-version";
import { ExitCodeError, getCliError } from "./cli-errors";
/**
* The version of the SARIF format that we are using.
@@ -420,7 +421,9 @@ export class CodeQLCliServer implements Disposable {
stderrBuffers.push(newData);
});
// Listen for process exit.
process.addListener("close", (code) => reject(code));
process.addListener("close", (code) =>
reject(new ExitCodeError(code)),
);
// Write the command followed by a null terminator.
process.stdin.write(JSON.stringify(args), "utf8");
process.stdin.write(this.nullBuffer);
@@ -436,19 +439,18 @@ export class CodeQLCliServer implements Disposable {
} catch (err) {
// Kill the process if it isn't already dead.
this.killProcessIfRunning();
// Report the error (if there is a stderr then use that otherwise just report the error code or nodejs error)
const newError =
stderrBuffers.length === 0
? new Error(
`${description} failed with args:${EOL} ${argsString}${EOL}${err}`,
)
: new Error(
`${description} failed with args:${EOL} ${argsString}${EOL}${Buffer.concat(
stderrBuffers,
).toString("utf8")}`,
);
newError.stack += getErrorStack(err);
throw newError;
const cliError = getCliError(
err,
stderrBuffers.length > 0
? Buffer.concat(stderrBuffers).toString("utf8")
: undefined,
description,
args,
);
cliError.stack += getErrorStack(err);
throw cliError;
} finally {
if (!silent) {
void this.logger.log(Buffer.concat(stderrBuffers).toString("utf8"));

View File

@@ -13,6 +13,8 @@ import { redactableError } from "../../common/errors";
import { UserCancellationException } from "./progress";
import { telemetryListener } from "./telemetry";
import type { AppTelemetry } from "../telemetry";
import { CliError } from "../../codeql-cli/cli-errors";
import { EOL } from "os";
/**
* Create a command manager for VSCode, wrapping registerCommandWithErrorHandling
@@ -62,6 +64,16 @@ export function registerCommandWithErrorHandling(
} else {
void showAndLogWarningMessage(logger, errorMessage.fullMessage);
}
} else if (e instanceof CliError) {
const fullMessage = `${e.commandDescription} failed with args:${EOL}${
e.stderr ?? e.cause
} ${e.commandArgs.join(" ")}`;
void showAndLogExceptionWithTelemetry(logger, telemetry, errorMessage, {
fullMessage,
extraTelemetryProperties: {
command: commandId,
},
});
} else {
// Include the full stack in the error log only.
const fullMessage = errorMessage.fullMessageWithStack;

View File

@@ -0,0 +1,93 @@
import {
CliError,
ExitCodeError,
getCliError,
} from "../../../src/codeql-cli/cli-errors";
import { EOL } from "os";
describe("getCliError", () => {
it("returns an error with an unknown error", () => {
const error = new Error("foo");
expect(getCliError(error, undefined, "bar", ["baz"])).toEqual(
new CliError("foo", undefined, error, "bar", ["baz"]),
);
});
it("returns an error with an unknown error with stderr", () => {
const error = new Error("foo");
expect(getCliError(error, "Something failed", "bar", ["baz"])).toEqual(
new CliError("Something failed", "Something failed", error, "bar", [
"baz",
]),
);
});
it("returns an error with an unknown error with stderr", () => {
const error = new Error("foo");
expect(getCliError(error, "Something failed", "bar", ["baz"])).toEqual(
new CliError("Something failed", "Something failed", error, "bar", [
"baz",
]),
);
});
it("returns an error with an exit code error with unhandled exit code", () => {
const error = new ExitCodeError(99); // OOM
expect(getCliError(error, "OOM!", "bar", ["baz"])).toEqual(
new CliError("OOM!", "OOM!", error, "bar", ["baz"]),
);
});
it("returns an error with an exit code error with handled exit code without string", () => {
const error = new ExitCodeError(2);
expect(getCliError(error, "Something happened!", "bar", ["baz"])).toEqual(
new CliError("Something happened!", "Something happened!", error, "bar", [
"baz",
]),
);
});
it("returns an error with a user code error with identifying string", () => {
const error = new ExitCodeError(2);
const stderr = `Something happened!${EOL}A fatal error occurred: The query did not run successfully.${EOL}The correct columns were not present.`;
expect(getCliError(error, stderr, "bar", ["baz"])).toEqual(
new CliError(
`A fatal error occurred: The query did not run successfully.${EOL}The correct columns were not present.`,
stderr,
error,
"bar",
["baz"],
),
);
});
it("returns an error with a user code error with cancelled string", () => {
const error = new ExitCodeError(2);
const stderr = `Running query...${EOL}Something is happening...${EOL}Computation was cancelled: Cancelled by user`;
expect(getCliError(error, stderr, "bar", ["baz"])).toEqual(
new CliError(stderr, stderr, error, "bar", ["baz"]),
);
});
it("returns an error with a cancelled error with identifying string", () => {
const error = new ExitCodeError(98);
const stderr = `Running query...${EOL}Something is happening...${EOL}Computation was cancelled: Cancelled by user`;
expect(getCliError(error, stderr, "bar", ["baz"])).toEqual(
new CliError(
"Computation was cancelled: Cancelled by user",
stderr,
error,
"bar",
["baz"],
),
);
});
});