Surface fatal CLI errors in interpret-results and run-queries

[angelapwen]

Our error logging often doesn't highlight the most relevant part of CLI errors, when they occur (see #1283 for an example, where the actual error from CLI was hidden amongst other emitted logs).

This change only addresses this specific case; it:

• adds an errorMatcher that will allow us to match any error from the CLI that includes "A fatal error occurred"
• uses the error matcher on the codeql database run-queries and codeql database interpret-results commands.

In the future, we will refactor the error logging mechanism or use the existing mechanism to match more classes of errors.

I have tested this change on a private repository (for Hubbers, link). Once the step fails, the log for the analyze step ends with:

which will allow users to scroll to the bottom of the log and find the appropriate error as they expect.

(It's perhaps not ideal that the stack trace shows the stack trace of the error matcher itself, which correctly matches the error and throws it, but this was the mechanism in place and I haven't been able to think of something better. The appropriate CLI error prints just before the stack trace).

Merge / deployment checklist

• Confirm this change is backwards compatible with existing workflows.
• Confirm the readme has been updated if necessary.
• Confirm the changelog has been updated if necessary.

[angelapwen]

I have some unit test errors resulting from my changes. Now that database interpret-command is called using toolRunnerErrorMatcher, when the database interpret-command tests run, they fail a line in the error matching code:

  Rejected promise returned by test. Reason:

  Error {
    message: 'Could not find program codeql-for-testing on PATH.',
  }

  › Object.safeWhich (node_modules/@chrisgavin/safe-which/src/index.ts:39:8)
  › toolrunnerErrorCatcher (src/toolrunner-error-catcher.ts:51:7)
  › Object.databaseInterpretResults (src/codeql.ts:1062:27)
  › <anonymous> (src/codeql.test.ts:893:3)

It looks like safeWhich tries to find the CodeQL object, but since we mocked it, it's not able to be found. I'm not sure how to stub this method in the error matcher so that it doesn't run, though.

[henrymercer]

It looks like safeWhich tries to find the CodeQL object, but since we mocked it, it's not able to be found. I'm not sure how to stub this method in the error matcher so that it doesn't run, though.

I think we'll need to stub toolrunnerErrorCatcher instead of the tool runner constructor, so we can check the arguments passed to toolrunnerErrorCatcher.

[angelapwen]

I think we'll need to stub toolrunnerErrorCatcher instead of the tool runner constructor, so we can check the arguments passed to toolrunnerErrorCatcher.

Ugh! Not sure how I missed that line. Thanks!!

[angelapwen]

Actually, I see that toolrunnerErrorCatcher calls toolRunner as well so I think the original test stands. I've stubbed the safeWhich method and the test is passing locally.

[henrymercer]

Nice!