cancellation and log messages

[hannesm]

but there's no entry in the connection table (we won't cancel DNS resolution)

This is based on the observation that cancellation will only be reasonable for connecton attempts (since they already have identifiers and use resources).

//cc @reynir

[hannesm]

I pushed a second commit to support cancellation (Lwt.cancel) of connection attempts. This also passes the error messages forward (so a client sees the error in more detail).

Since the API changed, we'll need to bump the version. WDYT, @reynir (also, this should be tested a bit more) ;)

[reynir]

In the following snippet I thought, reading the documentation for Lwt.bind, that when cancelling th it would propagate forwards to the right hand side of the bind in th'. It turns out that Lwt.cancel works 'backwards' to Lwt_unix.connect and cancels that - and then that gets propagated forwards and caught in the Lwt.catch, and a normal Error _ value is resolved. So the original implementation was just fine - I was confused about how cancellation works.

let th =
  Lwt.catch
    (fun () -> Lwt_unix.connect fd addr >|= fun () -> Ok fd)
    (function
     | Lwt.Cancelled -> safely_close fd >|= fun () -> Error (`Msg "cancelled"))
in
let th' = th >>= fun r -> ... in
Lwt.cancel th

[reynir]

Hmm, we use the spelling 'cancelled' while lwt uses 'canceled'. I think we are consistent with ourselves, but not with lwt. I don't have strong opinions on this...

[hannesm]

@reynir I re-applied your change as the last commit here. from my perspective, this is good to go now. I find the Lwt.pick based code much clearer to understand than calling Lwt.cancel on some task. WDYT?

[reynir]

Yes, I'm more convinced I know how the Lwt.pick based code works.