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

.wait_startup should handle the case of BoxError #74

Open
hanekoo opened this issue Oct 24, 2024 · 4 comments
Open

.wait_startup should handle the case of BoxError #74

hanekoo opened this issue Oct 24, 2024 · 4 comments
Assignees
Labels
enhancement New feature or request

Comments

@hanekoo
Copy link

hanekoo commented Oct 24, 2024

Motivation

I would like to propose an enhancement to the .wait_startup method to ensure that, even if it returns a BoxError, it provides better visibility into the actor's internal state and the reason for any failures. This will prevent blocking on wait_startup.await without clear feedback about what went wrong.

Additional Context

An example of the current implementation is as follows:

async fn wait_startup() -> Result<(), BoxError> {
    // Implementation
}

Or is there a better solution?

@hanekoo hanekoo added the enhancement New feature or request label Oct 24, 2024
@tqwewe
Copy link
Owner

tqwewe commented Oct 24, 2024

This is a great recommendation! So you're only looking to handle any errors that occurred during startup, and no return value for when startup is successful and returns Ok?

@hanekoo
Copy link
Author

hanekoo commented Oct 25, 2024

Of course, it would be ideal if a specific Result could be returned.

  • Ok: start_up might be performing some blocking initialization tasks and returning reference objects, such as File, TcpStream, or Sender/Receiver.
  • Err: When calling wait_startup, we often care about the specific result and need to understand the exact error that occurred, not just BoxError (on a related note, the same applies to ActorStopReason).

So, if a custom Result could be returned, that would be ideal. While this might be difficult to implement or could go against the actor model principles, sometimes breaking the paradigm to offer developers more flexibility is necessary. What do you think?

@tqwewe
Copy link
Owner

tqwewe commented Oct 25, 2024

This could work, however I'm trying to think how this could be done internally. I guess this means each actor ref would need shared access to a watch channel, which is set when the actors on_start method is called.

Some implications with this however are:

  • The Actor trait would need two associated types added: type StartupValue and type Error, both of which would be required to implement Clone, since multiple places might call .wait_startup().
  • Might be a slight performance hit for the majority of actors who never will use any return value from the on_start method. And cloning an ActorRef might also be slightly slower, but not much I'd imagine.

I do indeed think that having a type Error associated type with Actors would be more ideal than the generic BoxError. However I'm a little cautious to start adding too many features such as return values from on_start, as this can be solved by simply messaging an actor from within the on_start method.

Eg:

async fn on_start(&mut self, _actor_ref: ActorRef<Self>) -> Result<(), BoxError> {
    let file = tokio::fs::File::open("...").await?;
    self.other_actor_ref.tell(FileOpened(file)).await?;
    Ok(())
}

@hanekoo
Copy link
Author

hanekoo commented Oct 25, 2024

  • Might be a slight performance hit for the majority of actors who never will use any return value from the on_start method. And cloning an ActorRef might also be slightly slower, but not much I'd imagine.

It would also work to provide two methods, similar to tell and ask: one that does not return a value and one that does.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants