Display FFI errors to user #268
Conversation
|
Not sure why pete-murphy@50a500a isn't showing up here 🤔 maybe GH is just slow to sync |
client/src/Try/Loader.purs
Outdated
| @@ -112,7 +112,7 @@ makeLoader rootPath = Loader (go Object.empty <<< parseDeps "<file>") | |||
| deps = { name: _, path: Nothing } <$> shim.deps | |||
| pure { name, path, deps, src } | |||
| Nothing -> | |||
| pure { name, path, deps: [], src: ffiDep name } | |||
| throwError ("FFI dependency not provided: " <> name) | |||
There was a problem hiding this comment.
I just copied the error message from the ffiDep that this replaces, but maybe there could be more info here about why?
There was a problem hiding this comment.
Maybe:
Compilation succeeded, but the following FFI dependency is missing:
- url
We don't provide FFI shims for all libraries in the PureScript package sets; you can't execute Node libraries on Try PureScript.
(Adapted from #265 (comment) 🙂 )
There was a problem hiding this comment.
Perhaps: "We don't provide FFI shims for all libraries; for example, Node libraries are not supported on Try PureScript. If you would like to suggest a new FFI shim be supported, please open an issue."
There was a problem hiding this comment.
Updated the copy here:
However I also realized that rendering this error message prevents the compiled JS code from displaying, since both want to set to the compiled state ... 😬
There was a problem hiding this comment.
I think it's fine to surface this error in the iframe instead of only in the console. However, if we find a better place to put it in the future, then I'm happy to move it around!
| @@ -383,7 +388,7 @@ component = H.mkComponent | |||
|
|
|||
| renderCompiled = case _ of | |||
| Left err -> | |||
| renderPlaintext "Unable to parse the response from the server." | |||
There was a problem hiding this comment.
Ah, just realized why we weren't hitting this case even if we pass a bad loaderUrl. This line
trypurescript/client/src/Try/Container.purs
Line 202 in a989a85
loader.
pete-murphy
force-pushed
the
pm/265/show-ffi-errors
branch
2 times, most recently
from
c72418a to
c0d4669
Compare
|
Thanks! |
Description of the change
Fixes #265
These are minimal changes to render an error message in the case of a missing FFI dependency. We are displaying the message in the right pane (using the same
renderPlainTextfunction that's used for displaying the JS output for example) as well as throwing an error so that it's visible in the browser console. If there are multiple missing dependencies, only the first will be rendered to the user.I looked into accumulating an array of misses using
Data.Validation.Semigroupin #267, but I don't know that the added complexity was justified, since it seems like there would rarely be more than a single missing FFI dep.Screenshots
Single missing FFI dependency
Before
After
Multiple missing FFI dependencies
Same error messages as single-missing, even thoughpuppeteeris another missing dependency here.Before
After
Server error
Simulated by settingTry.Config.loaderUrlto a non-existent URLBefore (not sure why this isn't showing "Unable to parse the response from the server.")
After
Successfully compiled code
I added word-wrap to thecodetag rendered byrenderPlainTextto avoid horizontal scrolling for error messages, which also affects how the compiled JS looks (maybe not for the better 🤷 )Before
After
Checklist: