PipeTo creates an invalid Status.Failure object if task is cancelled

[akoshelev]

The following code cause problems if tresult is cancelled

akka.net/src/core/Akka/Actor/PipeToSupport.cs

Line 29 in e724d60

if (tresult.IsCanceled || tresult.IsFaulted)

 if (tresult.IsCanceled || tresult.IsFaulted)
                    recipient.Tell(failure != null
                        ? failure((Exception)tresult.Exception)
                        // if tresult is cancelled, tresult.Exception == null
                        : new Status.Failure((Exception)tresult.Exception), sender);
...

If user does not handle this message in recipient actor and akka.actor.debug.unhandled = true, NullReferenceException is thrown, as UnhandledMessageForwarder calls message.ToString()

akka.net/src/core/Akka/Event/LoggingBus.cs

Line 235 in cf7cd6f

message.Message

Status.Failure does not check the input parameter and ToString() overload calls Cause.ToString() w/o checking for null.

akka.net/src/core/Akka/Actor/ActorBase.cs

Line 48 in 4974426

return "Failure: " + Cause.ToString();

What'd be the correct way to fix this? I see a couple of possible fixes:

• Create a TaskCancelledException inside PipeToSupport class
• Extend Status.Failure and allow to create an instance without specifying an exception - maybe indicate that task was cancelled by passing a task instance

[rogeralsing]

Extend Status.Failure and allow to create an instance without specifying an exception - maybe indicate that task was cancelled by passing a task instance

Do note that the Failure class could be passed between nodes, and thus needs to serialize well.
So if we give it an overload for failed Task then we need to extract the exception information from it so that we do not hold a reference to the task itself.
Maybe I'm stating the obvious here but whatever solution we go for, Failure needs to be serialization friendly.

[Aaronontheweb]

Create a TaskCancelledException inside PipeToSupport class

I've refactored to this method to do this like 3 times, but then I remember that the way it's designed currently is consistent with the JVM design and even enforced via unit test.

Frankly I think the JVM design here is pretty shitty - just throw an exception.

[akoshelev]

What if we just create a TaskCancelledException there? It won't break current design, although looks a bit weird, and I see similar code in other modules

akka.net/src/core/Akka/Actor/Inbox.Actor.cs

Line 127 in cf7cd6f

query.Client.Tell(new Status.Failure(new TimeoutException("Deadline passed")));

[guessmyname]

Hello. Any word on a fix for this issue? I'm getting the same issue.
I would be happy to work on this bug. Just let me know how I should approach the fix.
Thanks.
Brad

[Horusiath]

Related: #3320

[Aaronontheweb]

Resolved via #5123