HeinrichApfelmus / threepenny-gui

GUI framework that uses the web browser as a display.
https://heinrichapfelmus.github.io/threepenny-gui/
Other
439 stars 77 forks source link

Improve getElementById error reporting #129

Closed blitzcode closed 7 years ago

blitzcode commented 8 years ago

getElementById is typed to return a Maybe, but actually never returns a Nothing in case of failure, but throws an exception. I'd suggest to either actually return a Maybe or remove it from the type to make it clear it throws an exception. Also, the exception is unhelpful for understanding why generating the page failed, making it extremely hard to track down the error.

For now, I wrote this wrapper:

getElementByIdSafe :: Window -> String -> UI Element
getElementByIdSafe window elementID =
    (liftIO $ try (runUI window $ getElementById window elementID)) >>= \case
        Left (e :: SomeException) -> do
            traceS TLError $ printf "getElementById for '%s' failed: %s" elementID (show e)
            liftIO . throwIO $ e
        Right Nothing -> do
            traceS TLError $ printf "getElementById for '%s' failed" elementID
            liftIO . throwIO $ userError "getElementById failure"
        Right (Just e) ->
            return e

This gives at least the failure reason and the name of the missing element.

blitzcode commented 8 years ago

Hm, I was sure I tried the wrapper above and it worked, but right now I can't even get that to work. Calling getElementById with an invalid ID will just completely freeze the handler, no error or exception whatsoever. Any workarounds much appreciated! It's very concerning that a single type in a string just silently stops the application with no tools for debugging available.

HeinrichApfelmus commented 7 years ago

Thanks for reporting! I had simply not implemented the required functionality yet. The recent commit mentioned above should fix the behavior of getElementById, so that it now returns Nothing as expected.

blitzcode commented 7 years ago

Seems to work as documented now! That's a huge relief, it was rather scary that a single wrong string could cause the entire application to hang client-side with no way to react or even log on the server.

HeinrichApfelmus commented 7 years ago

Yup, sorry about that. Threepenny was always a bit on the prototype side, but it's getting more robust now. On the first attempt, you simply ignore all the exceptional code paths. 😄

Looks like this can be closed, then. Please feel free to reopen if necessary!