jaspervdj / websockets-snap

Snap integration for the websockets library
Other
33 stars 21 forks source link

ServerAppDone exception escapes to top level #9

Open ozataman opened 10 years ago

ozataman commented 10 years ago

I am not exactly sure why this is happening, as copyIterateeToMVar below knows how to catch the exception. Perhaps it finishes its job too early, perhaps the exception is terminated before it can.

        _ <- lift $ forkIO $ app pc >> throwTo thisThread ServerAppDone
        copyIterateeToMVar tickle mvar

Our use case involves a 4-5 second WebSockets interaction that terminates gracefully. However, the handler on the server side always exits with:

[16/Apr/2014:19:17:20 -0400] ["127.0.0.1"]: an exception escaped to toplevel:
ServerAppDone

Preventing further machinery from executing to do some necessary clean-up (in our case).

ozataman commented 10 years ago

@gregorycollins I wonder if this is yet another example for our classic enumerator-exceptions leakage problem.

gregorycollins commented 10 years ago

a) looks like "app pc" can finish before the exception handler is installed in "copyIterateeToMVar"

b) more importantly, "copyIterateeToMVar" uses "E.catchError", which only handles synchronous iteratee exceptions, not asynchronous exceptions (see the implementation here and note that nothing from Control.Exception is called within)

jaspervdj commented 10 years ago

@ozataman In the enumerator-based iteratee, there isn't much we can do about the exception escaping. The solution is usually that the meat of the websockets-using code should be wrapped in a finally or bracket in order to do cleanup.

ozataman commented 10 years ago

@gregorycollins (a) is what I was guessing, but I didn't realize (b), which completely defeats the implementation!

@jaspervdj I would propose then that we expose the ServerApp exception type from this module (for now, until the new streams based snap is live) so that it can be manually caught by the calling thread - it is currently an un-exported internal exception type. In other words, an internal exception is leaking to API user land as part of normal operation to signal successful completion of the communication, which is pretty bad. I'll catch it for now using SomeException, but that doesn't seem very clean.

ozataman commented 10 years ago

Wow, dealing with these exceptions is worse than I thought.

Here's a bit of code that runs arbitrary jobs and communicates their results back with an MVar to the handler initiating the request. The the WebSockets part is there to also inform the GUI client of the ongoing progress on the job, details of which are not relevant here.

I've sprinkled in numerous debugging statements and the error handling code, but none of that ever gets called. It never reaches the waiting for job debug print, but instead shows the ServerAppDone exception in error logs., i.e. the runWebSocketsSnap function never returns and the exception catching does not work.

Isn't this quite bizarre?

-------------------------------------------------------------------------------
-- | Accept incoming job requests in Snap context and communicate back
-- their results.
initJobH
    :: (ToJSON a, FromJSON i, Show i, ToJSON o, MonadSnap m)
    => RNG -> App i o a -> m (Job a)
initJobH rng f = do
    mv <- liftIO newEmptyMVar
    let grab = liftIO (putStrLn "grabbing" >> takeMVar mv)
    job <- (runWebSocketsSnap (initJob rng f mv) >>
            liftIO (putStrLn "waiting for job") >>
            grab)
      `MIO.catch` \ (e::SomeException) ->
        case show e of
          "ServerAppDone" -> grab
          e'              -> error ("Async Job Error: " ++ e')
    liftIO $ putStrLn "Job completed."
    return job