RFC: Include intended recipient name in SendError

[strohel]

Add test for SendError

We're going to touch it, the tests will demonstrate the changes.

Include intended recipient name in SendError

This aids with debugging send errors. Thanks to type decoupling made by Recipient, senders may not even know the actual receiving actor, so senders cannot replicate this behaviour fully on their own.

Should help with debugging downstream issues like tonarino/portal#1153.

Caveats:

• Increases the size of Recipient, Addr, SendError by ~one pointer~ two pointers (one &str).
• Introduces API-breaking changes to SendError and SendResultExt::on_full().

  • ~Specifically, SendError, DisconnectedError can no longer be constructed by downstream code. That feels correct now, but can be fixed in non-API-breaking manner if needed.~ - no longer true after the last commit.

Is it worth it?

[strohel]

Did you consider including the intended message instead, like crossbeam_channel does? Some pros and cons that I can think of:

Pros:

• We can inspect the content of the message that is being dropped in case of error
• Particularly useful for actors that take an enum message

Cons:

• Messages need to implement Debug + Send?
• There are cases multiple actors take the same message type e.g. MediaFrame.

That's a very valid remark. Agreed that seeing the message is even more useful than seeing the recipient.

The approach is made tricky by our ability to create Recipient<N> out of Addr<M> if N can be converted to M, introduced in #38.

1. SendError cannot simply contain M as Recipient<N> must have the M type erased.
2. If we also require that M is convertible back to N, then SendError can contain N. But such a requirement feels weird and may be impossible to satisfy.
3. SendError could contain Box<dyn Any> (Any being M), but Any doesn't unfortunately allow "try downcasting to Debug trait". It only allows trying downcasting to a particular type, M in this case. That would make the whole M -> N type erasure a second class citizen.
4. SendError could contain Box<dyn Debug>, which would indeed require adding the Debug bound to Actor::Message (it is already Send) - and possibly also Sync (see below).

The ideal API would probably be: provide Option<Box<dyn Debug>> in SendError which would be Some if M: Debug and None otherwise. Maybe specialization would allow this one day, but I see no way how to achieve that in current stable rust (it looks like one can "require" a bound, but cannot "detect optional bound being satisfied").

Anyways, option (4) looks viable, so I've submitted and alternate PR #60 but it suffers from its own problems.

It is possible that we've closed a lot of doors with #38 - it could be feasible to touch some of its core ideas. For example we may try to Box the messages and convert then later on the receiving side. That could allow option (1) above.

[skywhale]

Thank you for all the considerations. The PR is still worth merging as is, but I would also love to hear other Rust experts thoughts @bschwind @mcginty @efyang @slightknack