Add error message when interpretation fails

One way it can fail is if the SARIF is too large. We explicitly call
out that error because the raw message received from the node runtime
is not very understandable.

Author: aeisenberg

extensions/ql-vscode/CHANGELOG.md

@@ -4,6 +4,7 @@
 
 - Respect the `codeQL.runningQueries.numberOfThreads` setting when creating SARIF files during result interpretation. [#771](https://github.com/github/vscode-codeql/pull/771)
 - Allow using raw LGTM project slugs for fetching LGTM databases. [#769](https://github.com/github/vscode-codeql/pull/769)
+- Better error messages when BQRS interpretation fails to produce SARIF. [#770](https://github.com/github/vscode-codeql/pull/770)
 
 ## 1.4.3 - 22 February 2021
 

extensions/ql-vscode/src/cli.ts

@@ -609,8 +609,12 @@ export class CodeQLCliServer implements Disposable {
     let output: string;
     try {
       output = await fs.readFile(interpretedResultsPath, 'utf8');
-    } catch (err) {
-      throw new Error(`Reading output of interpretation failed: ${err.stderr || err}`);
+    } catch (e) {
+      const rawMessage = e.stderr || e.message;
+      const errorMessage = rawMessage.startsWith('Cannot create a string')
+        ? `SARIF too large. ${rawMessage}`
+        : rawMessage;
+      throw new Error(`Reading output of interpretation failed: ${errorMessage}`);
     }
     try {
       return JSON.parse(output) as sarif.Log;

extensions/ql-vscode/src/interface.ts

@@ -236,61 +236,67 @@ export class InterfaceManager extends DisposableObject {
   }
 
   private async handleMsgFromView(msg: FromResultsViewMsg): Promise<void> {
-    switch (msg.t) {
-      case 'viewSourceFile': {
-        await jumpToLocation(msg, this.databaseManager, this.logger);
-        break;
-      }
-      case 'toggleDiagnostics': {
-        if (msg.visible) {
-          const databaseItem = this.databaseManager.findDatabaseItem(
-            Uri.parse(msg.databaseUri)
-          );
-          if (databaseItem !== undefined) {
-            await this.showResultsAsDiagnostics(
-              msg.origResultsPaths,
-              msg.metadata,
-              databaseItem
+    try {
+      switch (msg.t) {
+        case 'viewSourceFile': {
+          await jumpToLocation(msg, this.databaseManager, this.logger);
+          break;
+        }
+        case 'toggleDiagnostics': {
+          if (msg.visible) {
+            const databaseItem = this.databaseManager.findDatabaseItem(
+              Uri.parse(msg.databaseUri)
             );
+            if (databaseItem !== undefined) {
+              await this.showResultsAsDiagnostics(
+                msg.origResultsPaths,
+                msg.metadata,
+                databaseItem
+              );
+            }
+          } else {
+            // TODO: Only clear diagnostics on the same database.
+            this._diagnosticCollection.clear();
           }
-        } else {
-          // TODO: Only clear diagnostics on the same database.
-          this._diagnosticCollection.clear();
+          break;
         }
-        break;
+        case 'resultViewLoaded':
+          this._panelLoaded = true;
+          this._panelLoadedCallBacks.forEach((cb) => cb());
+          this._panelLoadedCallBacks = [];
+          break;
+        case 'changeSort':
+          await this.changeRawSortState(msg.resultSetName, msg.sortState);
+          break;
+        case 'changeInterpretedSort':
+          await this.changeInterpretedSortState(msg.sortState);
+          break;
+        case 'changePage':
+          if (msg.selectedTable === ALERTS_TABLE_NAME) {
+            await this.showPageOfInterpretedResults(msg.pageNumber);
+          }
+          else {
+            await this.showPageOfRawResults(
+              msg.selectedTable,
+              msg.pageNumber,
+              // When we are in an unsorted state, we guarantee that
+              // sortedResultsInfo doesn't have an entry for the current
+              // result set. Use this to determine whether or not we use
+              // the sorted bqrs file.
+              this._displayedQuery?.sortedResultsInfo.has(msg.selectedTable) || false
+            );
+          }
+          break;
+        case 'openFile':
+          await this.openFile(msg.filePath);
+          break;
+        default:
+          assertNever(msg);
       }
-      case 'resultViewLoaded':
-        this._panelLoaded = true;
-        this._panelLoadedCallBacks.forEach((cb) => cb());
-        this._panelLoadedCallBacks = [];
-        break;
-      case 'changeSort':
-        await this.changeRawSortState(msg.resultSetName, msg.sortState);
-        break;
-      case 'changeInterpretedSort':
-        await this.changeInterpretedSortState(msg.sortState);
-        break;
-      case 'changePage':
-        if (msg.selectedTable === ALERTS_TABLE_NAME) {
-          await this.showPageOfInterpretedResults(msg.pageNumber);
-        }
-        else {
-          await this.showPageOfRawResults(
-            msg.selectedTable,
-            msg.pageNumber,
-            // When we are in an unsorted state, we guarantee that
-            // sortedResultsInfo doesn't have an entry for the current
-            // result set. Use this to determine whether or not we use
-            // the sorted bqrs file.
-            this._displayedQuery?.sortedResultsInfo.has(msg.selectedTable) || false
-          );
-        }
-        break;
-      case 'openFile':
-        await this.openFile(msg.filePath);
-        break;
-      default:
-        assertNever(msg);
+    } catch (e) {
+      showAndLogErrorMessage(e.message, {
+        fullMessage: e.stack
+      });
     }
   }
 
@@ -626,7 +632,7 @@ export class InterfaceManager extends DisposableObject {
       } catch (e) {
         // If interpretation fails, accept the error and continue
         // trying to render uninterpreted results anyway.
-        this.logger.log(
+        showAndLogErrorMessage(
           `Exception during results interpretation: ${e.message}. Will show raw results instead.`
         );
       }