Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat!: relax request impls to be generic to any mailbox #71

Merged
merged 2 commits into from
Oct 20, 2024

Conversation

tqwewe
Copy link
Owner

@tqwewe tqwewe commented Oct 19, 2024

This PR relaxes the implementation of the mailbox for the following traits:

  • MessageSend :: send
  • TryMessageSend :: try_send
  • BlockingMessageSend :: blocking_send
  • TryBlockingMessageSend :: try_blocking_send

Previously, sending a message to a generic actor reference required explicitly specifying the mailbox type:

async fn send_message<A, M>(actor_ref: ActorRef<A>, msg: M)
where
    A: Actor + Message<M>,
    M: Send + 'static,
{
    let _ = actor_ref.tell(msg).send().await; // Error: `.send` isn't available here because the actor mailbox was not specified
}

To resolve this, the mailbox had to be declared as A: Actor<Mailbox = BoundedMailbox<A>>. However, this restricted flexibility when we wanted to support both bounded and unbounded mailboxes.

With this PR, this code will now work seamlessly, as the MessageSend::send method is now generic over any mailbox type. There's no longer a need to specify the mailbox. The only method that still requires a specified mailbox is send_sync, which is only applicable for unbounded tell requests.

The Message trait now requires T to implement Send + 'static. This is necessary because sending a message to an actor requires these bounds. Without this constraint, if a user writes A: Actor + Message<M> but forgets to specify M: Send + 'static, they'll encounter issues when messaging the actor, and the resulting error message won't clearly indicate what's missing.

Before this PR, specifying a generic actor capable of handling a message required a complex and verbose trait bound like this:

where
    for<'a> TellRequest<LocalTellRequest<'a, A, A::Mailbox>, A::Mailbox, M, T>:
        MessageSend<Ok = (), Error = SendError<M, <A::Reply as Reply>::Error>>,

You needed separate bounds for each message type, and for both tell and ask requests, which made the code unnecessarily verbose. This PR significantly improves the developer experience by simplifying these trait bounds, making Kameo much more ergonomic to use.


A nice resource for checking which requests types are supported when can be found here: https://docs.page/tqwewe/kameo/core-concepts/requests#request-methods

This PR fixes and relates to this discussion: #38

@tqwewe tqwewe merged commit 0aea82b into main Oct 20, 2024
2 checks passed
@tqwewe tqwewe deleted the feat/relax-request-impls branch October 20, 2024 08:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant