feat: add result-returning send variants to Env and OwnedEnv

[bcksl]

Env::send() and OwnedEnv::send_and_clear() should ideally not elide the return value of enif_send.

Included are several variants, all of which provide exactly the same information, given the current NIF API.

My take is that the preferred solution would be to pick one and resignature send(), but we could also keep some/all of the variants provided instead.

In all cases, resignature would not a breaking API change, but will generate warnings for the _result variants. The _i32 and _bool variants are less idiomatic, but do not suffer from this issue. Nonetheless, my thinking is that an unhandled Result warning is ideal in this case, as it makes users aware of the fact that they are currently discarding information without breaking the build.

The naming is intentionally clunky, and if we decide to keep one or more of the send_*() variants rather than resignature send(), I'm happy to redraft with nicer names. Additional/modified tests will also follow that decision.

[filmor]

I think only the bool and Result<(), ()> variants make sense. Alternatively, you could add a (for now empty) error type pub struct SendError;.

I'm not quite sure, where you would see an improvement with this, though. The two documented error states (sender process is not alive or receiver process is not alive) are only checked at the time of sending, it could still be that the receiver process dies before processing the message. I'm also not sure whether the first case (sender process died) is even possible with our interface, at least not for owned envs. It is definitely wrong to just blindly unwrap the result here. Additionally, Erlang's ! operator also doesn't report any of this information back.

[bcksl]

For sure, the majority of cases that are using Env::send currently either (a) don't care if the send fails or (b) care that it reaches the destination and is processed. In the latter case, although the return of enif_send could be safely ignored—assuming users already implement a mechanism with stronger guarantees—the destination liveness check from enif_send provides fast-path feedback that the message will not be delivered.

Regarding OwnedEnv, yes, in the case that there is no calling process, i.e. we're calling from a thread outside the supervision of the BEAM, enif_send only checks liveness of the receiver. Both Env::send and OwnedEnv::send_and_clear already check the thread type as well, so we should be safe here.

I'm happy with a Result<(), SendError> if that works for you folks. Would you prefer to keep this as e.g. Env::send_result, or are we good to resignature Env::send?

[filmor]

Thinking a bit more about this, I'd propose to change the signature to send(...) -> bool. As said, I don't think we should encourage people to aggressively check the result if they don't know what they are doing, certainly not with an easy way to cause a panic from it. Exposing it makes sense, though, and conceptually it is a boolean, after all.

@evnu @hansihe @scrogson Opinions?

[evnu]

I think whether to use a bool or Result comes down to how a user wants to handle failure in this case, and if the extra information with Result would be useful. With Result, we can distinguish if the receiver failed (for the case of OwnedEnv), or if either the sender or the receiver failed. Now, if this is actually useful to handle the error is another question. I'd expect that usually a failure with this function would only be logged and maybe retried. If that is so, a bool is probably just fine.

As said, I don't think we should encourage people to aggressively check the result if they don't know what they are doing, certainly not with an easy way to cause a panic from it.

But maybe we should encourage users to read the documentation for that function, as the boolean fuses two distinct error cases. That is useful information when debugging why an expected message might not turn up at a receiver. When using a Result which needs to be handled, somebody who reads the code using send() will also have an easier time seeing that this might error out, even if that reader does not know the documentation (yet).

In any case, I don't have a strong opinion on this one. I can see benefits in both. As -> bool simply forwards what the NIF API exposes, we should probably go with that and avoid documentation friction between the rustler documentation and what the NIF API defines.

[bcksl]

Certainly a Result<(), SendError> of some variety—empty or otherwise—would draw users' attention to the documentation of each function and the error class itself, which could be expanded to include the relevant information.

I have a slight preference for this route over bool, but I'll leave it up to you folks to decide whether that would make too many waves downstream.

Who knows, maybe somewhere down the line OTP would accept an enif_send_x that reports the two failure modes separately.

[filmor]

I have a strong preference for fn send(...) -> bool. So if no one else has a strong opinion, I'd say we go with that :)

[hansihe]

An advantage of using a Result might be that the user is actually warned if they ignore the return value.

It forces the user to deal with the possibility of error, even if they just end up doing let _ = [send]

[filmor]

I have incorporated the results of this discussion (going with returning Result<(), SendError> now) and added tests for the simpler case of the receiver being dead. I'm not quite sure how to test the other case (sender dead), but it seems to be quite esoteric anyhow.

[evnu]

LGTM.

[bcksl]

Looks great, cheers!