diff --git a/src/ZB.MOM.WW.ScadaBridge.Communication/Actors/CentralCommunicationActor.cs b/src/ZB.MOM.WW.ScadaBridge.Communication/Actors/CentralCommunicationActor.cs index e60fd276..a6450021 100644 --- a/src/ZB.MOM.WW.ScadaBridge.Communication/Actors/CentralCommunicationActor.cs +++ b/src/ZB.MOM.WW.ScadaBridge.Communication/Actors/CentralCommunicationActor.cs @@ -539,9 +539,23 @@ public class CentralCommunicationActor : ReceiveActor { _log.Info("Updating ClusterClient for site {0} (addresses changed)", siteId); Context.Stop(_siteClients[siteId].Client); + // Remove now: if the replacement create below fails, a stale entry + // would route envelopes to a stopping actor. + _siteClients.Remove(siteId); } - var client = _siteClientFactory.Create(Context.System, siteId, contactPaths); + IActorRef client; + try + { + client = _siteClientFactory.Create(Context.System, siteId, contactPaths); + } + catch (Exception ex) + { + _log.Error(ex, + "Failed to create ClusterClient for site {0}; site is unroutable until the next refresh", + siteId); + continue; + } _siteClients[siteId] = (client, contactStrings); _log.Info("Created ClusterClient for site {0} with {1} contact(s)", siteId, addresses.Count); } diff --git a/tests/ZB.MOM.WW.ScadaBridge.Communication.Tests/CentralCommunicationActorClientLifecycleTests.cs b/tests/ZB.MOM.WW.ScadaBridge.Communication.Tests/CentralCommunicationActorClientLifecycleTests.cs new file mode 100644 index 00000000..ce421f1a --- /dev/null +++ b/tests/ZB.MOM.WW.ScadaBridge.Communication.Tests/CentralCommunicationActorClientLifecycleTests.cs @@ -0,0 +1,82 @@ +using System.Collections.Immutable; +using Akka.Actor; +using Akka.Cluster.Tools.Client; +using Akka.Configuration; +using Akka.TestKit.Xunit2; +using Microsoft.Extensions.DependencyInjection; +using NSubstitute; +using Xunit; +using ZB.MOM.WW.ScadaBridge.Commons.Entities.Sites; +using ZB.MOM.WW.ScadaBridge.Commons.Interfaces.Repositories; +using ZB.MOM.WW.ScadaBridge.Communication; +using ZB.MOM.WW.ScadaBridge.Communication.Actors; + +namespace ZB.MOM.WW.ScadaBridge.Communication.Tests; + +public class CentralCommunicationActorClientLifecycleTests : TestKit +{ + private static readonly Config TestConfig = ConfigurationFactory.ParseString(@" + akka.actor.provider = cluster + akka.remote.dot-netty.tcp.port = 0 + akka.remote.dot-netty.tcp.hostname = localhost") + .WithFallback(ClusterClientReceptionist.DefaultConfig()); + + public CentralCommunicationActorClientLifecycleTests() : base(TestConfig) { } + + // Empty ServiceProvider with a no-op ISiteRepository so the actor's periodic + // db-refresh (fired at PreStart) resolves and returns no sites, keeping the + // logs clean; the tests drive SiteAddressCacheLoaded directly. + private static IServiceProvider EmptyProvider() + { + var siteRepo = Substitute.For(); + siteRepo.GetAllSitesAsync(Arg.Any()).Returns(new List()); + var services = new ServiceCollection(); + services.AddScoped(_ => siteRepo); + return services.BuildServiceProvider(); + } + + private static SiteAddressCacheLoaded Load(string siteId, params string[] addrs) => + new(new Dictionary> + { [siteId] = addrs.ToList().AsReadOnly() }); + + [Fact] + public void AddressEdit_RecreatesClient_WithoutRestartLoop() + { + var actor = Sys.ActorOf(Props.Create(() => new CentralCommunicationActor( + EmptyProvider(), new DefaultSiteClientFactory(), null))); + + // First load creates the client; second load (edited NodeA address) stops + // the old one and creates a replacement in the same message handling. + // Pre-fix this throws InvalidActorNameException and restarts the actor. + EventFilter.Exception().Expect(0, () => + { + actor.Tell(Load("site-a", "akka.tcp://scadabridge@node-a:8081")); + actor.Tell(Load("site-a", "akka.tcp://scadabridge@node-a-edited:8081")); + actor.Tell(Load("site-a", "akka.tcp://scadabridge@node-a:8081")); // and back — third generation + }); + + // The actor must still be alive and routing (not crash-looping): + // an envelope for an unknown site produces the "No ClusterClient" warning, + // proving the Receive pipeline is healthy. + EventFilter.Warning(contains: "No ClusterClient for site").ExpectOne(() => + actor.Tell(new SiteEnvelope("unknown-site", new object()))); + } + + [Fact] + public void FactoryThrow_SkipsSite_DoesNotCrashActor() + { + var throwingFactory = Substitute.For(); + throwingFactory.Create(Arg.Any(), Arg.Any(), Arg.Any>()) + .Returns(_ => throw new InvalidOperationException("boom")); + var actor = Sys.ActorOf(Props.Create(() => new CentralCommunicationActor( + EmptyProvider(), throwingFactory, null))); + + EventFilter.Error(contains: "Failed to create ClusterClient").ExpectOne(() => + actor.Tell(Load("site-a", "akka.tcp://scadabridge@node-a:8081"))); + + // Actor survived — a subsequent envelope for that (unrouted) site still + // produces the healthy "No ClusterClient" warning rather than a dead actor. + EventFilter.Warning(contains: "No ClusterClient for site").ExpectOne(() => + actor.Tell(new SiteEnvelope("site-a", new object()))); + } +}