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

[Feature Request] - awaitAll throwing CompositeException #442

Open
rela589n opened this issue Aug 11, 2024 · 0 comments
Open

[Feature Request] - awaitAll throwing CompositeException #442

rela589n opened this issue Aug 11, 2024 · 0 comments

Comments

@rela589n
Copy link

Hello,

First of all, I'd like to thank you for all the effort you spent implementing and supporting such an wonderful library.
I'm very excited due to the fact that third version has been released and now every project could benefit from AMP.

Now, using it on one of my last projects, I'd like to ask for a small feature that would slightly improve DX.

Consider this example:

private function createEvent(ResetUserPasswordCommand $command): UserPasswordResetEvent
{
    [$user, $passwordResetRequest] = Future\awaitAnyN(2, [
        async(fn (): User => $this->findUser($command)),
        async(fn (): PasswordResetRequest => $this->findPasswordResetRequest($command)),
    ]);

    return new UserPasswordResetEvent($user, $passwordResetRequest, $this->clock->now());
}

In the code above, methods findUser() and findPasswordResetRequest() have two characteristics:

  • they are blocking
  • they could throw

By using async/await, these methods benefit from making two concurrent database requests and besides that we have the full context of thrown exceptions in the error scenario owing to the fact that awaitAnyN collects them into CompositeException.

In case of my project, the calling code has exception handler adapter for CompositeException to show to the end user the detailed information about what went wrong.

This code works perfectly fine, but from developer POV it might not be so convenient to pass $count every time to the function. In addition, if we add new futures to the list, we have not to forget to update the count, and this is just not convenient.

You might say that awaitAnyN is not designed to await all given futures and that I should've used awaitAll function instead.

I agree that awaitAnyN is not so designed for such use case, but in my case, I would like to leverage CompositeException
for exception handling, since it provides a neat way to deal with the bunch of exceptions.

Therefore, requested feature could be stated as "awaitAll function throwing CompositeException".

  1. I'm not sure if it's feasible to modify awaitAll, since it would be a BC break;
  2. Another option could be introducing some flag argument for awaitAll, but it's considered as a bad practice and I agree with that;
  3. Maybe creating another function that does the same as awaitAll but throws $errors rather than returning them would be better option given that we leave awaitAll alone.

In any case, please let me know what you think about this feature request and if I could provide a PR for it.

Best regards,
Yevhen

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

No branches or pull requests

2 participants