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.

Sign up for GitHub

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

Jump to bottom

Ask<object> will not throw exception when the response is Status.Failure #7254

Open
ondravondra opened this issue Jun 13, 2024 · 3 comments · May be fixed by #7286
Open

Ask<object> will not throw exception when the response is Status.Failure #7254

ondravondra opened this issue Jun 13, 2024 · 3 comments · May be fixed by #7286
Labels
akka-actor confirmed bug
Milestone
1.5.31

Comments

@ondravondra
Copy link
Contributor

Version Information
Version of Akka.NET? 1.5.15

Describe the bug
Ask will not throw exception when the response is Status.Failure

This is caused by the order of case statements in FutureActorRef<T>.TellInternal:

            switch (message)
            {
                case ISystemMessage msg:
                    handled = _result.TrySetException(new InvalidOperationException($"system message of type '{msg.GetType().Name}' is invalid for {nameof(FutureActorRef<T>)}"));
                    break;
                case T t:
                    handled = _result.TrySetResult(t);
                    break;
                case null:
                    handled = _result.TrySetResult(default);
                    break;
                case Status.Failure f:
                    handled = _result.TrySetException(f.Cause
                        ?? new TaskCanceledException("Task cancelled by actor via Failure message."));
                    break;

when T is object and message is Status.Failure the case T t: is executed instead of case Status.Failure f:.
@ondravondra
Copy link
Contributor Author

This behaviour can be fixed by reordering the case labels, e.g.:

        {
            switch (reply)
            {
                case T wtv:
                    return wtv;
                case null:
                    return default;
                case Status.Failure fail:
                    throw fail.Cause;
                default:
                    throw new ArgumentException();
            }
        }

        private T GetResult2<T>(object reply)
        {
            switch (reply)
            {
                case Status.Failure fail:
                    throw fail.Cause;
                case T wtv:
                    return wtv;
                case null:
                    return default;
                default:
                    throw new ArgumentException();
            }
        }
        ```
The first method returns the Failure as object but the second method rethrows the exception.

@Aaronontheweb
Copy link
Member

Nice catch! Yes, we should apply your fix and just re-do the case labels here. If you'd like to send in a small PR with a reproduction spec we'd be happy to merge it. We can write something ourselves too - but totally up to you.

@Aaronontheweb Aaronontheweb modified the milestones: 1.5.25, 1.5.26 Jun 13, 2024
@ondravondra
Copy link
Contributor Author

reproduction spec we'd be happy to merge it. We can write something ourselves too - but totally up to you.

I am afraid I won't be able to write a PR with tests soon, I am really busy at work, sorry. Thanks for accepting the bug report tho, fixing it will allow us to simplify our company code.

@Aaronontheweb Aaronontheweb modified the milestones: 1.5.26, 1.5.27 Jun 27, 2024
@ondravondra ondravondra linked a pull request Jul 17, 2024 that will close this issue
Open
@Aaronontheweb Aaronontheweb modified the milestones: 1.5.27, 1.5.28 Jul 25, 2024
@Aaronontheweb Aaronontheweb modified the milestones: 1.5.28, 1.5.29 Sep 4, 2024
@Aaronontheweb Aaronontheweb modified the milestones: 1.5.29, 1.5.30, 1.5.31 Oct 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
akka-actor confirmed bug
Projects
None yet
Milestone
1.5.31
Development

Successfully merging a pull request may close this issue.

convert Failure to exception when using Ask<object> ondravondra/akka.net
2 participants
@Aaronontheweb @ondravondra