Organize Recipient::send() and friends differently?

[strohel]

There are currently 4 related methods:

    /// ... Use this if you need to react when the channel is full.
    pub fn try_send<N: Into<M>>(&self, message: N) -> Result<(), TrySendError<M>>;

    /// ... Use this if there is nothing you can do when the channel is full.
    /// The method still logs a warning for you in that case.
    pub fn send<N: Into<M>>(&self, message: N) -> Result<(), TrySendError<M>>;

    /// ... Use this if you do not care if messages are being dropped.
    pub fn send_quiet<N: Into<M>>(&self, message: N);

    /// ... Use if you expect the channel to be
    /// frequently full (slow consumer), but would still like to be notified if a
    /// different error occurs (e.g. disconnection).
    pub fn send_if_not_full<N: Into<M>>(&self, message: N) -> Result<(), TrySendError<M>>;

From API user PoV, I see some possible shortcomings:

1. The most prominent send() method "swallows" (just logs) certain class of errors (channel full). That seems good for real-time processing. For a generic actor crate, I would expect the "default" send method to be the most defensive one, i.e. current try_send() the returns all the errors.
2. The 3 methods all return the same error type, though in some cases, some variants should be impossible (send(), send_if_not_full()).
3. There are 4 similar methods with somewhat overlapping functionality.

Brainstorm of an alternative API (usage standpoint):

// current                          equivalent in new API
recipient.try_send()          ->    recipient.send()
recipient.send()              ->    recipient.send().warn_when_full()
recipient.send_quiet()        ->    recipient.send().ok()    // generic way to ignore errors and suppress unused_result
recipient.send_if_not_full()  ->    recipient.send().ignore_full()
// The last case is currenly implemented a bit differently internally - through
// self.remaining_capacity(). That would need to be considered.

That should be possible by introducing custom error types + extension trait for Result<(), ThatErrorType>. Downsides are the added boilerplate, call site verbosity, need for users to import that extension trait.

[skywhale]

I agree that these variants are confusing. In our codebase, too, we don't seem to have a good guideline when to use send vs. try_send vs. send_quiet. I personally like the verboseness of warn_when_full() and ignore_full().

I'm sorry for the current documentation being confusing, but send_quiet() should map to send().ignore_full(). send_if_not_full() was added for a very specific use case, and I want to actually remove it. Please see https://github.com/tonarino/actor/issues/22

[strohel]

I'm sorry for the current documentation being confusing, but send_quiet() should map to send().ignore_full().

Hmm, now I realize that ignore_full method name could be potentially ambiguous, as that could mean both "ignore the case when channel is full" and "ignore everything". Perhaps ignore_when_full() would be better.

But let me present the suggested API more properly:

impl<M> Recipient<M> {
    /// Send a message to the actor at this address. Non-blocking. See methods from
    /// [SendResultExt] trait for convenient handling of possible errors.
    pub fn send<N: Into<M>>(&self, message: N) -> Result<(), FullOrDisconnectedError<M>> { ... }

    ...
}

// basically crossbeam::channel::TrySendError, but circumvents orphan rules.
enum FullOrDisconnectedError<M> {
    Full(M),
    Disconnected(M),
}

struct DisconnectedError<M>(pub M);

impl SendResultExt for Result<(), FullOrDisconnectedError> {
    /// Handle channel full error by printing a warning. Propagate disconnected error. 
    fn warn_when_full(self) -> Result<(), DisconnectedError<M>> { ... }

    /// Quietly ignore channel full error. Propagate disconnected error. 
    fn ignore_when_full(self) -> Result<(), DisconnectedError<M>> { ... }
}

Usage in a Result-returning function, from most defensive to most carefree:

// propagate both disconnected and channel full errors.
recipient.send(msg)?;
// propagate disconnected error, log & discard channel full error.
recipient.send(msg).warn_when_full()?;
// propagate disconnected error, ignore channel full error.
recipient.send(msg).ignore_when_full()?;
// silently ignore both disconnected and channel full errors. Two equivalent methods.
let _ = recipient.send(msg);
recipient.send(msg).ok();

Here the last line is plain Result::ok(), which some folks describe as slightly more defensive alternative to let _ = ....

Does it make sense? Maybe ignore_when_full() is not really needed.

[mcginty]