Skip to content

Commit 926856f

Browse files
committed
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.
1 parent 0f82875 commit 926856f

3 files changed

Lines changed: 65 additions & 54 deletions

File tree

extensions/ql-vscode/CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
## [UNRELEASED]
44

55
- Allow using raw LGTM project slugs for fetching LGTM databases. [#769](https://github.com/github/vscode-codeql/pull/769)
6+
- Better error messages when BQRS interpretation fails to produce SARIF. [#770](https://github.com/github/vscode-codeql/pull/770)
67

78
## 1.4.3 - 22 February 2021
89

extensions/ql-vscode/src/cli.ts

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -603,8 +603,12 @@ export class CodeQLCliServer implements Disposable {
603603
let output: string;
604604
try {
605605
output = await fs.readFile(interpretedResultsPath, 'utf8');
606-
} catch (err) {
607-
throw new Error(`Reading output of interpretation failed: ${err.stderr || err}`);
606+
} catch (e) {
607+
const rawMessage = e.stderr || e.message;
608+
const errorMessage = rawMessage.startsWith('Cannot create a string')
609+
? `SARIF too large. ${rawMessage}`
610+
: rawMessage;
611+
throw new Error(`Reading output of interpretation failed: ${errorMessage}`);
608612
}
609613
try {
610614
return JSON.parse(output) as sarif.Log;

extensions/ql-vscode/src/interface.ts

Lines changed: 58 additions & 52 deletions
Original file line numberDiff line numberDiff line change
@@ -236,61 +236,67 @@ export class InterfaceManager extends DisposableObject {
236236
}
237237

238238
private async handleMsgFromView(msg: FromResultsViewMsg): Promise<void> {
239-
switch (msg.t) {
240-
case 'viewSourceFile': {
241-
await jumpToLocation(msg, this.databaseManager, this.logger);
242-
break;
243-
}
244-
case 'toggleDiagnostics': {
245-
if (msg.visible) {
246-
const databaseItem = this.databaseManager.findDatabaseItem(
247-
Uri.parse(msg.databaseUri)
248-
);
249-
if (databaseItem !== undefined) {
250-
await this.showResultsAsDiagnostics(
251-
msg.origResultsPaths,
252-
msg.metadata,
253-
databaseItem
239+
try {
240+
switch (msg.t) {
241+
case 'viewSourceFile': {
242+
await jumpToLocation(msg, this.databaseManager, this.logger);
243+
break;
244+
}
245+
case 'toggleDiagnostics': {
246+
if (msg.visible) {
247+
const databaseItem = this.databaseManager.findDatabaseItem(
248+
Uri.parse(msg.databaseUri)
254249
);
250+
if (databaseItem !== undefined) {
251+
await this.showResultsAsDiagnostics(
252+
msg.origResultsPaths,
253+
msg.metadata,
254+
databaseItem
255+
);
256+
}
257+
} else {
258+
// TODO: Only clear diagnostics on the same database.
259+
this._diagnosticCollection.clear();
255260
}
256-
} else {
257-
// TODO: Only clear diagnostics on the same database.
258-
this._diagnosticCollection.clear();
261+
break;
259262
}
260-
break;
263+
case 'resultViewLoaded':
264+
this._panelLoaded = true;
265+
this._panelLoadedCallBacks.forEach((cb) => cb());
266+
this._panelLoadedCallBacks = [];
267+
break;
268+
case 'changeSort':
269+
await this.changeRawSortState(msg.resultSetName, msg.sortState);
270+
break;
271+
case 'changeInterpretedSort':
272+
await this.changeInterpretedSortState(msg.sortState);
273+
break;
274+
case 'changePage':
275+
if (msg.selectedTable === ALERTS_TABLE_NAME) {
276+
await this.showPageOfInterpretedResults(msg.pageNumber);
277+
}
278+
else {
279+
await this.showPageOfRawResults(
280+
msg.selectedTable,
281+
msg.pageNumber,
282+
// When we are in an unsorted state, we guarantee that
283+
// sortedResultsInfo doesn't have an entry for the current
284+
// result set. Use this to determine whether or not we use
285+
// the sorted bqrs file.
286+
this._displayedQuery?.sortedResultsInfo.has(msg.selectedTable) || false
287+
);
288+
}
289+
break;
290+
case 'openFile':
291+
await this.openFile(msg.filePath);
292+
break;
293+
default:
294+
assertNever(msg);
261295
}
262-
case 'resultViewLoaded':
263-
this._panelLoaded = true;
264-
this._panelLoadedCallBacks.forEach((cb) => cb());
265-
this._panelLoadedCallBacks = [];
266-
break;
267-
case 'changeSort':
268-
await this.changeRawSortState(msg.resultSetName, msg.sortState);
269-
break;
270-
case 'changeInterpretedSort':
271-
await this.changeInterpretedSortState(msg.sortState);
272-
break;
273-
case 'changePage':
274-
if (msg.selectedTable === ALERTS_TABLE_NAME) {
275-
await this.showPageOfInterpretedResults(msg.pageNumber);
276-
}
277-
else {
278-
await this.showPageOfRawResults(
279-
msg.selectedTable,
280-
msg.pageNumber,
281-
// When we are in an unsorted state, we guarantee that
282-
// sortedResultsInfo doesn't have an entry for the current
283-
// result set. Use this to determine whether or not we use
284-
// the sorted bqrs file.
285-
this._displayedQuery?.sortedResultsInfo.has(msg.selectedTable) || false
286-
);
287-
}
288-
break;
289-
case 'openFile':
290-
await this.openFile(msg.filePath);
291-
break;
292-
default:
293-
assertNever(msg);
296+
} catch (e) {
297+
showAndLogErrorMessage(e.message, {
298+
fullMessage: e.stack
299+
});
294300
}
295301
}
296302

@@ -626,7 +632,7 @@ export class InterfaceManager extends DisposableObject {
626632
} catch (e) {
627633
// If interpretation fails, accept the error and continue
628634
// trying to render uninterpreted results anyway.
629-
this.logger.log(
635+
showAndLogErrorMessage(
630636
`Exception during results interpretation: ${e.message}. Will show raw results instead.`
631637
);
632638
}

0 commit comments

Comments
 (0)