Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remoting and an exception as a payload message #3903

Closed
Zetanova opened this issue Sep 4, 2019 · 20 comments · Fixed by #6300
Closed

Remoting and an exception as a payload message #3903

Zetanova opened this issue Sep 4, 2019 · 20 comments · Fixed by #6300

Comments

@Zetanova
Copy link
Contributor

Zetanova commented Sep 4, 2019

Akka.Remoting and Akka.Cluster implement a transparent IPC system.
The supervisor system is relying on exception types as it's message protocol.
This works fine in a single system but with combination of remote-deploy
it leads to problems.

Because not all exceptions and there fields are serializable
and/or the exception-type is known to every cluster node,
so the exception as a message can't be always transmitted.
And it is not known for the sender or receiver what Exception-Types
will be generated or can be handled.

To resolve this conflict, i see following options:

Option A

To transform every exception handled by the supervisor-strategy
to an well-known generic "ErrorMessage" and/or "ReasonMessage"
and add an Extension method to transform an ErrorMessage
back to an exception in user code.
This acts as a substitution, the exception-reference itself
can be held internally, but will not be serialized.

override void PostRestart(Akka.ErrorReason error) 
{
  //Reason contains a short message as string and a ErrorId as a guid
   Akka.Reason reason = error.Reason;   

  //The error instance can contain the exception instance or it can TRY to lazy deserialize it.  
  bool hasException = error.TryGetException(Exception ex);

  if(hasException)
      PostRestart(ex);
  else
     PostRestart(new AkkaReasonException(reason)); //generic error
}

With this default implementation the PostRestart(Exception error) is still be called
and nothing should breaks.

Option B

Akka.Remoting is treating an exception as a special-message.
Then on the receiving side, it will only try to deserialize it back.
If it fails, then it will generate a GenericException instead.

@Zetanova Zetanova changed the title Remoting the a exception as a payload message Remoting and an exception as a payload message Sep 4, 2019
@Zetanova
Copy link
Contributor Author

Related: #3811

@Aaronontheweb
Copy link
Member

Agree that this is a real issue that needs addressing. Slated it for the v1.4.0 release.

@Aaronontheweb Aaronontheweb modified the milestones: 1.4.0, 1.4.1 and Later Feb 13, 2020
@Aaronontheweb Aaronontheweb modified the milestones: 1.4.2, 1.4.3 Mar 13, 2020
@Robbert-Driven-It
Copy link

So I stumbled on this issue because of a problem I encountered when trying to return a Status.Failure with an exception from a remote actor. After some investigation I found that Newtonsoft is quite capable of serializing and deserializing an exception on it's own...but somewhere in Akka some value properties on an Exception are treated as an object.
Regular serialization
"RemoteStackIndex":0,
"HResult":-2146233088,

Akka produces
"RemoteStackIndex":{
"$":"I0"
},
"HResult":{
"$":"I-2146233088"
},

I don't recognize the json produced by Akka, is this some special value type setting that is used by Akka when sending and not understood when the receiving side tries to deserialize it?

@Robbert-Driven-It
Copy link

private static object TranslateSurrogate(object deserializedValue, NewtonSoftJsonSerializer parent, Type type)

Looks like this bit of code is causing havoc on exception (de)serialization. Any way to bypass it for exceptions?

@Aaronontheweb Aaronontheweb modified the milestones: 1.4.3, 1.4.4 Mar 18, 2020
@Aaronontheweb Aaronontheweb modified the milestones: 1.4.4, 1.4.5 Mar 31, 2020
@Aaronontheweb Aaronontheweb modified the milestones: 1.4.5, 1.4.6 Apr 29, 2020
@Aaronontheweb Aaronontheweb modified the milestones: 1.4.6, 1.4.7 May 12, 2020
@Aaronontheweb Aaronontheweb modified the milestones: 1.4.7, 1.4.8 May 26, 2020
@Aaronontheweb Aaronontheweb modified the milestones: 1.4.8, 1.4.9 Jun 17, 2020
@Aaronontheweb Aaronontheweb modified the milestones: 1.4.9, 1.4.10 Jul 21, 2020
@Aaronontheweb Aaronontheweb removed this from the 1.4.10 milestone Aug 20, 2020
@Aaronontheweb Aaronontheweb modified the milestones: 1.4.34, 1.4.35 Mar 7, 2022
@Aaronontheweb Aaronontheweb modified the milestones: 1.4.35, 1.4.36 Mar 18, 2022
@Aaronontheweb Aaronontheweb modified the milestones: 1.4.36, 1.4.40 Jun 7, 2022
@Aaronontheweb Aaronontheweb modified the milestones: 1.4.40, 1.4.41 Jul 27, 2022
@Aaronontheweb Aaronontheweb modified the milestones: 1.4.41, 1.4.42 Sep 7, 2022
@Aaronontheweb Aaronontheweb modified the milestones: 1.4.42, 1.4.43, 1.4.44 Sep 23, 2022
@Aaronontheweb Aaronontheweb modified the milestones: 1.4.44, 1.4.45, 1.4.46 Oct 17, 2022
@Aaronontheweb Aaronontheweb modified the milestones: 1.4.46, 1.4.47 Nov 15, 2022
@Aaronontheweb
Copy link
Member

Looking into this per @Zetanova 's comments on #6294 (comment)

It looks like we already have remote Exception serialization support for remotely deployed actors via ExceptionData and the ExceptionSupport class in Akka.Remote:

internal class ExceptionSupport
{
private readonly WrappedPayloadSupport _wrappedPayloadSupport;
private const BindingFlags All = BindingFlags.Instance | BindingFlags.NonPublic | BindingFlags.Public;
private readonly HashSet<string> _defaultProperties = new HashSet<string>
{
"ClassName",
"Message",
"StackTraceString",
"Source",
"InnerException",
"HelpURL",
"RemoteStackTraceString",
"RemoteStackIndex",
"ExceptionMethod",
"HResult",
"Data",
"TargetSite",
"HelpLink",
"StackTrace",
"WatsonBuckets"
};
public ExceptionSupport(ExtendedActorSystem system)
{
_wrappedPayloadSupport = new WrappedPayloadSupport(system);
}
public byte[] SerializeException(Exception exception)
{
return ExceptionToProto(exception).ToByteArray();
}
internal Proto.Msg.ExceptionData ExceptionToProto(Exception exception)
{
return ExceptionToProtoNet(exception);
}
public Exception DeserializeException(byte[] bytes)
{
var proto = Proto.Msg.ExceptionData.Parser.ParseFrom(bytes);
return ExceptionFromProto(proto);
}
internal Exception ExceptionFromProto(Proto.Msg.ExceptionData proto)
{
return ExceptionFromProtoNet(proto);
}
private readonly FormatterConverter _defaultFormatterConverter = new FormatterConverter();
public Proto.Msg.ExceptionData ExceptionToProtoNet(Exception exception)
{
var message = new Proto.Msg.ExceptionData();
if (exception == null)
return message;
var exceptionType = exception.GetType();
message.TypeName = exceptionType.TypeQualifiedName();
message.Message = exception.Message;
message.StackTrace = exception.StackTrace ?? "";
message.Source = exception.Source ?? "";
message.InnerException = ExceptionToProto(exception.InnerException);
var serializable = exception as ISerializable;
var serializationInfo = new SerializationInfo(exceptionType, _defaultFormatterConverter);
serializable.GetObjectData(serializationInfo, new StreamingContext());
foreach (var info in serializationInfo)
{
if (_defaultProperties.Contains(info.Name)) continue;
if (info.Value is Exception exceptionValue)
{
var exceptionPayload = ExceptionToProto(exceptionValue);
var preparedValue = _wrappedPayloadSupport.PayloadToProto(exceptionPayload);
message.CustomFields.Add(info.Name, preparedValue);
} else
{
var preparedValue = _wrappedPayloadSupport.PayloadToProto(info.Value);
message.CustomFields.Add(info.Name, preparedValue);
}
}
return message;
}
public Exception ExceptionFromProtoNet(Proto.Msg.ExceptionData proto)
{
if (string.IsNullOrEmpty(proto.TypeName))
return null;
Type exceptionType = Type.GetType(proto.TypeName);
var serializationInfo = new SerializationInfo(exceptionType, _defaultFormatterConverter);
serializationInfo.AddValue("ClassName", proto.TypeName);
serializationInfo.AddValue("Message", ValueOrNull(proto.Message));
serializationInfo.AddValue("StackTraceString", ValueOrNull(proto.StackTrace));
serializationInfo.AddValue("Source", ValueOrNull(proto.Source));
serializationInfo.AddValue("InnerException", ExceptionFromProto(proto.InnerException));
serializationInfo.AddValue("HelpURL", string.Empty);
serializationInfo.AddValue("RemoteStackTraceString", null);
serializationInfo.AddValue("RemoteStackIndex", 0);
serializationInfo.AddValue("ExceptionMethod", null);
serializationInfo.AddValue("HResult", int.MinValue);
foreach (var field in proto.CustomFields)
{
var payload = _wrappedPayloadSupport.PayloadFrom(field.Value);
if (payload is ExceptionData exception)
payload = ExceptionFromProto(exception);
serializationInfo.AddValue(field.Key, payload);
}
Exception obj = null;
ConstructorInfo constructorInfo = exceptionType.GetConstructor(
All,
null,
new[] { typeof(SerializationInfo), typeof(StreamingContext) },
null);
if (constructorInfo != null)
{
object[] args = { serializationInfo, new StreamingContext() };
obj = constructorInfo.Invoke(args).AsInstanceOf<Exception>();
}
return obj;
}
private string ValueOrNull(string value)
=> string.IsNullOrEmpty(value) ? null : value;
}

I'll check the test suite to see if we have tests covering this scenario for remote deployments. We'll also add support for Status.Failed and Status.Succeeded provided that those are feasible.

Aaronontheweb added a commit to Aaronontheweb/akka.net that referenced this issue Dec 8, 2022
Aaronontheweb added a commit that referenced this issue Dec 8, 2022
* added end to end spec to validate error serialization for remote actor supervision

* defined `.proto`s for `Status.Failure` and `Status.Success`

* added `Status.,Failure` and `Status.Success` support to the `MiscMessageSerializer`

* added tests to `MiscMessageSerializerSpec`

* close #3903
Arkatufus pushed a commit to Arkatufus/akka.net that referenced this issue Dec 9, 2022
)

* added end to end spec to validate error serialization for remote actor supervision

* defined `.proto`s for `Status.Failure` and `Status.Success`

* added `Status.,Failure` and `Status.Success` support to the `MiscMessageSerializer`

* added tests to `MiscMessageSerializerSpec`

* close akkadotnet#3903

(cherry picked from commit 998dcca)
Aaronontheweb added a commit that referenced this issue Dec 9, 2022
* added end to end spec to validate error serialization for remote actor supervision

* defined `.proto`s for `Status.Failure` and `Status.Success`

* added `Status.,Failure` and `Status.Success` support to the `MiscMessageSerializer`

* added tests to `MiscMessageSerializerSpec`

* close #3903

(cherry picked from commit 998dcca)

Co-authored-by: Aaron Stannard <aaron@petabridge.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants