Add proc-macro for convenient creation of `Handler`s

[thomaseizinger]

Currently, defining a Handler requires the usage of async_trait and filling in various elements into a trait impl.

In theory however, a macro can learn all required pieces of information from the following code block:

impl Actor {
    pub async fn handle_message(&mut self, message: Message) -> i32 {
        todo!()
    }
}

This needs to be expanded to:

#[async_trait::async_trait]
impl Handler<Message> for Actor {
    type Return = i32;

    fn handle(&mut self, message: Message, _ctx: &mut Context<Self>) -> i32 {
        todo!()
    }
}

This is what https://github.com/comit-network/xtra-productivity does although it is not yet updated to the latest version of xtra where f.e. the Message type is already removed. @klochowicz and myself authored that crate at my last gig. I think it would be nice to pull this into this repository as a separate crate, re-export it through xtra and expose it as xtra::handler so it can be used like:

#[xtra::handler]
impl Actor {
    // function definitions here
}

The main features of the macro are:

• Creates 1 impl Handler per fn in the impl block
• Removes duplicate mention of message and return type
• Allows omitting Context if not used

https://github.com/comit-network/xtra-productivity would need a lot of polish before it can be officially released, esp. in regards to error messages. The implementation isn't too complex so we can also start from scratch if necessary.

[Restioson]

This is what https://github.com/comit-network/xtra-productivity does although it is not yet updated to the latest version of xtra

I have this updated on a local copy :) Will push my forks soon once I get a bit further along.

This sounds perfect for a crate to be re-exported by an extension crate! Might have some overlap with spaad

[thomaseizinger]

This sounds perfect for a crate to be re-exported by an extension crate! Might have some overlap with spaad

An alternative could be to:

• Move current code from xtra to xtra_core
• Make xtra re-export xtra_core
• Have xtra re-export all kinds of stuff under feature-flags

This kind of meta-crate approach is what rand for example does. We also do this in libp2p. Not sure if it is worth the complexity already.

The nice thing over just feature-flags is that it speeds up compile-times and allows for more fine-grained version updates.

[thomaseizinger]

Do we want this for 0.6?

[axos88]

With the preliminary support for async traits in latest stable, is this a necessary addition?

[axos88]

The explicit handler impls are much more... explicit, and easily searchable. I do feel this would be a bit black magic.

[thomaseizinger]

With the preliminary support for async traits in latest stable, is this a necessary addition?

*nightly. I think they are in latest nightly.

I haven't experimented with it yet, we might hit some of the current limitations in regards to Send inference.

[thomaseizinger]

The explicit handler impls are much more... explicit, and easily searchable. I do feel this would be a bit black magic.

You never invoke handlers directly anyway so really, you end up searching by message type and you land at the same implementation :)

[sinui0]

I really like this/spaad and would like to start using it. Hiding the message passing as an implementation detail seems like it would boost productivity quite a bit.

Is help needed on implementing this for 0.7?

An alternative could be to:

Move current code from xtra to xtra_core Make xtra re-export xtra_core Have xtra re-export all kinds of stuff under feature-flags

This is sounds good.

From #200 :

Spaad was always a little bit of an experiment with it and I was not totally satisfied with how it turned out.

@Restioson would you elaborate on what fell short with spaad? I really like the look of it.

[thomaseizinger]

Spaad was always a little bit of an experiment with it and I was not totally satisfied with how it turned out.

@Restioson would you elaborate on what fell short with spaad? I really like the look of it.

Personally, I am not a fan of proc-macros that generate APIs the user has to interact with because it means you can't jump to them and see what they do.

Generating Handler implementations from regular impl blocks is different because you still interact with the Address struct to send messages. IMO spaad hides too many of these details for practically no benefit (in terms of Loc for example).

My 2c: If you are using an actor framework, interacting with an Address is essential and shouldn't be hidden.

[sinui0]

If you are using an actor framework, interacting with an Address is essential and shouldn't be hidden.

Not sure I agree, at least not in general.

In my view a major pain with taking advantage of the actor model is the boilerplate that comes with the message passing. Ideally it would be hidden as an implementation detail and would feel a lot more like lock-based interfaces, like what spaad provides.

There is some shared resource, you want multiple references to it while being able to await on mutating/querying it.

Having to manually write out message structs (which are just handler arguments), destructure messages in their handlers, and interact with an Address via Address::send(Message { arg0: X, arg1: Y}).await.map_err(MyError::from)? gets unwieldy. Instead presenting it as a function call on a Clone interface which can be used via shared reference seems very nice.

practically no benefit (in terms of Loc for example)

I've not played with spaad, but I would have figured it has a pretty large impact on LOC?

Fully agree that interacting with macro generated code leaves much to be desired! Though, the indirection via a channel breaks code nav anyhow, assuming the user is trying to find the implementation of a message handler. If the method on the controller is named the same as the handler on the actor it may even be easier to find, eg:

struct HelloActor;

#[xtra::handler]
impl HelloActor {
    async fn hello(&mut self, name: String) {
        println!("Hello {name}!");
    }
}

// Macro generated
struct Hello {
    name: String,
}

struct HelloActorController {
  addr: Address<HelloActor>,
}

impl HelloActorController {
    async fn hello(&self, name: String) {
       // Takes care of message passing
    }
}

edit just realized the above example is no different than what spaad does, sorry for noise.

Of course, this all being optional is key. I do appreciate the minimalism of xtra, and like the idea of xtra_core with extension crates.

[thomaseizinger]

In my view a major pain with taking advantage of the actor model is the boilerplate that comes with the message passing. Ideally it would be hidden as an implementation detail and would feel a lot more like lock-based interfaces, like what spaad provides.

By design, a system based on actors introduces an indirection at certain points. An actor may not be active / alive and thus message delivery will fail. It is important to capture and expose this error case because the caller needs to handle it. You cannot generically handle it for the caller which means you cannot hide it behind a macro and pretend it doesn't exist.

xtra's message delivery also has two yield points:

• Message is queued into the actors mailbox
• Message handler has completed

Depending on your usecase, you may want to wait for both or send message "asynchronously" by continuing with your control flow after the first one and await the result of the handler later. Again, a macro cannot generically handle this for you. Not without exposing several config options at which point I am wondering why you don't just use the existing API of xtra :)

My guess would be that if the boilerplate of xtra is too much and/or the above points have not come up in your application design, then you may be breaking up your system into too many actors. I see actors like (micro)-services: Many systems are perfectly fine as monoliths or a handful of services and I'd be conservative in introducing additional ones.

[sinui0]

I generally agree with your stance which seems to be one preferring explicitness. For me it depends what I'm feeling for the boilerplate vs obscurity trade-off for the task at hand.

I don't think I agree with the diagnosis that the boilerplate is mostly an issue if one is over-encapsulating. If you humor for a moment more of a bias towards succinctness I have some thoughts on the rest of your points:

It is important to capture and expose this error case because the caller needs to handle it. You cannot generically handle it for the caller which means you cannot hide it behind a macro and pretend it doesn't exist.

I agree here, it's not something that can be ignored. But it doesn't need to be ignored, and infact the existing Rust function signature syntax can express it quite well. If the return type is Result<T, E> then you bound E: From<xtra::Error>, and if the return type is infallible you panic.

// Generated
impl FooController {
  // Handler::Return = ()
  async fn panic(&self) {
    self.address.send(Panic)
      .await
      .expect("message can be queued")
      .await
      .expect("actor handles without interruption")
  }

  async fn panic_detach(&self) {
    self.address
      .send(PanicDetach)
      .detach()
      .await
      .expect("message can be queued");
  }

  // Handler::Return = u8
  async fn fallible(&self) -> Result<u8, MyError> {
    self.address.send(Fallible)
      .await
      .map_err(MyError::from)?
      .await
      .map_err(MyError::from)
  }

  async fn fallible_detach(&self) -> Result<(), MyError> {
    self.address.send(FallibleDetach)
      .detach()
      .await
      .map_err(MyError::from)?;
    Ok(())
  }

  async fn fallible_detach_return(&self) -> Result<impl Future<Output = u8>, MyError> {
    self.address.send(FallibleDetachReturn)
      .detach()
      .await
      .map_err(MyError::from)
  }
}

Differentiating between the two yield points is pretty simple with an attribute:

#[xtra::handler]
impl Foo {
  #[detach]
  async fn fallible_detach(&mut self) -> Result<(), MyError> { .. }

  #[detach_return]
  async fn fallible_detach_return(&mut self) -> Result<u8, MyError> { .. }
}

As the number of methods an Actor has increases, a macro really does cut down a huge amount of boilerplate. Even just writing out the code block in this reply was exhausting :joy: and it doesn't even include message struct definitions or the handlers. The generated code isn't something I would personally be worried about having (optionally!) hidden.

[thomaseizinger]

As the number of methods an Actor has increases, a macro really does cut down a huge amount of boilerplate. Even just writing out the code block in this reply was exhausting 😂 and it doesn't even include message struct definitions or the handlers. The generated code isn't something I would personally be worried about having (optionally!) hidden.

I 100% agree for the definition of handlers (and perhaps message structs). In fact, that is what this issue is about :)

The problem I see is in the interaction with the actor. For example, whether or not I want to detach from awaiting the message isn't something that should be decided on the definition site but will change on a case-by-case basis when sending the message! If you are going to hide the SendFuture type then you need to generate one function for each possibility. Add broadcasting and priority into the mix and you start to generate a lot of code :)

I don't really see the boilerplate in writing:

address.send(Hello { foo, bar, baz }).await?

vs

actor.hello(foo, bar, baz).await?

[sinui0]

I 100% agree for the definition of handlers (and perhaps message structs)

:handshake:

In fact, that is what this issue is about :)

My most recent thoughts are mostly towards the boilerplate of a "Controller" (sorry for drifting off-topic again) ie a user facing API which shouldn't expose details such as message types, addresses, priority, or any actor related stuff. If you're writing a general purpose library and using the actor pattern under the hood to manage a shared resource, you most likely don't want to force your users to know about these things.

As the library author you will know what kind of access/control flow patterns are appropriate, and yes you may need to generate a different function for different cases and document them.

[thomaseizinger]

In fact, that is what this issue is about :)

My most recent thoughts are mostly towards the boilerplate of a "Controller" (sorry for drifting off-topic again) ie a user facing API which shouldn't expose details such as message types, addresses, priority, or any actor related stuff. If you're writing a general purpose library and using the actor pattern under the hood to manage a shared resource, you most likely don't want to force your users to know about these things.

As the library author you will know what kind of access/control flow patterns are appropriate, and yes you may need to generate a different function for different cases and document them.

Thanks for expanding! I'd argue that whilst using macros may make sense for that, it is out of scope for us. It very much depends on your usecase what you'd need from such a macro and it is hard/impossible for us to make a good general purpose one. Most likely, writing your own macros will be faster and easuer because you can adapt it to your usecase :)