Optionally install a killer in runServer

[jaspervdj]

1. We add a new runServerWithOptions to deprecate runServerWith.
   This new function takes a ServerOptions type that we can extend in
   a more future-compatible way.

2. We add a serverRequirePong parameter to ServerOptions. This is
   set to N, we kill the application whenever a time period of N
   seconds passes without receiving a pong.

   In order for this to work, we obviously need to ensure that the
   client will send us pongs, so this option needs to be used in
   combination with withPingThread.

[nbouscal]

Hey @jaspervdj, I tested this out locally a bit and it seems to work correctly. Specifically, I verified these properties, using websockets for both client and server:

• A server with default options seems to work the same as always
• A server with a pong timeout and a ping thread doesn't close connections
• A server with a pong timeout but no ping thread does close connections
• A server with a pong timeout and a ping thread does close the connection if I pause the client process (with ctrl-z) and then resume it after longer than the timeout

The main high-level change I'd consider is having runServerWithOptions handle sending the pings as well if you have this setting turned on, as you really never want to have this setting on without also sending pings. That would probably imply allowing configuration of both the ping interval and the pong timeout (whether behind one Maybe or two I'm not sure). I can't think of a reason a user would need more control over sending pings than just the interval; if that is a requirement for some users then it would make sense to keep them separate.

Enjoy the rest of your vacation! 🙂

[jaspervdj]

Hi @nbouscal, thanks for testing all of that! I owe you a beer if you ever make it to ZuriHac.

I initially wanted to do it as you suggested; I agree that it would be nicer for users. However, I got stuck trying to implement it that way: the problem is that starting the ping thread requires a Connection, which isn't available in runServerWith. We only get that after the user uses acceptRequest. In a way, this makes sense -- we don't want to start pinging until we accept the request.

The type of acceptRequest is a bit too restricted to start the ping thread there -- we would need to move it to something like withAcceptThread so we can reliably kill the ping thread there. But I'm not sure if that's a change I want to force users to make... although it may be useful to install other kinds of cleanups as well.

[nbouscal]

Ah, I see, that makes sense. Given that, I think the current version requiring the user to handle sending pings for now is reasonable.

I am hoping to make it back to ZuriHac next year, but I think you've got the balance of debt backwards! My life would've been much harder without all the work you've done on this library 😉