Merge pull request #402 from AArnott/fix400

Report argument deserialization failures more precisely

src/StreamJsonRpc.Tests/Exceptions/RemoteInvocationExceptionTests.cs

@@ -0,0 +1,55 @@
+// Copyright (c) Microsoft Corporation. All rights reserved.
+// Licensed under the MIT license. See LICENSE file in the project root for full license information.
+
+using System;
+using StreamJsonRpc;
+using StreamJsonRpc.Protocol;
+using Xunit;
+using Xunit.Abstractions;
+
+public class RemoteInvocationExceptionTests : TestBase
+{
+    private const string SomeMessage = "test message";
+    private static readonly Exception SomeInnerException = new Exception();
+
+    public RemoteInvocationExceptionTests(ITestOutputHelper logger)
+        : base(logger)
+    {
+    }
+
+    [Fact]
+    public void Ctor_Message_Code_Data()
+    {
+        var data = new CommonErrorData();
+        var ex = new RemoteInvocationException(SomeMessage, 123, data);
+        Assert.Equal(SomeMessage, ex.Message);
+        Assert.Equal(123, ex.ErrorCode);
+        Assert.Same(data, ex.ErrorData);
+        Assert.Null(ex.DeserializedErrorData);
+    }
+
+    [Fact]
+    public void Ctor_Message_Code_Data_DeserializedData()
+    {
+        var data = new CommonErrorData();
+        var deserializedData = new CommonErrorData();
+        var ex = new RemoteInvocationException(SomeMessage, 123, data, deserializedData);
+        Assert.Equal(SomeMessage, ex.Message);
+        Assert.Equal(123, ex.ErrorCode);
+        Assert.Same(data, ex.ErrorData);
+        Assert.Same(deserializedData, ex.DeserializedErrorData);
+    }
+
+    [Fact]
+    public void Serializable()
+    {
+        var data = new CommonErrorData();
+        var deserializedData = new CommonErrorData();
+        var original = new RemoteInvocationException(SomeMessage, 123, data, deserializedData);
+        var deserialized = BinaryFormatterRoundtrip(original);
+        Assert.Equal(original.Message, deserialized.Message);
+        Assert.Equal(original.ErrorCode, deserialized.ErrorCode);
+        Assert.Null(deserialized.ErrorData);
+        Assert.Null(deserialized.DeserializedErrorData);
+    }
+}

src/StreamJsonRpc.Tests/Exceptions/RpcArgumentDeserializationExceptionTests.cs

@@ -0,0 +1,63 @@
+// Copyright (c) Microsoft Corporation. All rights reserved.
+// Licensed under the MIT license. See LICENSE file in the project root for full license information.
+
+using System;
+using StreamJsonRpc;
+using Xunit;
+using Xunit.Abstractions;
+
+public class RpcArgumentDeserializationExceptionTests : TestBase
+{
+    private const string SomeMessage = "test message";
+    private static readonly Exception SomeInnerException = new Exception();
+
+    public RpcArgumentDeserializationExceptionTests(ITestOutputHelper logger)
+        : base(logger)
+    {
+    }
+
+    [Fact]
+    public void Ctor_Message()
+    {
+        var ex = new RpcArgumentDeserializationException(SomeMessage);
+        Assert.Equal(SomeMessage, ex.Message);
+    }
+
+    [Fact]
+    public void Ctor_Message_Exception()
+    {
+        var ex = new RpcArgumentDeserializationException(SomeMessage, SomeInnerException);
+        Assert.Equal(SomeMessage, ex.Message);
+        Assert.Same(SomeInnerException, ex.InnerException);
+    }
+
+    [Fact]
+    public void Ctor_WithDetails()
+    {
+        var ex = new RpcArgumentDeserializationException("argName", 67856, typeof(RpcArgumentDeserializationExceptionTests), SomeInnerException);
+        Assert.Equal("argName", ex.ArgumentName);
+        Assert.Equal(67856, ex.ArgumentPosition);
+        Assert.Equal(typeof(RpcArgumentDeserializationExceptionTests), ex.DeserializedType);
+        Assert.Same(SomeInnerException, ex.InnerException);
+    }
+
+    [Fact]
+    public void Serializable_WithPosition()
+    {
+        var original = new RpcArgumentDeserializationException("argName", 67856, typeof(RpcArgumentDeserializationExceptionTests), SomeInnerException);
+        var deserialized = BinaryFormatterRoundtrip(original);
+        Assert.Equal(original.Message, deserialized.Message);
+        Assert.Equal(original.ArgumentName, deserialized.ArgumentName);
+        Assert.Equal(original.ArgumentPosition, deserialized.ArgumentPosition);
+    }
+
+    [Fact]
+    public void Serializable_WithNoPosition()
+    {
+        var original = new RpcArgumentDeserializationException("argName", argumentPosition: null, typeof(RpcArgumentDeserializationExceptionTests), SomeInnerException);
+        var deserialized = BinaryFormatterRoundtrip(original);
+        Assert.Equal(original.Message, deserialized.Message);
+        Assert.Equal(original.ArgumentName, deserialized.ArgumentName);
+        Assert.Equal(original.ArgumentPosition, deserialized.ArgumentPosition);
+    }
+}

src/StreamJsonRpc.Tests/JsonRpcJsonHeadersTests.cs

@@ -240,7 +240,7 @@ private class TypeThrowsWhenDeserializedConverter : JsonConverter<TypeThrowsWhen
     {
         public override TypeThrowsWhenDeserialized ReadJson(JsonReader reader, Type objectType, TypeThrowsWhenDeserialized existingValue, bool hasExistingValue, JsonSerializer serializer)
         {
-            throw new Exception("This exception is meant to be thrown.");
+            throw CreateExceptionToBeThrownByDeserializer();
         }
 
         public override void WriteJson(JsonWriter writer, TypeThrowsWhenDeserialized value, JsonSerializer serializer)

src/StreamJsonRpc.Tests/JsonRpcMessagePackLengthTests.cs

@@ -86,7 +86,7 @@ private class TypeThrowsWhenDeserializedFormatter : IMessagePackFormatter<TypeTh
     {
         public TypeThrowsWhenDeserialized Deserialize(ref MessagePackReader reader, MessagePackSerializerOptions options)
         {
-            throw new Exception("This exception is meant to be thrown.");
+            throw CreateExceptionToBeThrownByDeserializer();
         }
 
         public void Serialize(ref MessagePackWriter writer, TypeThrowsWhenDeserialized value, MessagePackSerializerOptions options)

src/StreamJsonRpc.Tests/JsonRpcTests.cs

@@ -1641,6 +1641,16 @@ public async Task ReturnTypeThrowsOnDeserialization()
         await Assert.ThrowsAnyAsync<Exception>(() => this.clientRpc.InvokeWithCancellationAsync<TypeThrowsWhenDeserialized>(nameof(Server.GetTypeThrowsWhenDeserialized), cancellationToken: this.TimeoutToken)).WithCancellation(this.TimeoutToken);
     }
 
+    [Fact]
+    public async Task MethodArgThrowsOnDeserialization()
+    {
+        var ex = await Assert.ThrowsAsync<RemoteMethodNotFoundException>(() => this.clientRpc.InvokeWithCancellationAsync(nameof(Server.MethodWithArgThatFailsToDeserialize), new object[] { new TypeThrowsWhenDeserialized() }, this.TimeoutToken)).WithCancellation(this.TimeoutToken);
+        var expectedErrorMessage = CreateExceptionToBeThrownByDeserializer().Message;
+        Assert.Equal(JsonRpcErrorCode.InvalidParams, ex.ErrorCode);
+        var data = Assert.IsType<CommonErrorData>(ex.DeserializedErrorData);
+        Assert.Contains(FlattenCommonErrorData(data), d => d.Message?.Contains(expectedErrorMessage) ?? false);
+    }
+
     [Fact]
     public async Task CanPassExceptionFromServer_DeserializedErrorData()
     {
@@ -1652,6 +1662,8 @@ public async Task CanPassExceptionFromServer_DeserializedErrorData()
         Assert.StrictEqual(COR_E_UNAUTHORIZEDACCESS, errorData.HResult);
     }
 
+    protected static Exception CreateExceptionToBeThrownByDeserializer() => new Exception("This exception is meant to be thrown.");
+
     protected override void Dispose(bool disposing)
     {
         if (disposing)
@@ -1692,6 +1704,15 @@ private static void SendObject(Stream receivingStream, object jsonObject)
         receivingStream.Write(buffer, 0, buffer.Length);
     }
 
+    private static IEnumerable<CommonErrorData> FlattenCommonErrorData(CommonErrorData? errorData)
+    {
+        while (errorData is object)
+        {
+            yield return errorData;
+            errorData = errorData.Inner;
+        }
+    }
+
     private void ReinitializeRpcWithoutListening()
     {
         var streams = Nerdbank.FullDuplexStream.CreateStreams();
@@ -1886,6 +1907,10 @@ public int InstanceMethodWithSingleObjectParameterAndCancellationToken(XAndYFiel
 
         public TypeThrowsWhenDeserialized GetTypeThrowsWhenDeserialized() => new TypeThrowsWhenDeserialized();
 
+        public void MethodWithArgThatFailsToDeserialize(TypeThrowsWhenDeserialized arg1)
+        {
+        }
+
         public int? MethodReturnsNullableInt(int a) => a > 0 ? (int?)a : null;
 
         public int MethodAcceptsNullableArgs(int? a, int? b) => (a.HasValue ? 1 : 0) + (b.HasValue ? 1 : 0);

src/StreamJsonRpc.Tests/TestBase.cs

@@ -3,7 +3,9 @@
 
 using System;
 using System.Diagnostics;
+using System.IO;
 using System.Linq;
+using System.Runtime.Serialization.Formatters.Binary;
 using System.Threading;
 using System.Threading.Tasks;
 using Microsoft.VisualStudio.Threading;
@@ -66,6 +68,15 @@ protected static Task WhenAllSucceedOrAnyFault(params Task[] tasks)
         return resultTcs.Task;
     }
 
+    protected static T BinaryFormatterRoundtrip<T>(T original)
+    {
+        var bf = new BinaryFormatter();
+        var ms = new MemoryStream();
+        bf.Serialize(ms, original);
+        ms.Position = 0;
+        return (T)bf.Deserialize(ms);
+    }
+
     protected virtual void Dispose(bool disposing)
     {
         if (disposing)

src/StreamJsonRpc/Exceptions/RemoteInvocationException.cs

@@ -5,6 +5,7 @@ namespace StreamJsonRpc
 {
     using System;
     using System.IO;
+    using System.Runtime.Serialization;
     using System.Text;
     using Newtonsoft.Json.Linq;
     using StreamJsonRpc.Protocol;
@@ -15,7 +16,7 @@ namespace StreamJsonRpc
     /// <remarks>
     /// The details of the target method exception can be found on the <see cref="ErrorCode"/> and <see cref="ErrorData"/> properties.
     /// </remarks>
-    [System.Serializable]
+    [Serializable]
     public class RemoteInvocationException : RemoteRpcException
     {
         /// <summary>
@@ -51,11 +52,10 @@ public RemoteInvocationException(string? message, int errorCode, object? errorDa
         /// </summary>
         /// <param name="info">Serialization info.</param>
         /// <param name="context">Streaming context.</param>
-        protected RemoteInvocationException(
-          System.Runtime.Serialization.SerializationInfo info,
-          System.Runtime.Serialization.StreamingContext context)
+        protected RemoteInvocationException(SerializationInfo info, StreamingContext context)
             : base(info, context)
         {
+            this.ErrorCode = info.GetInt32(nameof(this.ErrorCode));
         }
 
         /// <summary>
@@ -110,6 +110,13 @@ public override string ToString()
             return result;
         }
 
+        /// <inheritdoc/>
+        public override void GetObjectData(SerializationInfo info, StreamingContext context)
+        {
+            base.GetObjectData(info, context);
+            info.AddValue(nameof(this.ErrorCode), this.ErrorCode);
+        }
+
         private static void ContributeInnerExceptionDetails(StringBuilder builder, CommonErrorData errorData)
         {
             builder.AppendLine($" ---> {errorData.TypeName}: {errorData.Message}");

src/StreamJsonRpc/Exceptions/RemoteMethodNotFoundException.cs

@@ -6,6 +6,8 @@ namespace StreamJsonRpc
     using System;
     using System.Runtime.Serialization;
     using Microsoft;
+    using Newtonsoft.Json.Linq;
+    using StreamJsonRpc.Protocol;
 
     /// <summary>
     /// Remote RPC exception that indicates that the requested target method was not found on the server.
@@ -24,11 +26,17 @@ public class RemoteMethodNotFoundException : RemoteRpcException
         /// </summary>
         /// <param name="message">Exception message describing why the method was not found.</param>
         /// <param name="targetMethod">Target method that was not found.</param>
-        internal RemoteMethodNotFoundException(string? message, string targetMethod)
+        /// <param name="errorCode">The value of the error.code field in the response.</param>
+        /// <param name="errorData">The value of the error.data field in the response.</param>
+        /// <param name="deserializedErrorData">The value of the error.data field in the response, deserialized according to <see cref="JsonRpc.GetErrorDetailsDataType(JsonRpcError)"/>.</param>
+        internal RemoteMethodNotFoundException(string? message, string targetMethod, JsonRpcErrorCode errorCode, object? errorData, object? deserializedErrorData)
             : base(message)
         {
             Requires.NotNullOrEmpty(targetMethod, nameof(targetMethod));
+            this.ErrorCode = errorCode;
             this.TargetMethod = targetMethod;
+            this.ErrorData = errorData;
+            this.DeserializedErrorData = deserializedErrorData;
         }
 
         /// <summary>
@@ -47,6 +55,34 @@ protected RemoteMethodNotFoundException(SerializationInfo info, StreamingContext
         /// </summary>
         public string TargetMethod { get; }
 
+        /// <summary>
+        /// Gets the value of the <c>error.code</c> field in the response.
+        /// </summary>
+        /// <value>
+        /// The value is typically either <see cref="JsonRpcErrorCode.InvalidParams"/> or <see cref="JsonRpcErrorCode.MethodNotFound"/>.
+        /// </value>
+        public JsonRpcErrorCode ErrorCode { get; }
+
+        /// <summary>
+        /// Gets the <c>error.data</c> value in the error response, if one was provided.
+        /// </summary>
+        /// <remarks>
+        /// Depending on the <see cref="IJsonRpcMessageFormatter"/> used, the value of this property, if any,
+        /// may be a <see cref="JToken"/> or a deserialized object.
+        /// If a deserialized object, the type of this object is determined by <see cref="JsonRpc.GetErrorDetailsDataType(JsonRpcError)"/>.
+        /// The default implementation of this method produces a <see cref="CommonErrorData"/> object.
+        /// </remarks>
+        public object? ErrorData { get; }
+
+        /// <summary>
+        /// Gets the <c>error.data</c> value in the error response, if one was provided.
+        /// </summary>
+        /// <remarks>
+        /// The type of this object is determined by <see cref="JsonRpc.GetErrorDetailsDataType(JsonRpcError)"/>.
+        /// The default implementation of this method produces a <see cref="CommonErrorData"/> object.
+        /// </remarks>
+        public object? DeserializedErrorData { get; }
+
         /// <inheritdoc/>
         public override void GetObjectData(SerializationInfo info, StreamingContext context)
         {