This repository has been archived by the owner on Dec 28, 2021. It is now read-only.
Catch syntax errors in custom visualisations #1341
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Syntax errors in custom visualisations caused panics during IDE startup. Now they are caught and handled like errors during execution of the visualisation code, which writes a log message to the console. This commit duplicates some code from wasm-bindgen to to allow for an easy fix. Hopefully the necessary feature will be included in wasm-bindgen soon.
s9ferech
added
Category: GUI
wdanilo
approved these changes
Mar 17, 2021
There was a problem hiding this comment.
I love this. We can merge it now. Would you be so nice and open also PR in the wasm-bindgen repo and link the PR in this function.rs doc comment that you've created, please?
Please ask someone from the team to test it as well before the merge, I cant handle it right now.
...ide/view/graph-editor/src/component/visualization/foreign/java_script/definition/function.rs
Outdated
Show resolved
Hide resolved
...ide/view/graph-editor/src/component/visualization/foreign/java_script/definition/function.rs
Outdated
Show resolved
Hide resolved
|
Also, before merging this, please test this on the issue that Maciek had: https://github.com/enso-org/ide/issues/1310 The issue was NOT about panic - it was about hanging gui on startup, while still you were able to drag nodes etc. Just test it fixes this thing. @BinarySoftware please test it too. |
|
I created the PR at wasm-bindgen. And yes, this solves the original issue. |
mwu-tow
pushed a commit
to enso-org/enso
that referenced
this pull request
Oct 30, 2021
Catch syntax errors in custom visualisations Syntax errors in custom visualisations caused panics during IDE startup. Now they are caught and handled like errors during execution of the visualisation code, which writes a log message to the console. This commit duplicates some code from wasm-bindgen to to allow for an easy fix. Hopefully the necessary feature will be included in wasm-bindgen soon. Original commit: enso-org/ide@2d04d0e
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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.
Pull Request Description
This is a potential solution to #1310.
Syntax errors in custom visualisations caused panics during IDE startup. Now they are caught and handled like errors during execution of the visualisation code, which means that we write a log message to the console.
Important Notes
The wasm-bindgen API that we used to parse visualisation code from JS, does not allow to handle syntax errors, it always panics on those. I filed an issue at their GitHub repo, suggesting that they allow error handling. As a temporary fix, this PR includes a copy of their relevant code with the necessary changes. We could merge this now or wait to see if they react to the issue and provide the possibility for a cleaner solution.
With this PR, syntax errors in visualisations will only be logged to the console, which means that they stay invisible to normal users. In the long term, they should be reported on the GUI.
Checklist
Please include the following checklist in your PR:
CHANGELOG.mdwas updated with the changes introduced in this PR.