Skip to content

Commit

Permalink
Merge pull request #1927 from github/henrymercer/reduce-log-duplication
Browse files Browse the repository at this point in the history
Reduce duplication in the logs when errors occur in CLI commands
  • Loading branch information
henrymercer authored Oct 9, 2023
2 parents 2125352 + 83d1db3 commit 4ab9237
Show file tree
Hide file tree
Showing 10 changed files with 72 additions and 29 deletions.
2 changes: 1 addition & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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

0 comments on commit 4ab9237

Please sign in to comment.