Skip to content

Commit 9868197

Browse files
authored
Do not crash entire extension when analysis fails (#880)
* do not crash entire extension when analysis fails, instead report the error as a message but only every 15 minutes * changelog
1 parent 80379fe commit 9868197

File tree

4 files changed

+53
-3
lines changed

4 files changed

+53
-3
lines changed

CHANGELOG.md

+2
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,8 @@
1212
1313
## master
1414

15+
- Better error recovery when analysis fails. https://github.com/rescript-lang/rescript-vscode/pull/880
16+
1517
## 1.32.0
1618

1719
- Expand type aliases in hovers. https://github.com/rescript-lang/rescript-vscode/pull/881

server/src/errorReporter.ts

+22
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
type cb = (msg: string) => void;
2+
3+
let subscribers: Array<cb> = [];
4+
const errorLastNotified: Record<string, number> = {};
5+
6+
export const onErrorReported = (cb: (msg: string) => void) => {
7+
subscribers.push(cb);
8+
return () => {
9+
subscribers = subscribers.filter((s) => s !== cb);
10+
};
11+
};
12+
13+
export const reportError = (identifier: string, msg: string) => {
14+
// Warn once per 15 min per error
15+
if (
16+
errorLastNotified[identifier] == null ||
17+
errorLastNotified[identifier] < Date.now() - 15 * 1000 * 60
18+
) {
19+
errorLastNotified[identifier] = Date.now();
20+
subscribers.forEach((cb) => cb(msg));
21+
}
22+
};

server/src/server.ts

+19
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ import { fileURLToPath } from "url";
2525
import { ChildProcess } from "child_process";
2626
import { WorkspaceEdit } from "vscode-languageserver";
2727
import { filesDiagnostics } from "./utils";
28+
import { onErrorReported } from "./errorReporter";
2829

2930
interface extensionConfiguration {
3031
allowBuiltInFormatter: boolean;
@@ -1306,3 +1307,21 @@ function onMessage(msg: p.Message) {
13061307
}
13071308
}
13081309
}
1310+
1311+
// Gate behind a debug setting potentially?
1312+
onErrorReported((msg) => {
1313+
let params: p.ShowMessageParams = {
1314+
type: p.MessageType.Warning,
1315+
message: `ReScript tooling: Internal error. Something broke. Here's the error message that you can report if you want:
1316+
1317+
${msg}
1318+
1319+
(this message will only be reported once every 15 minutes)`,
1320+
};
1321+
let message: p.NotificationMessage = {
1322+
jsonrpc: c.jsonrpcVersion,
1323+
method: "window/showMessage",
1324+
params: params,
1325+
};
1326+
send(message);
1327+
});

server/src/utils.ts

+10-3
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ import * as os from "os";
1212
import * as codeActions from "./codeActions";
1313
import * as c from "./constants";
1414
import * as lookup from "./lookup";
15+
import { reportError } from "./errorReporter";
1516

1617
let tempFilePrefix = "rescript_format_file_" + process.pid + "_";
1718
let tempFileId = 0;
@@ -186,9 +187,15 @@ export let runAnalysisAfterSanityCheck = (
186187
RESCRIPT_VERSION: rescriptVersion,
187188
},
188189
};
189-
let stdout = childProcess.execFileSync(binaryPath, args, options);
190-
191-
return JSON.parse(stdout.toString());
190+
try {
191+
let stdout = childProcess.execFileSync(binaryPath, args, options);
192+
return JSON.parse(stdout.toString());
193+
} catch (e) {
194+
console.error(e);
195+
// Element 0 is the action we're performing
196+
reportError(String(args[0]), String(e));
197+
return null;
198+
}
192199
};
193200

194201
export let runAnalysisCommand = (

0 commit comments

Comments
 (0)