-
Notifications
You must be signed in to change notification settings - Fork 226
Add listeners for unhandled errors to web views #2139
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
7176f69
d8c2562
47fa752
969fdb6
3fe0699
64531f5
f2f1b1d
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,54 @@ | ||
| import { getErrorMessage, getErrorStack } from "../../pure/helpers-pure"; | ||
| import { vscode } from "../vscode-api"; | ||
|
|
||
| // Keep track of previous errors that have happened. | ||
| // The listeners for uncaught errors and rejections can get triggered | ||
| // twice for each error. This is believed to be an effect caused | ||
| // by React's error boundaries. Adding an error boundary stops | ||
| // this duplicate reporting for errors that happen during component | ||
| // rendering, but unfortunately errors from event handlers and | ||
| // timeouts are still duplicated and there does not appear to be | ||
| // a way around this. | ||
| const previousErrors: Set<Error> = new Set(); | ||
|
|
||
| function shouldReportError(error: Error): boolean { | ||
| const seenBefore = previousErrors.has(error); | ||
| previousErrors.add(error); | ||
| setTimeout(() => { | ||
| previousErrors.delete(error); | ||
| }, 1000); | ||
| return !seenBefore; | ||
| } | ||
|
|
||
| const unhandledErrorListener = (event: ErrorEvent) => { | ||
| if (shouldReportError(event.error)) { | ||
| vscode.postMessage({ | ||
| t: "unhandledError", | ||
| error: { | ||
| message: getErrorMessage(event.error), | ||
| stack: getErrorStack(event.error), | ||
| }, | ||
| }); | ||
| } | ||
| }; | ||
|
|
||
| const unhandledRejectionListener = (event: PromiseRejectionEvent) => { | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should we be making a distinction between the unhandled errors and unhandled rejections? As far as I am aware, unhandled rejections don't cause any user-visible behaviour (except perhaps some data not loading), but unhandled errors might cause the complete view to disappear. It would be useful to make that distinction in the telemetry.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. For now I'm going to leave them combined. I don't think it's quite as well defined as saying unhandled errors are always more important. Missing or incorrect data is also bad, and maybe worse if the user don't know the data they're looking at is incorrect. Unhandled errors can also happen outside of rendering. Also, we can certainly split these up in the future, and doing it after this PR will be fine. The telemetry is quite schema-less and we only keep it for a short period, so changing the format is quite cheap to do. |
||
| if (shouldReportError(event.reason)) { | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm not 100% sure but I think the problem of duplicate errors only happens for thrown errors and not rejected promises. So this check here may be unnecessary, but it also shouldn't hurt so I decided to keep it in. I'm not really sure about this though and I'm happy to remove it. |
||
| vscode.postMessage({ | ||
| t: "unhandledError", | ||
| error: { | ||
| message: getErrorMessage(event.reason), | ||
| stack: getErrorStack(event.reason), | ||
| }, | ||
| }); | ||
| } | ||
| }; | ||
|
|
||
| /** | ||
| * Adds listeners for unhandled errors / rejected promises. | ||
| * When an error is detected a "unhandledError" message is posted to the view. | ||
| */ | ||
| export function registerUnhandledErrorListener() { | ||
| window.addEventListener("error", unhandledErrorListener); | ||
| window.addEventListener("unhandledrejection", unhandledRejectionListener); | ||
| } | ||
Uh oh!
There was an error while loading. Please reload this page.