Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Reduce duplication in the logs when errors occur in CLI commands #1927

Merged
merged 3 commits into from Oct 9, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion CHANGELOG.md
Expand Up @@ -4,7 +4,7 @@ See the [releases page](https://github.com/github/codeql-action/releases) for th

## [UNRELEASED]

No user facing changes.
- Improve the log output when an error occurs in an invocation of the CodeQL CLI. [#1927](https://github.com/github/codeql-action/pull/1927)

## 2.22.1 - 09 Oct 2023

Expand Down
6 changes: 1 addition & 5 deletions lib/analyze.js

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion lib/analyze.js.map

Large diffs are not rendered by default.

21 changes: 15 additions & 6 deletions lib/codeql.js

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion lib/codeql.js.map

Large diffs are not rendered by default.

16 changes: 14 additions & 2 deletions lib/codeql.test.js

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion lib/codeql.test.js.map

Large diffs are not rendered by default.

6 changes: 1 addition & 5 deletions src/analyze.ts
Expand Up @@ -394,14 +394,10 @@ export async function runQueries(
await runPrintLinesOfCode(language);
}
} catch (e) {
logger.info(String(e));
if (e instanceof Error) {
logger.info(e.stack!);
}
statusReport.analyze_failure_language = language;
throw new CodeQLAnalysisError(
statusReport,
`Error running analysis for ${language}: ${e}`,
`Error running analysis for ${language}: ${util.wrapError(e).message}`,
);
}
}
Expand Down
23 changes: 21 additions & 2 deletions src/codeql.test.ts
Expand Up @@ -1133,7 +1133,7 @@ test("database finalize does not override no code found error on CodeQL 2.12.4",
{
message:
'Encountered a fatal error while running "codeql-for-testing database finalize --finalize-dataset --threads=2 --ram=2048 db". ' +
`Exit code was 32 and error was: ${cliMessage}`,
`Exit code was 32 and last log line was: ${cliMessage} See the logs for more details.`,
},
);
});
Expand All @@ -1158,7 +1158,26 @@ test("runTool summarizes several fatal errors", async (t) => {
{
message:
'Encountered a fatal error while running "codeql-for-testing database finalize --finalize-dataset --threads=2 --ram=2048 db". ' +
`Exit code was 32 and error was: ${datasetImportError}. Context: ${heapError}.`,
`Exit code was 32 and error was: ${datasetImportError}. Context: ${heapError}. See the logs for more details.`,
},
);
});

test("runTool outputs last line of stderr if fatal error could not be found", async (t) => {
const cliStderr = "line1\nline2\nline3\nline4\nline5";
stubToolRunnerConstructor(32, cliStderr);
const codeqlObject = await codeql.getCodeQLForTesting();
sinon.stub(codeqlObject, "getVersion").resolves(makeVersionInfo("2.12.4"));
// safeWhich throws because of the test CodeQL object.
sinon.stub(safeWhich, "safeWhich").resolves("");

await t.throwsAsync(
async () =>
await codeqlObject.finalizeDatabase("db", "--threads=2", "--ram=2048"),
{
message:
'Encountered a fatal error while running "codeql-for-testing database finalize --finalize-dataset --threads=2 --ram=2048 db". ' +
"Exit code was 32 and last log line was: line5. See the logs for more details.",
},
);
});
Expand Down
21 changes: 16 additions & 5 deletions src/codeql.ts
Expand Up @@ -54,15 +54,27 @@ export class CommandInvocationError extends Error {
cmd: string,
args: string[],
public exitCode: number,
public error: string,
public output: string,
public stderr: string,
public stdout: string,
) {
const prettyCommand = [cmd, ...args]
.map((x) => (x.includes(" ") ? `'${x}'` : x))
.join(" ");

const fatalErrors = extractFatalErrors(stderr);
const lastLine = stderr.trim().split("\n").pop()?.trim();
let error = fatalErrors
? ` and error was: ${fatalErrors.trim()}`
: lastLine
? ` and last log line was: ${lastLine}`
: "";
if (error[error.length - 1] !== ".") {
error += ".";
}

super(
`Encountered a fatal error while running "${prettyCommand}". ` +
`Exit code was ${exitCode} and error was: ${error.trim()}`,
`Exit code was ${exitCode}${error} See the logs for more details.`,
);
}
}
Expand Down Expand Up @@ -1238,7 +1250,6 @@ async function runTool(
...(opts.stdin ? { input: Buffer.from(opts.stdin || "") } : {}),
}).exec();
if (exitCode !== 0) {
error = extractFatalErrors(error) || error;
throw new CommandInvocationError(cmd, args, exitCode, error, output);
}
return output;
Expand Down Expand Up @@ -1454,7 +1465,7 @@ function isNoCodeFoundError(e: CommandInvocationError): boolean {
*/
const javascriptNoCodeFoundWarning =
"No JavaScript or TypeScript code found.";
return e.exitCode === 32 || e.error.includes(javascriptNoCodeFoundWarning);
return e.exitCode === 32 || e.stderr.includes(javascriptNoCodeFoundWarning);
}

async function isDiagnosticsExportInvalidSarifFixed(
Expand Down