Fixed server-side exception handling

CoreRemoting.Tests/RpcTests.cs

@@ -1,6 +1,8 @@
 using System;
 using System.Diagnostics;
 using System.Threading;
+using System.Threading.Tasks;
+using CoreRemoting.Serialization;
 using CoreRemoting.Tests.ExternalTypes;
 using CoreRemoting.Tests.Tools;
 using Xunit;
@@ -405,5 +407,107 @@ public void Missing_service_throws_RemoteInvocationException()
             Assert.NotNull(ex);
             Assert.Contains("IDisposable", ex.Message);
         }
+
+        [Fact]
+        public void Error_method_throws_Exception()
+        {
+            try
+            {
+                using var client = new RemotingClient(new ClientConfig()
+                {
+                    ConnectionTimeout = 5,
+                    InvocationTimeout = 5,
+                    SendTimeout = 5,
+                    MessageEncryption = false,
+                    ServerPort = _serverFixture.Server.Config.NetworkPort
+                });
+
+                client.Connect();
+
+                var proxy = client.CreateProxy<ITestService>();
+                var ex = Assert.Throws<RemoteInvocationException>(() =>
+                    proxy.Error(nameof(Error_method_throws_Exception)))
+                        .GetInnermostException();
+
+                Assert.NotNull(ex);
+                Assert.Equal(nameof(Error_method_throws_Exception), ex.Message);
+            }
+            finally
+            {
+                // reset the error counter for other tests
+                _serverFixture.ServerErrorCount = 0;
+            }
+        }
+
+        [Fact]
+        public async Task ErrorAsync_method_throws_Exception()
+        {
+            try
+            {
+                using var client = new RemotingClient(new ClientConfig()
+                {
+                    ConnectionTimeout = 5,
+                    InvocationTimeout = 5,
+                    SendTimeout = 5,
+                    MessageEncryption = false,
+                    ServerPort = _serverFixture.Server.Config.NetworkPort
+                });
+
+                client.Connect();
+
+                var proxy = client.CreateProxy<ITestService>();
+                var ex = (await Assert.ThrowsAsync<RemoteInvocationException>(async () =>
+                    await proxy.ErrorAsync(nameof(ErrorAsync_method_throws_Exception))))
+                        .GetInnermostException();
+
+                Assert.NotNull(ex); 
+                Assert.Equal(nameof(ErrorAsync_method_throws_Exception), ex.Message);
+            }
+            finally
+            {
+                // reset the error counter for other tests
+                _serverFixture.ServerErrorCount = 0;
+            }
+        }
+
+        [Fact]
+        public void NonSerializableError_method_throws_Exception()
+        {
+            try
+            {
+                using var client = new RemotingClient(new ClientConfig()
+                {
+                    ConnectionTimeout = 5,
+                    InvocationTimeout = 5,
+                    SendTimeout = 5,
+                    MessageEncryption = false,
+                    ServerPort = _serverFixture.Server.Config.NetworkPort
+                });
+
+                client.Connect();
+
+                var proxy = client.CreateProxy<ITestService>();
+                var ex = Assert.Throws<RemoteInvocationException>(() =>
+                    proxy.NonSerializableError("Hello", "Serializable", "World"))
+                        .GetInnermostException();
+
+                Assert.NotNull(ex);
+                Assert.IsType<SerializableException>(ex);
+
+                if (ex is SerializableException sx)
+                {
+                    Assert.Equal("NonSerializable", sx.SourceTypeName);
+                    Assert.Equal("Hello", ex.Message);
+                    Assert.Equal("Serializable", ex.Data["Serializable"]);
+                    Assert.Equal("World", ex.Data["World"]);
+                    Assert.NotNull(ex.StackTrace);
+                }
+            }
+            finally
+            {
+                // reset the error counter for other tests
+                _serverFixture.ServerErrorCount = 0;
+            }
+        }
     }
 }

CoreRemoting.Tests/Tools/ITestService.cs

@@ -1,4 +1,5 @@
 using System;
+using System.Threading.Tasks;
 using CoreRemoting.Tests.ExternalTypes;
 
 namespace CoreRemoting.Tests.Tools
@@ -28,5 +29,11 @@ public interface ITestService : IBaseService
         string Echo(string text);
 
         void MethodWithOutParameter(out int counter);
+
+        void Error(string text);
+
+        Task ErrorAsync(string text);
+
+        void NonSerializableError(string text, params object[] data);
     }
 }

CoreRemoting.Tests/Tools/TestService.cs

@@ -1,4 +1,5 @@
 using System;
+using System.Threading.Tasks;
 using CoreRemoting.Tests.ExternalTypes;
 
 namespace CoreRemoting.Tests.Tools
@@ -62,5 +63,34 @@ public bool BaseMethod()
         {
             return true;
         }
+
+        public void Error(string text)
+        {
+            throw new Exception(text);
+        }
+
+        public async Task ErrorAsync(string text)
+        {
+            await Task.Delay(1);
+            Error(text);
+        }
+
+        private class NonSerializable : Exception
+        {
+            public NonSerializable(string message)
+                : base(message)
+            {
+            }
+        }
+
+        public void NonSerializableError(string text, params object[] data)
+        {
+            var ex = new NonSerializable(text);
+
+            foreach (var item in data)
+                ex.Data[item] = item;
+
+            throw ex;
+        }
     }
 }

CoreRemoting/CallContext.cs

@@ -12,7 +12,7 @@ public static class CallContext
     {
         private static readonly ConcurrentDictionary<string, AsyncLocal<object>> State = 
             new ConcurrentDictionary<string, AsyncLocal<object>>();
-        
+
         /// <summary>
         /// Stores a given object and associates it with the specified name.
         /// </summary>
@@ -67,7 +67,7 @@ public static void RestoreFromSnapshot(IEnumerable<CallContextEntry> entries)
                 }
                 return;
             }
-            
+
             foreach (var entry in entries)
             {
                 CallContext.SetData(entry.Name, entry.Value);

CoreRemoting/ClientConfig.cs

@@ -18,17 +18,17 @@ public ClientConfig()
         {
             UniqueClientInstanceName = Guid.NewGuid().ToString();
         }
-        
+
         /// <summary>
         /// Gets or sets the unique name of the configured client instance.
         /// </summary>
         public string UniqueClientInstanceName { get; set; }
-        
+
         /// <summary>
         /// Gets or sets the connection timeout in seconds (0 means infinite).
         /// </summary>
         public int ConnectionTimeout { get; set; } = 120;
-        
+
         /// <summary>
         /// Gets or sets the authentication timeout in seconds (0 means infinite).
         /// </summary>
@@ -43,7 +43,7 @@ public ClientConfig()
         /// Gets or sets the send timeout in seconds (0 means infinite).
         /// </summary>
         public int SendTimeout { get; set; } = 30;
-        
+
         /// <summary>
         /// Gets or sets the host name of the CoreRemoting server to be connected to.
         /// </summary>
@@ -53,7 +53,7 @@ public ClientConfig()
         /// Gets or sets the network port of the CoreRemoting server to be connected to.
         /// </summary>
         public int ServerPort { get; set; } = 9090;
-        
+
         /// <summary>
         /// Gets or sets the serializer to be used (Bson serializer is used, if set to null).
         /// </summary>
@@ -68,13 +68,13 @@ public ClientConfig()
         /// Gets or sets whether messages should be encrypted or not.
         /// </summary>
         public bool MessageEncryption { get; set; } = true;
-        
+
         /// <summary>
         /// Gets or sets the client channel to be used for transport of messages over the wire (WebsocketClientChannel is used, if set to null).
         /// </summary>
         [SuppressMessage("ReSharper", "UnusedAutoPropertyAccessor.Global")]
         public IClientChannel Channel { get; set; }
-        
+
         /// <summary>
         /// Gets or sets an array of credentials for authentication (depends on the authentication provider used on server side). 
         /// </summary>

CoreRemoting/ClientRpcContext.cs

@@ -41,7 +41,7 @@ internal ClientRpcContext()
         /// <summary>
         /// Gets a wait handle that is set, when the response of this RPC call is received from server.
         /// </summary>
-        public EventWaitHandle WaitHandle { get; }
+        public EventWaitHandle WaitHandle { get; } // TODO: replace with a Task?
 
         /// <summary>
         /// Frees managed resources.

CoreRemoting/RemotingClient.cs

@@ -645,7 +645,7 @@ internal Task<ClientRpcContext> InvokeRemoteMethod(MethodCallMessage methodCallM
                     lock (_syncObject)
                     {
                         if (_activeCalls == null)
-                            throw new RemoteInvocationException("ServerDisconnected");                        
+                            throw new RemoteInvocationException("ServerDisconnected");
                     }
 
                     var clientRpcContext = new ClientRpcContext();
@@ -686,7 +686,8 @@ internal Task<ClientRpcContext> InvokeRemoteMethod(MethodCallMessage methodCallM
 
                     if (oneWay || clientRpcContext.ResultMessage != null) 
                         return clientRpcContext;
-
+
+                    // TODO: replace with a Task to avoid freezing the current thread?
                     if (_config.InvocationTimeout <= 0)
                         clientRpcContext.WaitHandle.WaitOne();
                     else

CoreRemoting/RemotingSession.cs

@@ -506,7 +506,7 @@ private void ProcessRpcMessage(WireMessage request)
                     serverRpcContext.Exception =
                         new RemoteInvocationException(
                             message: ex.Message,
-                            innerEx: ex.GetType().IsSerializable ? ex : null);
+                            innerEx: ex.ToSerializable());
 
                     ((RemotingServer)_server).OnAfterCall(serverRpcContext);
 
@@ -539,8 +539,12 @@ private void ProcessRpcMessage(WireMessage request)
                 if (oneWay)
                     return;
 
-                serializedResult =
-                    _server.Serializer.Serialize(serverRpcContext.MethodCallResultMessage);
+                // don't overwrite the serialized exception
+                if (ReferenceEquals(serializedResult, Array.Empty<byte>()))
+                {
+                    serializedResult =
+                        _server.Serializer.Serialize(serverRpcContext.MethodCallResultMessage);
+                }
             }
 
             var methodResultMessage =

CoreRemoting/Serialization/ExceptionExtensions.cs

@@ -0,0 +1,65 @@
+using System;
+using System.Linq;
+
+namespace CoreRemoting.Serialization;
+
+/// <summary>
+/// Extension methods for the exception classes.
+/// </summary>
+public static class ExceptionExtensions
+{
+    /// <summary>
+    /// Checks whether the exception is serializable.
+    /// </summary>
+    public static bool IsSerializable(this Exception ex) => ex switch
+    {
+        null => true,
+
+        AggregateException agg =>
+            agg.InnerExceptions.All(ix => ix.IsSerializable()) &&
+                agg.InnerException.IsSerializable() &&
+                agg.GetType().IsSerializable,
+
+        _ => ex.GetType().IsSerializable &&
+            ex.InnerException.IsSerializable()
+    };
+
+    /// <summary>
+    /// Converts the non-serializable exception to a serializable copy.
+    /// </summary>
+    public static Exception ToSerializable(this Exception ex) =>
+        ex.IsSerializable() ? ex :
+            new SerializableException(ex.GetType().Name, ex.Message,
+                ex.InnerException.ToSerializable(), ex.StackTrace)
+                    .CopyDataFrom(ex);
+
+    /// <summary>
+    /// Copies all exception data slots from the original exception.
+    /// </summary>
+    /// <typeparam name="TException">Exception type.</typeparam>
+    /// <param name="ex">Target exception.</param>
+    /// <param name="original">Original exception.</param>
+    /// <returns>Modified target exception.</returns>
+    public static TException CopyDataFrom<TException>(this TException ex, Exception original)
+        where TException : Exception
+    {
+        if (ex == null || original == null)
+            return ex;
+
+        foreach (var key in original.Data.Keys)
+            ex.Data[key] = original.Data[key];
+
+        return ex;
+    }
+
+    /// <summary>
+    /// Returns the most inner exception.
+    /// </summary>
+    public static Exception GetInnermostException(this Exception ex)
+    {
+        while (ex?.InnerException != null)
+            ex = ex.InnerException;
+
+        return ex;
+    }
+}