search

pkamenarsky / replica

A remote virtual DOM library for Haskell
BSD 3-Clause "New" or "Revised" License
139 stars 14 forks source link

Proper websocket connection closing and showing alert when internal error occurs #7

Closed kamoii closed 5 years ago

kamoii commented 5 years ago

For #5.

To notify user why the websocket connection is closed, this PR uses webosocket close code. The following pages show which code we can use and the code's meaning: https://tools.ietf.org/html/rfc6455#section-7.4 https://developer.mozilla.org/en-US/docs/Web/API/CloseEvent

We are using 1011 for internal error and 1000 for graceful closure. The websocket framework on the backend doesn't seem to send any code neither on exeption nor normal exit. So we need to explicity send close code. When the client-side recieves code 1011 on onclose callback, preseting an alert to reload the page.

pkamenarsky commented 5 years ago

Check out https://github.com/pkamenarsky/replica/commit/19ed174d74333df56de43bf32771fbfa96ea0b10. We need to throw in a couple of evaluates in there, otherwise exceptions in pure code are thrown inside sendTextData and the appropriate code/reason is not sent to the client.

Note, exceptions in lifted IO still won't get caught, since they will be thrown inside the widget's individual thread:

exWidget :: Widget HTML a
exWidget = do
  e <- div [ onClick ] [ text "BOOM" ]
  a <- liftIO $ evaluate ((5 `P.div` 0))
  text $ T.pack $ show "Done"
  exWidget

This exception won't be caught in websocketApp yet: I'll need to patch concur-core for that to work first.

kamoii commented 5 years ago

@pkamenarsky Slip my mind that pure value can raise exception too. Thank you for pointing it out! But I don't understand thrown inside sendTextData part. Is sendTextData using other thread to send data? I tried to follow sendTextData's implementation, but couldn't figure it out.

pkamenarsky commented 5 years ago

Is sendTextData using other thread to send data? I tried to follow sendTextData's implementation, but couldn't figure it out.

I'm not sure either, but without evaluate exceptions aren't caught in the try block; rather a ConnectionClosed exception is thrown when sendTextData tries to evaluate its payload and the websocket thread dies with a 1006 close message. Strange indeed.

I'd say this works well enough for now. The last piece of the puzzle for proper error handling is patching concur-core, which I hope I'll get around to soon.

Thanks a lot for the PR! 🎉