From 264855c0f2283652aaf862f9775f5e1add90fdf5 Mon Sep 17 00:00:00 2001 From: Gregorius Soedharmo Date: Fri, 4 Jun 2021 09:12:20 +0700 Subject: [PATCH 1/5] Fix exception bubbling and messages --- .../Akka.Remote.Tests.csproj | 1 + .../Serialization/BugFix5062Spec.cs | 97 +++++++++++++++++++ src/core/Akka/Serialization/Serialization.cs | 5 + 3 files changed, 103 insertions(+) create mode 100644 src/core/Akka.Remote.Tests/Serialization/BugFix5062Spec.cs diff --git a/src/core/Akka.Remote.Tests/Akka.Remote.Tests.csproj b/src/core/Akka.Remote.Tests/Akka.Remote.Tests.csproj index 2f5b6dd7a88..62923042aa3 100644 --- a/src/core/Akka.Remote.Tests/Akka.Remote.Tests.csproj +++ b/src/core/Akka.Remote.Tests/Akka.Remote.Tests.csproj @@ -8,6 +8,7 @@ + diff --git a/src/core/Akka.Remote.Tests/Serialization/BugFix5062Spec.cs b/src/core/Akka.Remote.Tests/Serialization/BugFix5062Spec.cs new file mode 100644 index 00000000000..abd537c9101 --- /dev/null +++ b/src/core/Akka.Remote.Tests/Serialization/BugFix5062Spec.cs @@ -0,0 +1,97 @@ +using System; +using System.Collections.Generic; +using System.Linq; +using System.Reflection; +using System.Runtime.Serialization; +using System.Text; +using System.Threading.Tasks; +using Akka.Actor; +using Akka.Cluster; +using Akka.Configuration; +using Akka.DistributedData; +using Akka.Remote.Configuration; +using Akka.Serialization; +using Akka.TestKit; +using FluentAssertions; +using Google.Protobuf; +using Xunit; +using Xunit.Abstractions; + +namespace Akka.Remote.Tests.Serialization +{ + public class BugFix5062Spec: AkkaSpec + { + private static Config DDataConfig => ConfigurationFactory.ParseString($@" + akka.actor {{ + serializers {{ + dummyWithManifest = ""{typeof(DummySerializerWithStringManifest).AssemblyQualifiedName}"" + }} + serialization-bindings {{ + ""{typeof(SomeMessage).AssemblyQualifiedName}"" = dummyWithManifest + }} + serialization-identifiers {{ + ""{typeof(DummySerializerWithStringManifest).AssemblyQualifiedName}"" = 13 + }} + }}") + .WithFallback(RemoteConfigFactory.Default()) + .WithFallback(DistributedData.DistributedData.DefaultConfig()); + + public BugFix5062Spec(ITestOutputHelper output) : base(output, DDataConfig) + { } + + [Fact] + public void Failed_serialization_should_give_proper_exception_message() + { + var node1 = new UniqueAddress(new Address("akka.tcp", "Sys", "localhost", 2551), 1); + var payload = new LWWRegister(node1, new SomeMessage()); + + var message = new ActorSelectionMessage( + payload, + new SelectionPathElement[] { new SelectChildName("dummy") }, + true); + + var serialized = MessageSerializer.Serialize((ExtendedActorSystem)Sys, node1.Address, message); + + var o = new object(); + var ex = o.Invoking(s => MessageSerializer.Deserialize((ExtendedActorSystem)Sys, serialized)).Should() + .Throw() + .WithMessage("Failed to deserialize object with serialization id [6] (manifest []).") + .WithInnerExceptionExactly() + .WithMessage("Failed to deserialize object with serialization id [11] (manifest [E]).") + .WithInnerExceptionExactly() + .WithMessage("Exception has been thrown by the target of an invocation.") + .WithInnerExceptionExactly() + .WithMessage("Failed to deserialize object with serialization id [13] (manifest [SM])."); + } + + public class SomeMessage + { + } + + public class DummySerializerWithStringManifest : SerializerWithStringManifest + { + public DummySerializerWithStringManifest(ExtendedActorSystem system) : base(system) + { + } + + public override byte[] ToBinary(object obj) + { + return Array.Empty(); + } + + public override object FromBinary(byte[] bytes, string manifest) + { + throw new NotImplementedException(); + } + + public override string Manifest(object o) + { + if (o is SomeMessage) + return "SM"; + throw new Exception("Unknown object type"); + } + } + + + } +} diff --git a/src/core/Akka/Serialization/Serialization.cs b/src/core/Akka/Serialization/Serialization.cs index 41a2d708d00..179ac35f265 100644 --- a/src/core/Akka/Serialization/Serialization.cs +++ b/src/core/Akka/Serialization/Serialization.cs @@ -429,6 +429,11 @@ public object Deserialize(byte[] bytes, int serializerId, string manifest) return serializer.FromBinary(bytes, type); } + catch (Exception e) + { + throw new SerializationException( + $"Failed to deserialize object with serialization id [{serializerId}] (manifest [{manifest}]).", e); + } finally { Serialization.CurrentTransportInformation = oldInfo; From ad373c8546ce725fd6867efd2ddbdd5b265408b0 Mon Sep 17 00:00:00 2001 From: Gregorius Soedharmo Date: Fri, 4 Jun 2021 22:18:24 +0700 Subject: [PATCH 2/5] Remove reference to DData project --- .../Akka.Remote.Tests/Akka.Remote.Tests.csproj | 1 - .../Serialization/BugFix5062Spec.cs | 16 +++++----------- 2 files changed, 5 insertions(+), 12 deletions(-) diff --git a/src/core/Akka.Remote.Tests/Akka.Remote.Tests.csproj b/src/core/Akka.Remote.Tests/Akka.Remote.Tests.csproj index 62923042aa3..2f5b6dd7a88 100644 --- a/src/core/Akka.Remote.Tests/Akka.Remote.Tests.csproj +++ b/src/core/Akka.Remote.Tests/Akka.Remote.Tests.csproj @@ -8,7 +8,6 @@ - diff --git a/src/core/Akka.Remote.Tests/Serialization/BugFix5062Spec.cs b/src/core/Akka.Remote.Tests/Serialization/BugFix5062Spec.cs index abd537c9101..1f2d00caf8c 100644 --- a/src/core/Akka.Remote.Tests/Serialization/BugFix5062Spec.cs +++ b/src/core/Akka.Remote.Tests/Serialization/BugFix5062Spec.cs @@ -8,7 +8,6 @@ using Akka.Actor; using Akka.Cluster; using Akka.Configuration; -using Akka.DistributedData; using Akka.Remote.Configuration; using Akka.Serialization; using Akka.TestKit; @@ -33,8 +32,7 @@ public class BugFix5062Spec: AkkaSpec ""{typeof(DummySerializerWithStringManifest).AssemblyQualifiedName}"" = 13 }} }}") - .WithFallback(RemoteConfigFactory.Default()) - .WithFallback(DistributedData.DistributedData.DefaultConfig()); + .WithFallback(RemoteConfigFactory.Default()); public BugFix5062Spec(ITestOutputHelper output) : base(output, DDataConfig) { } @@ -42,24 +40,20 @@ public BugFix5062Spec(ITestOutputHelper output) : base(output, DDataConfig) [Fact] public void Failed_serialization_should_give_proper_exception_message() { - var node1 = new UniqueAddress(new Address("akka.tcp", "Sys", "localhost", 2551), 1); - var payload = new LWWRegister(node1, new SomeMessage()); - var message = new ActorSelectionMessage( - payload, + new SomeMessage(), new SelectionPathElement[] { new SelectChildName("dummy") }, true); + var node1 = new UniqueAddress(new Address("akka.tcp", "Sys", "localhost", 2551), 1); var serialized = MessageSerializer.Serialize((ExtendedActorSystem)Sys, node1.Address, message); var o = new object(); var ex = o.Invoking(s => MessageSerializer.Deserialize((ExtendedActorSystem)Sys, serialized)).Should() .Throw() .WithMessage("Failed to deserialize object with serialization id [6] (manifest []).") - .WithInnerExceptionExactly() - .WithMessage("Failed to deserialize object with serialization id [11] (manifest [E]).") - .WithInnerExceptionExactly() - .WithMessage("Exception has been thrown by the target of an invocation.") + //.WithInnerExceptionExactly() + //.WithMessage("Failed to deserialize object with serialization id [11] (manifest [E]).") .WithInnerExceptionExactly() .WithMessage("Failed to deserialize object with serialization id [13] (manifest [SM])."); } From 2f46b345e00b6025149f90cc5af0b6f875ef2de9 Mon Sep 17 00:00:00 2001 From: Gregorius Soedharmo Date: Fri, 4 Jun 2021 22:30:22 +0700 Subject: [PATCH 3/5] Remove reference to Akka.Cluster --- src/core/Akka.Remote.Tests/Serialization/BugFix5062Spec.cs | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/core/Akka.Remote.Tests/Serialization/BugFix5062Spec.cs b/src/core/Akka.Remote.Tests/Serialization/BugFix5062Spec.cs index 1f2d00caf8c..fd43596696e 100644 --- a/src/core/Akka.Remote.Tests/Serialization/BugFix5062Spec.cs +++ b/src/core/Akka.Remote.Tests/Serialization/BugFix5062Spec.cs @@ -6,7 +6,6 @@ using System.Text; using System.Threading.Tasks; using Akka.Actor; -using Akka.Cluster; using Akka.Configuration; using Akka.Remote.Configuration; using Akka.Serialization; @@ -45,8 +44,8 @@ public void Failed_serialization_should_give_proper_exception_message() new SelectionPathElement[] { new SelectChildName("dummy") }, true); - var node1 = new UniqueAddress(new Address("akka.tcp", "Sys", "localhost", 2551), 1); - var serialized = MessageSerializer.Serialize((ExtendedActorSystem)Sys, node1.Address, message); + var node1 = new Address("akka.tcp", "Sys", "localhost", 2551); + var serialized = MessageSerializer.Serialize((ExtendedActorSystem)Sys, node1, message); var o = new object(); var ex = o.Invoking(s => MessageSerializer.Deserialize((ExtendedActorSystem)Sys, serialized)).Should() From e49799a5f5ceedcb9e908b90247205f5d7ba29ee Mon Sep 17 00:00:00 2001 From: Gregorius Soedharmo Date: Wed, 9 Jun 2021 22:16:21 +0700 Subject: [PATCH 4/5] Remove generic exception handling --- src/core/Akka/Serialization/Serialization.cs | 5 ----- 1 file changed, 5 deletions(-) diff --git a/src/core/Akka/Serialization/Serialization.cs b/src/core/Akka/Serialization/Serialization.cs index 179ac35f265..41a2d708d00 100644 --- a/src/core/Akka/Serialization/Serialization.cs +++ b/src/core/Akka/Serialization/Serialization.cs @@ -429,11 +429,6 @@ public object Deserialize(byte[] bytes, int serializerId, string manifest) return serializer.FromBinary(bytes, type); } - catch (Exception e) - { - throw new SerializationException( - $"Failed to deserialize object with serialization id [{serializerId}] (manifest [{manifest}]).", e); - } finally { Serialization.CurrentTransportInformation = oldInfo; From 9770008530062c2399ef85ed2587a0ad785e5c90 Mon Sep 17 00:00:00 2001 From: Gregorius Soedharmo Date: Wed, 9 Jun 2021 22:17:30 +0700 Subject: [PATCH 5/5] Add a very specific exception handling for failed payload deserialization --- .../Serialization/BugFix5062Spec.cs | 12 +++++------- .../Serialization/MessageContainerSerializer.cs | 14 +++++++++++++- 2 files changed, 18 insertions(+), 8 deletions(-) diff --git a/src/core/Akka.Remote.Tests/Serialization/BugFix5062Spec.cs b/src/core/Akka.Remote.Tests/Serialization/BugFix5062Spec.cs index fd43596696e..71d92f60064 100644 --- a/src/core/Akka.Remote.Tests/Serialization/BugFix5062Spec.cs +++ b/src/core/Akka.Remote.Tests/Serialization/BugFix5062Spec.cs @@ -39,22 +39,20 @@ public BugFix5062Spec(ITestOutputHelper output) : base(output, DDataConfig) [Fact] public void Failed_serialization_should_give_proper_exception_message() { + var childName = "dummy"; var message = new ActorSelectionMessage( new SomeMessage(), - new SelectionPathElement[] { new SelectChildName("dummy") }, + new SelectionPathElement[] { new SelectChildName(childName) }, true); var node1 = new Address("akka.tcp", "Sys", "localhost", 2551); var serialized = MessageSerializer.Serialize((ExtendedActorSystem)Sys, node1, message); var o = new object(); - var ex = o.Invoking(s => MessageSerializer.Deserialize((ExtendedActorSystem)Sys, serialized)).Should() + o.Invoking(s => MessageSerializer.Deserialize((ExtendedActorSystem)Sys, serialized)).Should() .Throw() - .WithMessage("Failed to deserialize object with serialization id [6] (manifest []).") - //.WithInnerExceptionExactly() - //.WithMessage("Failed to deserialize object with serialization id [11] (manifest [E]).") - .WithInnerExceptionExactly() - .WithMessage("Failed to deserialize object with serialization id [13] (manifest [SM])."); + .WithMessage($"Failed to deserialize payload object when deserializing {nameof(ActorSelectionMessage)} addressed to [{childName}]") + .WithInnerExceptionExactly(); } public class SomeMessage diff --git a/src/core/Akka.Remote/Serialization/MessageContainerSerializer.cs b/src/core/Akka.Remote/Serialization/MessageContainerSerializer.cs index 2c6ede2cfe9..6cb43393a61 100644 --- a/src/core/Akka.Remote/Serialization/MessageContainerSerializer.cs +++ b/src/core/Akka.Remote/Serialization/MessageContainerSerializer.cs @@ -7,7 +7,9 @@ using System; using System.Linq; +using System.Runtime.Serialization; using Akka.Actor; +using Akka.Remote.Serialization.Proto.Msg; using Akka.Serialization; using Akka.Util; using Google.Protobuf; @@ -71,7 +73,6 @@ public override byte[] ToBinary(object obj) public override object FromBinary(byte[] bytes, Type type) { var selectionEnvelope = Proto.Msg.SelectionEnvelope.Parser.ParseFrom(bytes); - var message = _payloadSupport.PayloadFrom(selectionEnvelope.Payload); var elements = new SelectionPathElement[selectionEnvelope.Pattern.Count]; for (var i = 0; i < selectionEnvelope.Pattern.Count; i++) @@ -85,6 +86,17 @@ public override object FromBinary(byte[] bytes, Type type) elements[i] = new SelectParent(); } + object message; + try + { + message = _payloadSupport.PayloadFrom(selectionEnvelope.Payload); + } + catch (Exception ex) + { + throw new SerializationException( + $"Failed to deserialize payload object when deserializing {nameof(ActorSelectionMessage)} addressed to [{string.Join(",", elements.Select(e => e.ToString()))}]", ex); + } + return new ActorSelectionMessage(message, elements); }