Stream interprets *any* exception as a reason to close itself

[thomasjm]

See

websockets/src/Network/WebSockets/Stream.hs

Line 93 in dbefa5f

mbBs <- onException receive (closeRef ref)

This is pretty confusing behavior, especially when you think about calling receiveDataMessage in an Async and calling cancel on that Async (which generates the AsyncCancelled exception).

I had written code like the following and found it fails due to this:

-- Start a proxy connection and tear it down when signalled
WS.runClientWithSocket sock hostname path opts headers $ \conn -> do

  -- Read from the websocket and write to the chan in an async
  containerWriteLoop <- async (forever $ WS.receiveDataMessage conn >>= writeChan someChan)
  -- Wait for a signal to exit
  shouldExit <- async $ readMVar shouldExitVar
   
  waitAnyCatchCancel [containerWriteLoop, shouldExit] >>= \case
    (x, result) | x == shouldExit -> do
      putStrLn "Got exit signal; tearing down connection"

      -- Send close and read messages until CloseRequest exception
      WS.sendClose rawConn ("Bye" :: B.ByteString)
      catch (forever $ void $ WS.receiveDataMessage conn) $ \case
        WS.CloseRequest _ _ -> return ()
        x -> putStrLn [i|Expected a CloseRequest when finalizing connection, but got '#{x}'|]
    
    (_, result) -> do
      putStrLn "Connection threw an exception; exiting..."

This client code is similar to some of the other websockets examples I've seen, except it waits for an external signal (via shouldExitVar) that it should tear down the connection. And rather than start a read loop via forkIO and let it die naturally via a close exception, I decided to use an Async and cancel it via waitAnyCatchCancel. But what happens is the websockets library interprets the AsyncCancelled as a reason to close the stream, and then the subsequent sendClose call fails with a ConnectionClosed exception. The net result for me is that the connection stays open forever.

I don't think it's reasonable for the stream to close itself on non-websocket related exceptions like this, since it makes it impossible to tear down the connection properly. I would suggest a couple fixes:

• Instead of using a blanket onException handler in Network.WebSockets.Stream, this library should only catch the exceptions it generates itself (ConnectionException, HandshakeException).
• Using exceptions to signal things in general makes this library really hard to program against correctly; it would be so much nicer if the API changed so that receiveDataMessage returned something like Either ConnectionException DataMessage. I saw in another issue that you're opposed to breaking changes but one can dream :)

[jaspervdj]

Fixed by #191