Allow handlers to timeout

[thomaseizinger]

TODO:

• Change Display implementation on Error so that we provide more context on the actor and handler that failed.

[bonomat]

Minor things, otherwise looks good.

With

The consequence of this is that a timed-out handler returns Disconnected.

you mean we should introduce a different return type?

[thomaseizinger]

I think we might want to re-evaluate this and move it into the envelope same as we did here: #5

That might also allow us to more easily return a different type when a handler times out!

[thomaseizinger]

With

The consequence of this is that a timed-out handler returns Disconnected.

you mean we should introduce a different return type?

Yes! IMO when you call .send, you should know whether the actor is down (Disconnected) or the handler timed out (different return value then).

[luckysori]

I think we might want to re-evaluate this and move it into the envelope same as we did here: #5

That might also allow us to more easily return a different type when a handler times out!

Okay, let's try that! @bonomat. I'm happy to give this a go if we want to give this change priority.

[bonomat]

Would it be worth it to write out the seconds futures state instead of _ here?
The way how it is written atm is that the first case will match if you have a result and timeout. Is that what you want?

[luckysori]

It is intentional that the first match arm corresponds to the result future being ready and whatever the result is of the timeout future. I want to always succeed if we have the result of the first channel, even if the second future failed for whatever reason.

For instance, if the second future reports that the timeout has passed but the result is actually ready, we obviously should ignore the timeout and return the result. The timeout is only there as a mechanism to stop handlers that have been running for too long without success.

In practice it may be impossible to ever have a situation where both futures are Poll::Ready(Ok(_)) since we're using future::select in envelope.rs. What certainly does happen (consistently) is that the timed_out_channel is dropped as soon as the result_channel returns a value. The Drop implementation of the catty channel construct actually causes the channel to produce an Err(Disconnected). So when we match against the catty::Receivers and we have a successful result for the result_channel, we will have an error for the second one. Which is why we must ignore it for this to work anyway.

[bonomat]

Looks good. Nice work

[luckysori]

As a last change, I've added the message name to the Error::TimedOut variant, as it was suggested in get10101/itchysats#1954. I tried to also include the actor name, but I was not convinced because I had to contend with MessageChannels not knowing the actor they are sending the message to.