From 0cf567a4d2f657ec409631ad16291d9e55f68aac Mon Sep 17 00:00:00 2001 From: Aaron Stannard Date: Fri, 20 Dec 2024 08:39:23 -0600 Subject: [PATCH] Fixed: `IWrappedMessage` + `IDeadLetterSuppression` handling (#7414) * WIP dead letter suppression testing * added `WrappedMessage.IsDeadLetterSuppressedAnywhere` * cleaned up message-handling for `DeadLetterActorRef` * added checks for wrapped messages in DeadLetter handling * fixed issue with recursive dead letter detection * added test cases for `ActorSelection` also * checking in `SuppressedDeadLetter` API changes --- ...oreAPISpec.ApproveCore.DotNet.verified.txt | 2 +- .../CoreAPISpec.ApproveCore.Net.verified.txt | 2 +- src/core/Akka.Tests/Actor/DeadLettersSpec.cs | 47 ++++++++++++++-- .../Akka.Tests/Actor/WrappedMessagesSpec.cs | 54 +++++++++++++++++++ src/core/Akka/Actor/BuiltInActors.cs | 35 ++++++++---- src/core/Akka/Actor/DeadLetterMailbox.cs | 8 ++- src/core/Akka/Actor/EmptyLocalActorRef.cs | 10 ++-- src/core/Akka/Event/DeadLetter.cs | 2 +- 8 files changed, 136 insertions(+), 24 deletions(-) create mode 100644 src/core/Akka.Tests/Actor/WrappedMessagesSpec.cs diff --git a/src/core/Akka.API.Tests/verify/CoreAPISpec.ApproveCore.DotNet.verified.txt b/src/core/Akka.API.Tests/verify/CoreAPISpec.ApproveCore.DotNet.verified.txt index 8ee24021bb3..5c67a933868 100644 --- a/src/core/Akka.API.Tests/verify/CoreAPISpec.ApproveCore.DotNet.verified.txt +++ b/src/core/Akka.API.Tests/verify/CoreAPISpec.ApproveCore.DotNet.verified.txt @@ -3686,7 +3686,7 @@ namespace Akka.Event } public sealed class SuppressedDeadLetter : Akka.Event.AllDeadLetters { - public SuppressedDeadLetter(Akka.Event.IDeadLetterSuppression message, Akka.Actor.IActorRef sender, Akka.Actor.IActorRef recipient) { } + public SuppressedDeadLetter(object message, Akka.Actor.IActorRef sender, Akka.Actor.IActorRef recipient) { } } public class TraceLogger : Akka.Actor.UntypedActor { diff --git a/src/core/Akka.API.Tests/verify/CoreAPISpec.ApproveCore.Net.verified.txt b/src/core/Akka.API.Tests/verify/CoreAPISpec.ApproveCore.Net.verified.txt index 42eb4852193..58d759a4813 100644 --- a/src/core/Akka.API.Tests/verify/CoreAPISpec.ApproveCore.Net.verified.txt +++ b/src/core/Akka.API.Tests/verify/CoreAPISpec.ApproveCore.Net.verified.txt @@ -3676,7 +3676,7 @@ namespace Akka.Event } public sealed class SuppressedDeadLetter : Akka.Event.AllDeadLetters { - public SuppressedDeadLetter(Akka.Event.IDeadLetterSuppression message, Akka.Actor.IActorRef sender, Akka.Actor.IActorRef recipient) { } + public SuppressedDeadLetter(object message, Akka.Actor.IActorRef sender, Akka.Actor.IActorRef recipient) { } } public class TraceLogger : Akka.Actor.UntypedActor { diff --git a/src/core/Akka.Tests/Actor/DeadLettersSpec.cs b/src/core/Akka.Tests/Actor/DeadLettersSpec.cs index 4ffc52bc6cd..d20bea7b2fc 100644 --- a/src/core/Akka.Tests/Actor/DeadLettersSpec.cs +++ b/src/core/Akka.Tests/Actor/DeadLettersSpec.cs @@ -13,7 +13,6 @@ namespace Akka.Tests { - public class DeadLettersSpec : AkkaSpec { [Fact] @@ -23,6 +22,48 @@ public async Task Can_send_messages_to_dead_letters() Sys.DeadLetters.Tell("foobar"); await ExpectMsgAsync(deadLetter=>deadLetter.Message.Equals("foobar")); } - } -} + private sealed record WrappedClass(object Message) : IWrappedMessage; + + private sealed class SuppressedMessage : IDeadLetterSuppression + { + + } + + [Fact] + public async Task ShouldLogNormalWrappedMessages() + { + Sys.EventStream.Subscribe(TestActor, typeof(DeadLetter)); + Sys.DeadLetters.Tell(new WrappedClass("chocolate-beans")); + await ExpectMsgAsync(); + } + + [Fact] + public async Task ShouldNotLogWrappedMessagesWithDeadLetterSuppression() + { + Sys.EventStream.Subscribe(TestActor, typeof(AllDeadLetters)); + Sys.DeadLetters.Tell(new WrappedClass(new SuppressedMessage())); + var msg = await ExpectMsgAsync(); + msg.Message.ToString()!.Contains("SuppressedMessage").ShouldBeTrue(); + } + + [Fact] + public async Task ShouldLogNormalActorSelectionWrappedMessages() + { + Sys.EventStream.Subscribe(TestActor, typeof(DeadLetter)); + var selection = Sys.ActorSelection("/user/foobar"); + selection.Tell(new WrappedClass("chocolate-beans")); + await ExpectMsgAsync(); + } + + [Fact] + public async Task ShouldNotLogActorSelectionWrappedMessagesWithDeadLetterSuppression() + { + Sys.EventStream.Subscribe(TestActor, typeof(AllDeadLetters)); + var selection = Sys.ActorSelection("/user/foobar"); + selection.Tell(new WrappedClass(new SuppressedMessage())); + var msg = await ExpectMsgAsync(); + msg.Message.ToString()!.Contains("SuppressedMessage").ShouldBeTrue(); + } + } +} \ No newline at end of file diff --git a/src/core/Akka.Tests/Actor/WrappedMessagesSpec.cs b/src/core/Akka.Tests/Actor/WrappedMessagesSpec.cs new file mode 100644 index 00000000000..c5c4601c6e4 --- /dev/null +++ b/src/core/Akka.Tests/Actor/WrappedMessagesSpec.cs @@ -0,0 +1,54 @@ +// ----------------------------------------------------------------------- +// +// Copyright (C) 2009-2024 Lightbend Inc. +// Copyright (C) 2013-2024 .NET Foundation +// +// ----------------------------------------------------------------------- + +using Akka.Actor; +using Akka.Event; +using Akka.TestKit; +using Xunit; + +namespace Akka.Tests; + +public class WrappedMessagesSpec +{ + private sealed record WrappedClass(object Message) : IWrappedMessage; + + private sealed record WrappedSuppressedClass(object Message) : IWrappedMessage, IDeadLetterSuppression; + + private sealed class SuppressedMessage : IDeadLetterSuppression + { + + } + + + [Fact] + public void ShouldUnwrapWrappedMessage() + { + var message = new WrappedClass("chocolate-beans"); + var unwrapped = WrappedMessage.Unwrap(message); + unwrapped.ShouldBe("chocolate-beans"); + } + + public static readonly TheoryData SuppressedMessages = new() + { + {new SuppressedMessage(), true}, + {new WrappedClass(new SuppressedMessage()), true}, + {new WrappedClass(new WrappedClass(new SuppressedMessage())), true}, + {new WrappedClass(new WrappedClass("chocolate-beans")), false}, + {new WrappedSuppressedClass("foo"), true}, + {new WrappedClass(new WrappedSuppressedClass("chocolate-beans")), true}, + {new WrappedClass("chocolate-beans"), false}, + {"chocolate-beans", false} + }; + + [Theory] + [MemberData(nameof(SuppressedMessages))] + public void ShouldDetectIfWrappedMessageIsSuppressed(object message, bool shouldBeSuppressed) + { + var isSuppressed = WrappedMessage.IsDeadLetterSuppressedAnywhere(message); + isSuppressed.ShouldBe(shouldBeSuppressed); + } +} \ No newline at end of file diff --git a/src/core/Akka/Actor/BuiltInActors.cs b/src/core/Akka/Actor/BuiltInActors.cs index 7ef187eb314..be25f41a383 100644 --- a/src/core/Akka/Actor/BuiltInActors.cs +++ b/src/core/Akka/Actor/BuiltInActors.cs @@ -197,6 +197,18 @@ public static object Unwrap(object message) } return message; } + + internal static bool IsDeadLetterSuppressedAnywhere(object message) + { + var isSuppressed = message is IDeadLetterSuppression; + while(!isSuppressed && message is IWrappedMessage wm) + { + message = wm.Message; + isSuppressed = message is IDeadLetterSuppression; + } + + return isSuppressed; + } } /// @@ -226,19 +238,20 @@ public DeadLetterActorRef(IActorRefProvider provider, ActorPath path, EventStrea /// This exception is thrown if the given is undefined. protected override void TellInternal(object message, IActorRef sender) { - if (message == null) throw new InvalidMessageException("Message is null"); - var i = message as Identify; - if (i != null) + switch (message) { - sender.Tell(new ActorIdentity(i.MessageId, ActorRefs.Nobody)); - return; - } - var d = message as DeadLetter; - if (d != null) - { - if (!SpecialHandle(d.Message, d.Sender)) { _eventStream.Publish(d); } - return; + case null: + throw new InvalidMessageException("Message is null"); + case Identify i: + sender.Tell(new ActorIdentity(i.MessageId, ActorRefs.Nobody)); + return; + case DeadLetter d: + { + if (!SpecialHandle(d.Message, d.Sender)) { _eventStream.Publish(d); } + return; + } } + if (!SpecialHandle(message, sender)) { _eventStream.Publish(new DeadLetter(message, sender.IsNobody() ? Provider.DeadLetters : sender, this)); } } diff --git a/src/core/Akka/Actor/DeadLetterMailbox.cs b/src/core/Akka/Actor/DeadLetterMailbox.cs index 387c7820b18..5455cd1dbbb 100644 --- a/src/core/Akka/Actor/DeadLetterMailbox.cs +++ b/src/core/Akka/Actor/DeadLetterMailbox.cs @@ -46,9 +46,13 @@ public DeadLetterMessageQueue(IActorRef deadLetters) /// TBD public void Enqueue(IActorRef receiver, Envelope envelope) { - if (envelope.Message is DeadLetter) + if (envelope.Message is AllDeadLetters) { - // actor subscribing to DeadLetter. Drop it. + /* We're receiving a DeadLetter sent to us by someone else (which is not normal - usually only happens + * if we were explicitly subscribed to DeadLetters on the EventStream). + * + * Have to terminate here in order to prevent a stack overflow. + */ return; } diff --git a/src/core/Akka/Actor/EmptyLocalActorRef.cs b/src/core/Akka/Actor/EmptyLocalActorRef.cs index be7ed417c11..56f6ae0e286 100644 --- a/src/core/Akka/Actor/EmptyLocalActorRef.cs +++ b/src/core/Akka/Actor/EmptyLocalActorRef.cs @@ -111,9 +111,9 @@ protected virtual bool SpecialHandle(object message, IActorRef sender) } else { - if (actorSelectionMessage.Message is IDeadLetterSuppression selectionDeadLetterSuppression) + if (WrappedMessage.IsDeadLetterSuppressedAnywhere(actorSelectionMessage.Message)) { - PublishSupressedDeadLetter(selectionDeadLetterSuppression, sender); + PublishSupressedDeadLetter(actorSelectionMessage.Message, sender); } else { @@ -123,16 +123,16 @@ protected virtual bool SpecialHandle(object message, IActorRef sender) return true; } - if (message is IDeadLetterSuppression deadLetterSuppression) + if (WrappedMessage.IsDeadLetterSuppressedAnywhere(message)) { - PublishSupressedDeadLetter(deadLetterSuppression, sender); + PublishSupressedDeadLetter(message, sender); return true; } return false; } - private void PublishSupressedDeadLetter(IDeadLetterSuppression msg, IActorRef sender) + private void PublishSupressedDeadLetter(object msg, IActorRef sender) { _eventStream.Publish(new SuppressedDeadLetter(msg, sender.IsNobody() ? _provider.DeadLetters : sender, this)); } diff --git a/src/core/Akka/Event/DeadLetter.cs b/src/core/Akka/Event/DeadLetter.cs index f2175264a06..fcdc6eb41f3 100644 --- a/src/core/Akka/Event/DeadLetter.cs +++ b/src/core/Akka/Event/DeadLetter.cs @@ -104,7 +104,7 @@ public sealed class SuppressedDeadLetter : AllDeadLetters /// /// This exception is thrown when either the sender or the recipient is undefined. /// - public SuppressedDeadLetter(IDeadLetterSuppression message, IActorRef sender, IActorRef recipient) : base(message, sender, recipient) + public SuppressedDeadLetter(object message, IActorRef sender, IActorRef recipient) : base(message, sender, recipient) { if (sender == null) throw new ArgumentNullException(nameof(sender), "SuppressedDeadLetter sender may not be null"); if (recipient == null) throw new ArgumentNullException(nameof(recipient), "SuppressedDeadLetter recipient may not be null");