Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve getElementById error reporting #129

Closed
blitzcode opened this Issue Apr 23, 2016 · 4 comments

Comments

Projects
None yet
2 participants
@blitzcode
Copy link
Contributor

blitzcode commented Apr 23, 2016

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

This comment has been minimized.

Copy link
Contributor Author

blitzcode commented Apr 24, 2016

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 added a commit that referenced this issue Dec 7, 2016

Implement exceptions for `callFunction`. #129
Implement exception handling the `UI` monad.

In particular, this fixes the previously broken
function `getElementById`. It now returns `Nothing`
when no element of that particular ID exists. #129

Update the `GetElementById.hs` example.

Version number is now 0.7.1.0 because a new type
and new type class instances have been added.

Update CHANGELOG
@HeinrichApfelmus

This comment has been minimized.

Copy link
Owner

HeinrichApfelmus commented Dec 7, 2016

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

This comment has been minimized.

Copy link
Contributor Author

blitzcode commented Dec 10, 2016

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

This comment has been minimized.

Copy link
Owner

HeinrichApfelmus commented Dec 10, 2016

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!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.