From e1f13f4dfa43a7ef60e627ce8d209ee2ae0312b8 Mon Sep 17 00:00:00 2001 From: Aaron Stannard Date: Thu, 15 May 2025 10:51:00 -0500 Subject: [PATCH 1/7] Fix #7626: Using DeadLetterWithNoSubscribers to clearly indicate when there are no subscribers --- .../DeadLetterWithNoSubscribers.cs | 40 +++++++++++++++++++ .../DistributedPubSubMediator.cs | 10 ++++- src/core/Akka/Event/DeadLetterListener.cs | 2 +- 3 files changed, 49 insertions(+), 3 deletions(-) create mode 100644 src/contrib/cluster/Akka.Cluster.Tools/PublishSubscribe/DeadLetterWithNoSubscribers.cs diff --git a/src/contrib/cluster/Akka.Cluster.Tools/PublishSubscribe/DeadLetterWithNoSubscribers.cs b/src/contrib/cluster/Akka.Cluster.Tools/PublishSubscribe/DeadLetterWithNoSubscribers.cs new file mode 100644 index 00000000000..e9a88b102b6 --- /dev/null +++ b/src/contrib/cluster/Akka.Cluster.Tools/PublishSubscribe/DeadLetterWithNoSubscribers.cs @@ -0,0 +1,40 @@ +//----------------------------------------------------------------------- +// +// Copyright (C) 2009-2022 Lightbend Inc. +// Copyright (C) 2013-2025 .NET Foundation +// +//----------------------------------------------------------------------- + +using Akka.Actor; +using Akka.Event; + +namespace Akka.Cluster.Tools.PublishSubscribe +{ + /// + /// Special case of Dead Letter that explicitly indicates the message was sent to + /// DeadLetters because there were no subscribers for the topic in DistributedPubSub, + /// NOT because the mediator itself is dead. + /// + public sealed class DeadLetterWithNoSubscribers : AllDeadLetters + { + /// + /// Initializes a new instance of the class. + /// + /// The original message that could not be delivered. + /// The actor that sent the message. + /// The actor that was to receive the message (usually the mediator itself). + public DeadLetterWithNoSubscribers(object message, IActorRef sender, IActorRef recipient) + : base(message, sender, recipient) + { + } + + /// + /// Returns a string that represents the current object. + /// + /// A string that represents the current object. + public override string ToString() + { + return $"DeadLetterWithNoSubscribers from {Sender} to {Recipient}: <{Message}> - No subscribers found for topic"; + } + } +} \ No newline at end of file diff --git a/src/contrib/cluster/Akka.Cluster.Tools/PublishSubscribe/DistributedPubSubMediator.cs b/src/contrib/cluster/Akka.Cluster.Tools/PublishSubscribe/DistributedPubSubMediator.cs index 7333424fd8d..48b28508d9a 100644 --- a/src/contrib/cluster/Akka.Cluster.Tools/PublishSubscribe/DistributedPubSubMediator.cs +++ b/src/contrib/cluster/Akka.Cluster.Tools/PublishSubscribe/DistributedPubSubMediator.cs @@ -1,4 +1,4 @@ -//----------------------------------------------------------------------- +//----------------------------------------------------------------------- // // Copyright (C) 2009-2022 Lightbend Inc. // Copyright (C) 2013-2025 .NET Foundation @@ -490,7 +490,13 @@ private void PutToRegistry(string key, IActorRef value) private void IgnoreOrSendToDeadLetters(object message) { if (_settings.SendToDeadLettersWhenNoSubscribers) - Context.System.DeadLetters.Tell(new DeadLetter(message, Sender, Context.Self)); + { + // Use the specialized DeadLetterWithNoSubscribers class to clearly indicate + // that the message was not delivered because there were no subscribers, + // not because the mediator itself is dead. + var deadLetter = new DeadLetterWithNoSubscribers(message, Sender, Context.Self); + Context.System.DeadLetters.Tell(deadLetter); + } } private void PublishMessage(string path, IWrappedMessage publish, bool allButSelf = false) diff --git a/src/core/Akka/Event/DeadLetterListener.cs b/src/core/Akka/Event/DeadLetterListener.cs index fe45d1cf6da..38e4f32af27 100644 --- a/src/core/Akka/Event/DeadLetterListener.cs +++ b/src/core/Akka/Event/DeadLetterListener.cs @@ -1,4 +1,4 @@ -//----------------------------------------------------------------------- +//----------------------------------------------------------------------- // // Copyright (C) 2009-2022 Lightbend Inc. // Copyright (C) 2013-2025 .NET Foundation From 9f625df579a323b4d01127157a98adc00d506777 Mon Sep 17 00:00:00 2001 From: Aaron Stannard Date: Thu, 15 May 2025 10:59:11 -0500 Subject: [PATCH 2/7] Fixed #7626: Using internal DeadLetterWithNoSubscribers class to clearly indicate no subscriber case in dead letter messages - Made DeadLetterWithNoSubscribers internal (class already existed) - Added test to verify dead letter messages are sent when no subscribers exist --- .../DistributedPubSubDeadLetterSpec.cs | 49 +++++++++++++++++++ .../DeadLetterWithNoSubscribers.cs | 2 +- 2 files changed, 50 insertions(+), 1 deletion(-) create mode 100644 src/contrib/cluster/Akka.Cluster.Tools.Tests/PublishSubscribe/DistributedPubSubDeadLetterSpec.cs diff --git a/src/contrib/cluster/Akka.Cluster.Tools.Tests/PublishSubscribe/DistributedPubSubDeadLetterSpec.cs b/src/contrib/cluster/Akka.Cluster.Tools.Tests/PublishSubscribe/DistributedPubSubDeadLetterSpec.cs new file mode 100644 index 00000000000..e00a548047f --- /dev/null +++ b/src/contrib/cluster/Akka.Cluster.Tools.Tests/PublishSubscribe/DistributedPubSubDeadLetterSpec.cs @@ -0,0 +1,49 @@ +//----------------------------------------------------------------------- +// +// Copyright (C) 2009-2022 Lightbend Inc. +// Copyright (C) 2013-2025 .NET Foundation +// +//----------------------------------------------------------------------- + +using System.Threading.Tasks; +using Akka.Actor; +using Akka.Cluster.Tools.PublishSubscribe; +using Akka.Configuration; +using Akka.Event; +using Akka.TestKit; +using Xunit; + +namespace Akka.Cluster.Tools.Tests.PublishSubscribe +{ + [Collection(nameof(DistributedPubSubDeadLetterSpec))] + public class DistributedPubSubDeadLetterSpec : AkkaSpec + { + public DistributedPubSubDeadLetterSpec() : base(GetConfig()) + { + } + + public static Config GetConfig() + { + return ConfigurationFactory.ParseString( + @"akka.actor.provider = cluster" + + "\nakka.loglevel = INFO" + + "\nakka.log-dead-letters = on" + + "\nakka.cluster.pub-sub.send-to-dead-letters-when-no-subscribers = on"); + } + + [Fact] + public async Task DistributedPubSubMediator_should_send_specialized_dead_letter_message_when_no_subscribers() + { + // arrange + var mediator = DistributedPubSub.Get(Sys).Mediator; + var testMessage = "test-message"; + + // act - publish to a topic that no one is subscribed to + await EventFilter.DeadLetter().ExpectAsync(1, () => + { + mediator.Tell(new Publish("unused-topic", testMessage)); + return Task.CompletedTask; + }); + } + } +} \ No newline at end of file diff --git a/src/contrib/cluster/Akka.Cluster.Tools/PublishSubscribe/DeadLetterWithNoSubscribers.cs b/src/contrib/cluster/Akka.Cluster.Tools/PublishSubscribe/DeadLetterWithNoSubscribers.cs index e9a88b102b6..4fb03fc67a1 100644 --- a/src/contrib/cluster/Akka.Cluster.Tools/PublishSubscribe/DeadLetterWithNoSubscribers.cs +++ b/src/contrib/cluster/Akka.Cluster.Tools/PublishSubscribe/DeadLetterWithNoSubscribers.cs @@ -15,7 +15,7 @@ namespace Akka.Cluster.Tools.PublishSubscribe /// DeadLetters because there were no subscribers for the topic in DistributedPubSub, /// NOT because the mediator itself is dead. /// - public sealed class DeadLetterWithNoSubscribers : AllDeadLetters + internal sealed class DeadLetterWithNoSubscribers : AllDeadLetters { /// /// Initializes a new instance of the class. From e51228f0e4d32d44c9ddf3539a66dc579b4fee58 Mon Sep 17 00:00:00 2001 From: Aaron Stannard Date: Thu, 15 May 2025 11:17:16 -0500 Subject: [PATCH 3/7] Update AGENT.md with additional TestKit guidance for ITestOutputHelper --- AGENT.md | 74 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 74 insertions(+) create mode 100644 AGENT.md diff --git a/AGENT.md b/AGENT.md new file mode 100644 index 00000000000..622aa90a130 --- /dev/null +++ b/AGENT.md @@ -0,0 +1,74 @@ +# Akka.NET Agent Guidelines + +## Build/Test Commands +- Build solution: `dotnet build` +- Build with warnings as errors: `dotnet build -warnaserror` +- Run all tests: `dotnet test -c Release` +- Run specific test: `dotnet test -c Release --filter DisplayName="TestName"` or `dotnet test path/to/project.csproj` +- Format check: `dotnet format --verify-no-changes` + +## Git Repository Management +- Setup remotes: + - `git remote add upstream https://github.com/akkadotnet/akka.net.git` (main repository) + - `git remote add origin https://github.com/yourusername/akka.net.git` (your fork) +- Sync with upstream: + - `git fetch upstream` (get latest changes from main repo) + - `git checkout dev` (switch to dev branch) + - `git merge upstream/dev` (merge changes from upstream) + - `git push origin dev` (update your fork) +- Create feature branch: + - `git checkout -b feature/your-feature-name` (create and switch to new branch) + - `git push -u origin feature/your-feature-name` (push branch to your fork) + +## Code Style Guidelines +- Use Allman style brackets for C# code (opening brace on new line) +- 4 spaces for indentation +- Prefer "var" everywhere when type is apparent +- Private fields start with `_` (underscore), PascalCase for public/protected members +- No "this." qualifier when unnecessary +- Use exceptions for error handling (IllegalStateException for invalid states) +- Sort using statements with System.* appearing first +- XML comments for public APIs +- Name tests with descriptive `DisplayName=` attributes +- Default to `sealed` classes and records for data objects +- Enable nullability in new/modified files with `#nullable enable` +- Never use `async void`, `.Result`, or `.Wait()` - these cause deadlocks +- Always pass `CancellationToken` in async methods + +## API Approvals +- Run API approval tests when making public API changes: `dotnet test -c Release src/core/Akka.API.Tests` +- Approval files are located at `src/core/Akka.API.Tests/CoreAPISpec.ApproveCore.approved.txt` +- Install a diff viewer like WinMerge or TortoiseMerge to approve API changes +- Follow extend-only design principles - don't modify existing public APIs, only extend them +- Mark deprecated APIs with `[Obsolete("Obsolete since v{current-akka-version}")]` + +## Conventions +- Stay close to JVM Akka where applicable but be .NET idiomatic +- Use Task instead of Future, TimeSpan instead of Duration +- Include unit tests with changes +- Preserve public API and wire compatibility +- Keep pull requests small and focused (<300 lines when possible) +- Fix warnings instead of suppressing them +- Treat TBD comments as action items to be resolved +- Benchmark performance-critical code changes with BenchmarkDotNet +- Avoid adding new dependencies without license/security checks + +## Akka.NET TestKit Guidelines +- Actor tests should derive from `AkkaSpec` or `TestKit` to access actor testing facilities +- Pass `ITestOutputHelper output` to the constructor and base constructor: `public MySpec(ITestOutputHelper output) : base(config, output)` +- Use the `ITestOutputHelper` output for debugging: it captures all test output including actor system logs +- Configure proper logging in tests: `akka.loglevel = DEBUG` or `akka.loglevel = INFO` +- Use `EventFilter` to assert on log messages (e.g., `EventFilter.Error().ExpectOne(() => { /* test code */ });`) +- For testing deadletters, use `EventFilter.DeadLetter().Expect(1, () => { /* code that should produce dead letter */ });` +- Test message assertions using `ExpectMsg()`, `ExpectNoMsg()`, or `FishForMessage()` +- Set explicit timeouts for message expectations to avoid long-running tests +- Use `TestProbe` to create lightweight test actors to verify interactions +- Tests should clean up after themselves (stop created actors, reset state) +- To test specialized message types, verify the type wrapper in logs: `wrapped in [$TypeName]` + +## Repository Landmarks +- `src/` - All runtime / library code +- `src/benchmark/` - Micro-benchmarks (BenchmarkDotNet) +- `src/…Tests/` - xUnit test projects +- `docs/community/contributing/` - Contributor policies & style guides +- `docs/` - Public facing documentation \ No newline at end of file From 9bbba7eecbdbbede56cca9bf5031b6fecb1b52a9 Mon Sep 17 00:00:00 2001 From: Aaron Stannard Date: Thu, 15 May 2025 11:17:40 -0500 Subject: [PATCH 4/7] fix `DistributedPubSubDeadLetterSpec` --- .../DistributedPubSubDeadLetterSpec.cs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/contrib/cluster/Akka.Cluster.Tools.Tests/PublishSubscribe/DistributedPubSubDeadLetterSpec.cs b/src/contrib/cluster/Akka.Cluster.Tools.Tests/PublishSubscribe/DistributedPubSubDeadLetterSpec.cs index e00a548047f..dcdd0537079 100644 --- a/src/contrib/cluster/Akka.Cluster.Tools.Tests/PublishSubscribe/DistributedPubSubDeadLetterSpec.cs +++ b/src/contrib/cluster/Akka.Cluster.Tools.Tests/PublishSubscribe/DistributedPubSubDeadLetterSpec.cs @@ -12,13 +12,13 @@ using Akka.Event; using Akka.TestKit; using Xunit; +using Xunit.Abstractions; namespace Akka.Cluster.Tools.Tests.PublishSubscribe { - [Collection(nameof(DistributedPubSubDeadLetterSpec))] public class DistributedPubSubDeadLetterSpec : AkkaSpec { - public DistributedPubSubDeadLetterSpec() : base(GetConfig()) + public DistributedPubSubDeadLetterSpec(ITestOutputHelper output) : base(GetConfig(), output) { } @@ -27,8 +27,7 @@ public static Config GetConfig() return ConfigurationFactory.ParseString( @"akka.actor.provider = cluster" + "\nakka.loglevel = INFO" - + "\nakka.log-dead-letters = on" - + "\nakka.cluster.pub-sub.send-to-dead-letters-when-no-subscribers = on"); + + "\nakka.log-dead-letters = on"); } [Fact] @@ -39,7 +38,8 @@ public async Task DistributedPubSubMediator_should_send_specialized_dead_letter_ var testMessage = "test-message"; // act - publish to a topic that no one is subscribed to - await EventFilter.DeadLetter().ExpectAsync(1, () => + await EventFilter.Info(contains: "DeadLetterWithNoSubscribers") + .ExpectAsync(1, () => { mediator.Tell(new Publish("unused-topic", testMessage)); return Task.CompletedTask; From 99abc7df074e133bc90d51a3b2ac25f2d673f3cd Mon Sep 17 00:00:00 2001 From: Aaron Stannard Date: Thu, 15 May 2025 11:24:28 -0500 Subject: [PATCH 5/7] added topic logging --- .../PublishSubscribe/DeadLetterWithNoSubscribers.cs | 8 ++++++-- .../PublishSubscribe/DistributedPubSubMediator.cs | 11 +++++++++-- 2 files changed, 15 insertions(+), 4 deletions(-) diff --git a/src/contrib/cluster/Akka.Cluster.Tools/PublishSubscribe/DeadLetterWithNoSubscribers.cs b/src/contrib/cluster/Akka.Cluster.Tools/PublishSubscribe/DeadLetterWithNoSubscribers.cs index 4fb03fc67a1..d3627c7ba46 100644 --- a/src/contrib/cluster/Akka.Cluster.Tools/PublishSubscribe/DeadLetterWithNoSubscribers.cs +++ b/src/contrib/cluster/Akka.Cluster.Tools/PublishSubscribe/DeadLetterWithNoSubscribers.cs @@ -21,20 +21,24 @@ internal sealed class DeadLetterWithNoSubscribers : AllDeadLetters /// Initializes a new instance of the class. /// /// The original message that could not be delivered. + /// The topic that the message was sent to. /// The actor that sent the message. /// The actor that was to receive the message (usually the mediator itself). - public DeadLetterWithNoSubscribers(object message, IActorRef sender, IActorRef recipient) + public DeadLetterWithNoSubscribers(object message, string? topic, IActorRef sender, IActorRef recipient) : base(message, sender, recipient) { + Topic = topic; } + public string Topic { get; } + /// /// Returns a string that represents the current object. /// /// A string that represents the current object. public override string ToString() { - return $"DeadLetterWithNoSubscribers from {Sender} to {Recipient}: <{Message}> - No subscribers found for topic"; + return $"DeadLetterWithNoSubscribers from {Sender} to {Recipient}: <{Message}> - No subscribers found for topic {Topic}"; } } } \ No newline at end of file diff --git a/src/contrib/cluster/Akka.Cluster.Tools/PublishSubscribe/DistributedPubSubMediator.cs b/src/contrib/cluster/Akka.Cluster.Tools/PublishSubscribe/DistributedPubSubMediator.cs index 48b28508d9a..ca7618c2134 100644 --- a/src/contrib/cluster/Akka.Cluster.Tools/PublishSubscribe/DistributedPubSubMediator.cs +++ b/src/contrib/cluster/Akka.Cluster.Tools/PublishSubscribe/DistributedPubSubMediator.cs @@ -487,14 +487,21 @@ private void PutToRegistry(string key, IActorRef value) _registry[_cluster.SelfAddress] = new Bucket(bucket.Owner, v, bucket.Content.SetItem(key, new ValueHolder(v, value))); } - private void IgnoreOrSendToDeadLetters(object message) + private void IgnoreOrSendToDeadLetters(IDistributedPubSubMessage message) { if (_settings.SendToDeadLettersWhenNoSubscribers) { + var topic = message switch + { + Publish publish => publish.Topic, + Send send => $"Send:{send.Path}", + _ => null + }; + // Use the specialized DeadLetterWithNoSubscribers class to clearly indicate // that the message was not delivered because there were no subscribers, // not because the mediator itself is dead. - var deadLetter = new DeadLetterWithNoSubscribers(message, Sender, Context.Self); + var deadLetter = new DeadLetterWithNoSubscribers(message, topic, Sender, Context.Self); Context.System.DeadLetters.Tell(deadLetter); } } From 77fdd9f051257e03ae5f9f9bab8286917ba80b32 Mon Sep 17 00:00:00 2001 From: Aaron Stannard Date: Thu, 15 May 2025 11:25:38 -0500 Subject: [PATCH 6/7] be consistent about nullables --- .../PublishSubscribe/DeadLetterWithNoSubscribers.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/contrib/cluster/Akka.Cluster.Tools/PublishSubscribe/DeadLetterWithNoSubscribers.cs b/src/contrib/cluster/Akka.Cluster.Tools/PublishSubscribe/DeadLetterWithNoSubscribers.cs index d3627c7ba46..91dec28d712 100644 --- a/src/contrib/cluster/Akka.Cluster.Tools/PublishSubscribe/DeadLetterWithNoSubscribers.cs +++ b/src/contrib/cluster/Akka.Cluster.Tools/PublishSubscribe/DeadLetterWithNoSubscribers.cs @@ -30,7 +30,7 @@ public DeadLetterWithNoSubscribers(object message, string? topic, IActorRef send Topic = topic; } - public string Topic { get; } + public string? Topic { get; } /// /// Returns a string that represents the current object. From 67063262c93139eba489406466226d692013d30f Mon Sep 17 00:00:00 2001 From: Aaron Stannard Date: Thu, 15 May 2025 11:35:52 -0500 Subject: [PATCH 7/7] fix compilation issue --- .../PublishSubscribe/DistributedPubSubMediator.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/contrib/cluster/Akka.Cluster.Tools/PublishSubscribe/DistributedPubSubMediator.cs b/src/contrib/cluster/Akka.Cluster.Tools/PublishSubscribe/DistributedPubSubMediator.cs index ca7618c2134..b5a4fc01ce9 100644 --- a/src/contrib/cluster/Akka.Cluster.Tools/PublishSubscribe/DistributedPubSubMediator.cs +++ b/src/contrib/cluster/Akka.Cluster.Tools/PublishSubscribe/DistributedPubSubMediator.cs @@ -487,7 +487,7 @@ private void PutToRegistry(string key, IActorRef value) _registry[_cluster.SelfAddress] = new Bucket(bucket.Owner, v, bucket.Content.SetItem(key, new ValueHolder(v, value))); } - private void IgnoreOrSendToDeadLetters(IDistributedPubSubMessage message) + private void IgnoreOrSendToDeadLetters(IWrappedMessage message) { if (_settings.SendToDeadLettersWhenNoSubscribers) {