fix(dotnet): broken error name deserialization (#3855)

Fixes a bug where the `name` field of an `ErrorResponse` would not deserialize into our enum of known names correctly. Initially it was thought that if the `name` field was non-null, it would only be one of our known values, but the kernel is in fact not setting this field explicitly in all cases of "user land" errors. This results in the `name` field being `"Error"`in the default case and would cause a JSON deserialization error.

Implements a custom deserializer for the `ErrorName` to explicitly return null if the string is not defined or unknown. Additionally makes the `name` field nullable.

Fixes aws/aws-cdk#22369

Co-authored-by: 🧑🏻‍💻 Romain Marcadier <rmuller@amazon.com>
Co-authored-by: Romain Marcadier <rmuller@amazon.fr>

gh-pages/content/specification/6-compliance-report.md

@@ -5,7 +5,7 @@
 This section details the current state of each language binding with respect to our standard compliance suite.
 
 
-| number | test                                                                                                                                                             | java (98.35%) | golang (79.34%)                              | Dotnet | Python |
+| number | test                                                                                                                                                             | java (98.36%) | golang (79.51%)                              | Dotnet | Python |
 | ------ | ---------------------------------------------------------------------------------------------------------------------------------------------------------------- | ------------- | -------------------------------------------- | ------ | ------ |
 | 1      | asyncOverrides_overrideCallsSuper                                                                                                                                | 🟢            | [🔴](https://github.com/aws/jsii/issues/2670) | ⭕      | ⭕      |
 | 2      | [arrayReturnedByMethodCanBeRead]("Array created in the kernel can be queried for its elements")                                                                  | 🟢            | 🟢                                           | ⭕      | ⭕      |
@@ -128,3 +128,4 @@ This section details the current state of each language binding with respect to
 | 119    | [classCanBeUsedWhenNotExpressedlyLoaded]("Validates that types not explicitly loaded by the user can safely be returned by JS code")                             | 🟢            | 🟢                                           | ⭕      | ⭕      |
 | 120    | [downcasting]("Ensures unsafe-cast features work as expected")                                                                                                   | ⭕             | 🟢                                           | ⭕      | ⭕      |
 | 121    | [strippedDeprecatedMemberCanBeReceived]("Ensures --strip-deprecated does not cause odd runtime errors")                                                          | 🟢            | 🟢                                           | ⭕      | ⭕      |
+| 122    | [exceptionMessage]("Verifies that custom exception names are correctly forwarded")                                                                               | 🟢            | 🟢                                           | ⭕      | ⭕      |

packages/@jsii/dotnet-runtime-test/test/Amazon.JSII.Runtime.IntegrationTests/ComplianceTests.cs

@@ -4,6 +4,7 @@
 using Amazon.JSII.Runtime.Deputy;
 using Amazon.JSII.Tests.CalculatorNamespace;
 using Amazon.JSII.Tests.CalculatorNamespace.Cdk16625;
+using Amazon.JSII.Tests.CalculatorNamespace.Cdk22369;
 using CompositeOperation = Amazon.JSII.Tests.CalculatorNamespace.Composition.CompositeOperation;
 using Amazon.JSII.Tests.CalculatorNamespace.LibNamespace;
 using Amazon.JSII.Tests.CalculatorNamespace.BaseOfBaseNamespace;
@@ -378,13 +379,22 @@ public void Exceptions()
             calc.Add(3);
             Assert.Equal(23d, calc.Value);
 
-            Assert.Throws<JsiiError>(() => calc.Add(10));
+            Assert.Throws<Exception>(() => calc.Add(10));
 
             calc.MaxValue = 40;
             calc.Add(10);
             Assert.Equal(33d, calc.Value);
         }
 
+        [Fact(DisplayName = Prefix + nameof(ExceptionMessage))]
+        public void ExceptionMessage()
+        {
+            var e = Assert.Throws<Exception>(() =>
+                new AcceptsPath(new AcceptsPathProps { SourcePath = "A Bad Path" })
+            );
+            Assert.Equal("Cannot find asset", e.Message);
+        }
+
         [Fact(DisplayName = Prefix + nameof(UnionProperties))]
         public void UnionProperties()
         {
@@ -491,7 +501,7 @@ public void AsyncOverrides_OverrideThrows()
         {
             AsyncVirtualMethodsChild obj = new AsyncVirtualMethodsChild();
 
-            JsiiError exception = Assert.Throws<JsiiError>(() => obj.CallMe());
+            var exception = Assert.Throws<Exception>(() => obj.CallMe());
             Assert.Contains("Thrown by native code", exception.Message);
         }
 
@@ -558,7 +568,7 @@ public void PropertyOverrides_Get_Throws()
         {
             SyncVirtualMethodsChild_Throws so = new SyncVirtualMethodsChild_Throws();
 
-            JsiiError exception = Assert.Throws<JsiiError>(() => so.RetrieveValueOfTheProperty());
+            var exception = Assert.Throws<Exception>(() => so.RetrieveValueOfTheProperty());
             Assert.Contains("Oh no, this is bad", exception.Message);
         }
 
@@ -576,7 +586,7 @@ public void PropertyOverrides_Set_Throws()
         {
             SyncVirtualMethodsChild_Throws so = new SyncVirtualMethodsChild_Throws();
 
-            JsiiError exception = Assert.Throws<JsiiError>(() => so.ModifyValueOfTheProperty("Hii"));
+            var exception = Assert.Throws<Exception>(() => so.ModifyValueOfTheProperty("Hii"));
             Assert.Contains("Exception from overloaded setter", exception.Message);
         }
 

packages/@jsii/dotnet-runtime/src/Amazon.JSII.Runtime.UnitTests/Client/ClientTests.cs

@@ -24,7 +24,7 @@ public sealed class ClientTests
         public abstract class ClientTestBase
         {
             private readonly IFileSystem _fileSystem;
-            
+
             internal readonly IRuntime _runtime;
             private readonly IReferenceMap _referenceMap;
             private readonly IFrameworkToJsiiConverter _frameworkToJsiiConverter;
@@ -498,7 +498,7 @@ public void SendsAndReceives()
                 CompleteRequest request = new CompleteRequest
                 (
                     callbackId: "myCallbackId",
-                    error: "myError",
+                    error: new NamedError("myError", null),
                     result: "myResult"
                 );
                 client.Complete(request);

packages/@jsii/dotnet-runtime/src/Amazon.JSII.Runtime/CallbackExtensions.cs

@@ -17,7 +17,7 @@ namespace Amazon.JSII.Runtime
     internal static class CallbackExtensions
     {
         public static object? InvokeCallback(this Callback callback, IReferenceMap referenceMap,
-            IFrameworkToJsiiConverter converter, out string? error)
+            IFrameworkToJsiiConverter converter, out Exception? error)
         {
             try
             {
@@ -36,13 +36,13 @@ out object? result
             catch (TargetInvocationException e)
             {
                 // An exception was thrown by the method being invoked
-                error = e.InnerException?.ToString();
+                error = e.InnerException;
                 return null;
             }
             catch (Exception e)
             {
                 // An exception was thrown while preparing the request or processing the result
-                error = e.ToString();
+                error = e;
                 return null;
             }
         }

packages/@jsii/dotnet-runtime/src/Amazon.JSII.Runtime/Deputy/DeputyBase.cs

@@ -328,8 +328,8 @@ private static void InvokeCallbacks()
                 foreach (var callback in callbacks.Callbacks)
                 {
                     var result = callback.InvokeCallback(referenceMap, converter, out var error);
-
-                    client.Complete(callback.CallbackId, error, result);
+                    var namedError = error is null ? null : new NamedError(error);
+                    client.Complete(callback.CallbackId, namedError, result);
                 }
 
                 callbacks = client.Callbacks();

packages/@jsii/dotnet-runtime/src/Amazon.JSII.Runtime/JsonModel/Api/NamedError.cs

@@ -0,0 +1,73 @@
+using Amazon.JSII.Runtime;
+using Newtonsoft.Json;
+using System;
+
+namespace Amazon.JSII.JsonModel.Api
+{
+    internal sealed class NamedError
+    {
+        internal NamedError(Exception exception): this(exception.ToString(), exception is JsiiError ? ErrorName.JsiiError : (ErrorName?)null) {}
+
+        internal NamedError(string message, ErrorName? name)
+        {
+            Message = message;
+            Name = name;
+        }
+
+        public string Message { get;  }
+        public ErrorName? Name { get;}
+    }
+
+    [JsonConverter(typeof(ErrorNameConverter))]
+    public enum ErrorName
+    {
+        JsiiError,
+        RuntimeException,
+    }
+
+    internal sealed class ErrorNameConverter : JsonConverter
+    {
+        private const string ErrorNameFault = "@jsii/kernel.Fault";
+        private const string ErrorNameRuntime = "@jsii/kernel.RuntimeError";
+
+        public override object? ReadJson(JsonReader reader, Type objectType, object? existingValue, JsonSerializer serializer)
+        {
+            string? enumString = (string?)reader.Value;
+            ErrorName? errorResponseName = null;
+            switch (enumString) {
+                case ErrorNameFault:
+                {
+                    errorResponseName = ErrorName.JsiiError;
+                    break;
+                }
+                case ErrorNameRuntime:
+                {
+                    errorResponseName = ErrorName.RuntimeException;
+                    break;
+                }
+            }
+
+            return errorResponseName;
+        }
+
+        public override void WriteJson(JsonWriter writer, object? value, JsonSerializer serializer)
+        {
+            switch (value)
+            {
+                case ErrorName.JsiiError:
+                    writer.WriteValue(ErrorNameFault);
+                    break;
+                case ErrorName.RuntimeException:
+                    writer.WriteValue(ErrorNameRuntime);
+                    break;
+                default: writer.WriteNull();
+                    break;
+            }
+        }
+
+        public override bool CanConvert(Type objectType)
+        {
+            return objectType == typeof(string);
+        }
+    }
+}

packages/@jsii/dotnet-runtime/src/Amazon.JSII.Runtime/JsonModel/Api/Request/CompleteRequest.cs

@@ -1,15 +1,32 @@
-using Newtonsoft.Json;
+using Amazon.JSII.JsonModel.Api;
+using Newtonsoft.Json;
 using System;
+using Amazon.JSII.JsonModel.Api.Response;
 
 namespace Amazon.JSII.JsonModel.Api.Request
 {
     [JsonObject(MemberSerialization = MemberSerialization.OptIn)]
     public sealed class CompleteRequest : IKernelRequest
     {
+        [Obsolete("Use one of the constructors that expose the errorName property")]
         public CompleteRequest(string callbackId, string? error = null, object? result = null)
+        : this(callbackId, error, null, result)
+        {
+        }
+
+        private CompleteRequest(string callbackId, string? error = null, ErrorName? errorName = null, object? result = null)
         {
             CallbackId = callbackId ?? throw new ArgumentNullException(nameof(callbackId));
             Error = error;
+            ErrorName = errorName;
+            Result = result;
+        }
+
+        internal CompleteRequest(string callbackId, NamedError? error = null, object? result = null)
+        {
+            CallbackId = callbackId ?? throw new ArgumentNullException(nameof(callbackId));
+            Error = error?.Message;
+            ErrorName = error?.Name;
             Result = result;
         }
 
@@ -21,6 +38,9 @@ public CompleteRequest(string callbackId, string? error = null, object? result =
 
         [JsonProperty("err", NullValueHandling = NullValueHandling.Ignore)]
         public string? Error { get; }
+
+        [JsonProperty("name", NullValueHandling = NullValueHandling.Ignore)]
+        internal ErrorName? ErrorName { get; }
 
         [JsonProperty("result", NullValueHandling = NullValueHandling.Ignore)]
         public object? Result { get; }

packages/@jsii/dotnet-runtime/src/Amazon.JSII.Runtime/JsonModel/Api/Response/ErrorResponse.cs

@@ -1,24 +1,16 @@
 using Newtonsoft.Json;
 using System;
-using System.Runtime.Serialization;
 
 namespace Amazon.JSII.JsonModel.Api.Response
 {
-    public enum ErrorResponseName
-    {
-        [EnumMember(Value = "@jsii/kernel.Fault")]
-        JsiiError,
-        [EnumMember(Value = "@jsii/kernel.RuntimeError")]
-        RuntimeException,
-    }
-
     [JsonObject(MemberSerialization = MemberSerialization.OptIn)]
     public sealed class ErrorResponse
     {
-        public ErrorResponse(string error, string? stack = null)
+        public ErrorResponse(string error, string? stack = null, ErrorName? name = null)
         {
             Error = error ?? throw new ArgumentNullException(nameof(error));
             Stack = stack;
+            Name = name;
         }
 
         [JsonProperty("error")]
@@ -28,6 +20,6 @@ public ErrorResponse(string error, string? stack = null)
         public string? Stack { get; }
 
         [JsonProperty("name", NullValueHandling = NullValueHandling.Ignore)]
-        public ErrorResponseName Name { get ; }
+        public ErrorName? Name { get; }
     }
-}
+}

packages/@jsii/dotnet-runtime/src/Amazon.JSII.Runtime/Services/Client.cs

@@ -109,7 +109,7 @@ TResponse TryDeserialize<TResponse>(string responseJson) where TResponse : class
             {
                 ErrorResponse errorResponse = responseObject.ToObject<ErrorResponse>()!;
 
-                if (errorResponse.Name.Equals(ErrorResponseName.JsiiError))
+                if (ErrorName.JsiiError.Equals(errorResponse.Name))
                 {
                     throw new JsiiError(errorResponse, null);
                 }
@@ -127,11 +127,12 @@ TResponse TryDeserialize<TResponse>(string responseJson) where TResponse : class
                 CallbackResponse callbackResponse = responseObject.ToObject<CallbackResponse>()!;
                 Callback callback = callbackResponse.Callback;
 
-                object? result = callback.InvokeCallback(_referenceMap, _frameworkToJsiiConverter, out string? error);
+                object? result = callback.InvokeCallback(_referenceMap, _frameworkToJsiiConverter, out Exception? error);
+                var namedError = error is null ? null : new NamedError(error);
 
                 return Send<SynchronousCompleteRequest, TResponse>(new SynchronousCompleteRequest
                 (
-                    new CompleteRequest(callback.CallbackId, error, result)
+                    new CompleteRequest(callback.CallbackId, namedError, result)
                 ));
             }
 
@@ -293,7 +294,7 @@ public CallbacksResponse Callbacks(CallbacksRequest request)
             return Send<CallbacksRequest, CallbacksResponse>(request);
         }
 
-        public CompleteResponse Complete(string callbackId, string? error = null, object? result = null)
+        public CompleteResponse Complete(string callbackId, NamedError? error = null, object? result = null)
         {
             return Complete(new CompleteRequest(callbackId, error, result));
         }

packages/@jsii/dotnet-runtime/src/Amazon.JSII.Runtime/Services/IClient.cs

@@ -58,7 +58,7 @@ internal interface IClient
 
         CallbacksResponse Callbacks(CallbacksRequest request);
 
-        CompleteResponse Complete(string callbackId, string? error = null, object? result = null);
+        CompleteResponse Complete(string callbackId, NamedError? error = null, object? result = null);
 
         CompleteResponse Complete(CompleteRequest request);
 

packages/@jsii/go-runtime-test/project/compliance_test.go

@@ -20,6 +20,7 @@ import (
 	"github.com/aws/jsii/go-runtime-test/internal/wallClock"
 	"github.com/aws/jsii/jsii-calc/go/jcb"
 	calc "github.com/aws/jsii/jsii-calc/go/jsiicalc/v3"
+	"github.com/aws/jsii/jsii-calc/go/jsiicalc/v3/cdk22369"
 	"github.com/aws/jsii/jsii-calc/go/jsiicalc/v3/composition"
 	"github.com/aws/jsii/jsii-calc/go/jsiicalc/v3/submodule/child"
 	calclib "github.com/aws/jsii/jsii-calc/go/scopejsiicalclib"
@@ -1660,6 +1661,18 @@ func (suite *ComplianceSuite) TestStrippedDeprecatedMemberCanBeReceived() {
 	require.NotNil(deprecationremoval.InterfaceFactory_Create())
 }
 
+func (suite *ComplianceSuite) TestExceptionMessage() {
+	require := suite.Require()
+
+	defer func() {
+		err := recover()
+		require.NotNil(err, "expected a failure to occur")
+		require.Contains(err.(error).Error(), "Cannot find asset")
+	}()
+
+	cdk22369.NewAcceptsPath(&cdk22369.AcceptsPathProps{SourcePath: jsii.String("A Bad Path")})
+}
+
 // required to make `go test` recognize the suite.
 func TestComplianceSuite(t *testing.T) {
 	suite.Run(t, new(ComplianceSuite))

packages/@jsii/java-runtime-test/project/src/test/java/software/amazon/jsii/testing/ComplianceTest.java

@@ -13,6 +13,7 @@
 import software.amazon.jsii.tests.calculator.*;
 import software.amazon.jsii.tests.calculator.baseofbase.StaticConsumer;
 import software.amazon.jsii.tests.calculator.cdk16625.Cdk16625;
+import software.amazon.jsii.tests.calculator.cdk22369.AcceptsPath;
 import software.amazon.jsii.tests.calculator.composition.CompositeOperation;
 import software.amazon.jsii.tests.calculator.custom_submodule_name.NestingClass.NestedStruct;
 import software.amazon.jsii.tests.calculator.lib.deprecation_removal.InterfaceFactory;
@@ -1831,4 +1832,18 @@ protected java.lang.Number unwrap(final IRandomNumberGenerator rng) {
     public void strippedDeprecatedMemberCanBeReceived() {
         assertNotNull(InterfaceFactory.create());
     }
+
+    @Test
+    public void exceptionMessage() {
+        boolean thrown = false;
+        try {
+            AcceptsPath.Builder.create()
+                .sourcePath("A Bad Path")
+                .build();
+        } catch (final RuntimeException e) {
+            assertTrue(e.getMessage().startsWith("Cannot find asset"));
+            thrown = true;
+        }
+        assertTrue(thrown);
+    }
 }

packages/@jsii/kernel/src/api.ts

@@ -1,3 +1,5 @@
+import { JsiiErrorType } from './kernel';
+
 export const TOKEN_REF = '$jsii.byref';
 export const TOKEN_INTERFACES = '$jsii.interfaces';
 export const TOKEN_DATE = '$jsii.date';
@@ -290,4 +292,5 @@ export interface OkayResponse {
 export interface ErrorResponse {
   readonly error: string;
   readonly stack?: string;
+  readonly name?: JsiiErrorType;
 }