Distributed Error Reporting: Frontend Error reporting #12291
Conversation
Build Artifacts
|
|
|
||
| window.addEventListener('error', e => { | ||
| logging.error(`Unexpected Error: ${e.error}`); | ||
| ErrorReportResource.report(e.error); |
There was a problem hiding this comment.
I think for purposes of context it would be good to get the entire error object?
There was a problem hiding this comment.
It would be a good thing. But the report() will have too many conditions to make the code look not so clean. Eg:
- The Vue handler object will be handled with e.message and e.stack
- The error event will be handled with e.error.message and e.error.stack
- The promise rejection will be handled as e.reason.message and e.reason.stack
There was a problem hiding this comment.
In the later stage, we can do that. We will be optimizing the model anyway.
There was a problem hiding this comment.
Thats my whole point, Its better to pass it as an object since we will be moving onto the optimizations next. We wouldn't need to change the contract in this case. We would just read values from it into the model. I think implementing the logic as utility function should suffice.
The better approach would be to adopt the adapter pattern that should standardize the data from all three or n sources. I recommend reading about the adapter pattern. Also here is an example in JS
{vueError}
There was a problem hiding this comment.
Alright I will do the changes
| window.addEventListener('unhandledrejection', event => { | ||
| event.preventDefault(); | ||
| logging.error(`Unhandled Rejection: ${event.reason}`); | ||
| ErrorReportResource.report(event.reason); |
There was a problem hiding this comment.
akolson
merged commit cb259fa
into
learningequality:distributed-error-reporting
Summary
errorandunhandledrejectionto detect the unhandler error…
References
Fixes #12284
…
Reviewer guidance
…
Testing checklist
PR process
Reviewer checklist
yarnandpip)