From 7f2d57d38ebbc6e71dd7d86dcd88ba09a5929f6b Mon Sep 17 00:00:00 2001 From: Aaron Stannard Date: Wed, 18 Dec 2024 11:54:49 -0600 Subject: [PATCH 1/7] WIP dead letter suppression testing --- src/core/Akka.Tests/Actor/DeadLettersSpec.cs | 32 +++++++++-- .../Akka.Tests/Actor/WrappedMessagesSpec.cs | 54 +++++++++++++++++++ 2 files changed, 83 insertions(+), 3 deletions(-) create mode 100644 src/core/Akka.Tests/Actor/WrappedMessagesSpec.cs diff --git a/src/core/Akka.Tests/Actor/DeadLettersSpec.cs b/src/core/Akka.Tests/Actor/DeadLettersSpec.cs index 4ffc52bc6cd..c8360d8f99d 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,33 @@ 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")); + + // this is just to make the test deterministic + await ExpectMsgAsync(); + } + + [Fact] + public async Task ShouldNotLogWrappedMessagesWithDeadLetterSuppression() + { + Sys.EventStream.Subscribe(TestActor, typeof(AllDeadLetters)); + Sys.DeadLetters.Tell(new WrappedClass(new SuppressedMessage())); + + // this is just to make the test deterministic + 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..91df0761542 --- /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 expected) + // { + // var isSuppressed = WrappedMessage.IsDeadLetterSuppressedAnywhere(message); + // isSuppressed.ShouldBe(expected); + // } +} \ No newline at end of file From 04e3186e004565854532217c267189acc4e69150 Mon Sep 17 00:00:00 2001 From: Aaron Stannard Date: Wed, 18 Dec 2024 13:11:00 -0600 Subject: [PATCH 2/7] added `WrappedMessage.IsDeadLetterSuppressedAnywhere` --- src/core/Akka.Tests/Actor/WrappedMessagesSpec.cs | 14 +++++++------- src/core/Akka/Actor/BuiltInActors.cs | 12 ++++++++++++ 2 files changed, 19 insertions(+), 7 deletions(-) diff --git a/src/core/Akka.Tests/Actor/WrappedMessagesSpec.cs b/src/core/Akka.Tests/Actor/WrappedMessagesSpec.cs index 91df0761542..c5c4601c6e4 100644 --- a/src/core/Akka.Tests/Actor/WrappedMessagesSpec.cs +++ b/src/core/Akka.Tests/Actor/WrappedMessagesSpec.cs @@ -44,11 +44,11 @@ public void ShouldUnwrapWrappedMessage() {"chocolate-beans", false} }; - // [Theory] - // [MemberData(nameof(SuppressedMessages))] - // public void ShouldDetectIfWrappedMessageIsSuppressed(object message, bool expected) - // { - // var isSuppressed = WrappedMessage.IsDeadLetterSuppressedAnywhere(message); - // isSuppressed.ShouldBe(expected); - // } + [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..34366149f89 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; + } } /// From c9e14fe2b0b6a68e2130ee7d3ecdd816b87bd071 Mon Sep 17 00:00:00 2001 From: Aaron Stannard Date: Wed, 18 Dec 2024 13:14:14 -0600 Subject: [PATCH 3/7] cleaned up message-handling for `DeadLetterActorRef` --- src/core/Akka.Tests/Actor/DeadLettersSpec.cs | 4 ---- src/core/Akka/Actor/BuiltInActors.cs | 23 ++++++++++---------- 2 files changed, 12 insertions(+), 15 deletions(-) diff --git a/src/core/Akka.Tests/Actor/DeadLettersSpec.cs b/src/core/Akka.Tests/Actor/DeadLettersSpec.cs index c8360d8f99d..718871b1801 100644 --- a/src/core/Akka.Tests/Actor/DeadLettersSpec.cs +++ b/src/core/Akka.Tests/Actor/DeadLettersSpec.cs @@ -35,8 +35,6 @@ public async Task ShouldLogNormalWrappedMessages() { Sys.EventStream.Subscribe(TestActor, typeof(DeadLetter)); Sys.DeadLetters.Tell(new WrappedClass("chocolate-beans")); - - // this is just to make the test deterministic await ExpectMsgAsync(); } @@ -45,8 +43,6 @@ public async Task ShouldNotLogWrappedMessagesWithDeadLetterSuppression() { Sys.EventStream.Subscribe(TestActor, typeof(AllDeadLetters)); Sys.DeadLetters.Tell(new WrappedClass(new SuppressedMessage())); - - // this is just to make the test deterministic var msg = await ExpectMsgAsync(); msg.Message.ToString()!.Contains("SuppressedMessage").ShouldBeTrue(); } diff --git a/src/core/Akka/Actor/BuiltInActors.cs b/src/core/Akka/Actor/BuiltInActors.cs index 34366149f89..be25f41a383 100644 --- a/src/core/Akka/Actor/BuiltInActors.cs +++ b/src/core/Akka/Actor/BuiltInActors.cs @@ -238,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)); } } From 1e8cb3aeea696e0863660d27b539c66d4c24d7c7 Mon Sep 17 00:00:00 2001 From: Aaron Stannard Date: Wed, 18 Dec 2024 13:21:29 -0600 Subject: [PATCH 4/7] added checks for wrapped messages in DeadLetter handling --- src/core/Akka/Actor/EmptyLocalActorRef.cs | 6 +++--- src/core/Akka/Event/DeadLetter.cs | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/core/Akka/Actor/EmptyLocalActorRef.cs b/src/core/Akka/Actor/EmptyLocalActorRef.cs index be7ed417c11..1b139c284c8 100644 --- a/src/core/Akka/Actor/EmptyLocalActorRef.cs +++ b/src/core/Akka/Actor/EmptyLocalActorRef.cs @@ -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"); From 0495506651f7bcab309bc26e2d6ee93d64775a2c Mon Sep 17 00:00:00 2001 From: Aaron Stannard Date: Wed, 18 Dec 2024 13:30:32 -0600 Subject: [PATCH 5/7] fixed issue with recursive dead letter detection --- src/core/Akka/Actor/DeadLetterMailbox.cs | 8 ++++++-- src/core/Akka/Actor/EmptyLocalActorRef.cs | 4 ++-- 2 files changed, 8 insertions(+), 4 deletions(-) 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 1b139c284c8..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 { From 5a1f914ad74e94255f71aa9ca45fc5f2eed47bbd Mon Sep 17 00:00:00 2001 From: Aaron Stannard Date: Wed, 18 Dec 2024 13:33:06 -0600 Subject: [PATCH 6/7] added test cases for `ActorSelection` also --- src/core/Akka.Tests/Actor/DeadLettersSpec.cs | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/src/core/Akka.Tests/Actor/DeadLettersSpec.cs b/src/core/Akka.Tests/Actor/DeadLettersSpec.cs index 718871b1801..d20bea7b2fc 100644 --- a/src/core/Akka.Tests/Actor/DeadLettersSpec.cs +++ b/src/core/Akka.Tests/Actor/DeadLettersSpec.cs @@ -46,5 +46,24 @@ public async Task ShouldNotLogWrappedMessagesWithDeadLetterSuppression() 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 From e9115f5aebff9e2b13de020618e92324e5697f21 Mon Sep 17 00:00:00 2001 From: Aaron Stannard Date: Wed, 18 Dec 2024 13:41:04 -0600 Subject: [PATCH 7/7] checking in `SuppressedDeadLetter` API changes --- .../verify/CoreAPISpec.ApproveCore.DotNet.verified.txt | 2 +- .../verify/CoreAPISpec.ApproveCore.Net.verified.txt | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) 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 {