Create an error state to catch untracked errors from Svelte to React #55
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Goal: The goal for this PR is to create a state in the React implementation to catch and handle untracked errors from Svelte and integrate them into react error boundary. This should be replaced by a global error boundary in Svelte when it will be available.
Context: while investigating on a chart update uncaught error (time scales not accepted by the chartjs chart update method), it appeared that the asynchronous code from the update wasn't caught by Svelte neither by React resulting in a uncontrolled crash. Svelte doesn't offer an error boundary solution for now : github discussion and PR and react error boundaries are not handling asynchronous code as well : react documentation. It appeared that handling specific risky part of the code by wrapping them into a try and catch and setting an error state might be a solution.
Open discussion: this implementation is not ideal as we'll have to add this mechanism specifically for each risky part of the code and gives only a partial safety. A global Svelte error boundary would be better but not yet available. The investigated bug that led to this solution was caused by wrong chartjs parameters sent, we should probably more focus on not allowing this to happen, in this specific case find a solution: chartjs zoom plugin or falling back to a close available time scale. The question is should we keep this error handler as a protection anyway to avoid violent crashes or ignore it and resolve quickly all incoming bugs ?
If the implementation is accepted, what are the other code part we want to wrap with a try and catch and error state ? The markdown rendering ?