Skip to content

Commit

Permalink
feat(dotnet): distinguish error types (#3764)
Browse files Browse the repository at this point in the history
Adds a new class `JsiiError` that subclasses `JsiiException` to help
distinguish jsii kernel errors that are likely unrecoverable from
`RuntimeErrors` which may be errors expected and handled within the JS
process and may be caught and handled in the host language runtime.

See #3747 for more information

---

By submitting this pull request, I confirm that my contribution is made under the terms of the [Apache 2.0 license].

[Apache 2.0 license]: https://www.apache.org/licenses/LICENSE-2.0
  • Loading branch information
MrArnoldPalmer authored Sep 24, 2022
1 parent 0cf5008 commit 283dae5
Show file tree
Hide file tree
Showing 12 changed files with 64 additions and 24 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -378,7 +378,7 @@ public void Exceptions()
calc.Add(3);
Assert.Equal(23d, calc.Value);

Assert.Throws<JsiiException>(() => calc.Add(10));
Assert.Throws<JsiiError>(() => calc.Add(10));

calc.MaxValue = 40;
calc.Add(10);
Expand Down Expand Up @@ -491,7 +491,7 @@ public void AsyncOverrides_OverrideThrows()
{
AsyncVirtualMethodsChild obj = new AsyncVirtualMethodsChild();

JsiiException exception = Assert.Throws<JsiiException>(() => obj.CallMe());
JsiiError exception = Assert.Throws<JsiiError>(() => obj.CallMe());
Assert.Contains("Thrown by native code", exception.Message);
}

Expand Down Expand Up @@ -558,7 +558,7 @@ public void PropertyOverrides_Get_Throws()
{
SyncVirtualMethodsChild_Throws so = new SyncVirtualMethodsChild_Throws();

JsiiException exception = Assert.Throws<JsiiException>(() => so.RetrieveValueOfTheProperty());
JsiiError exception = Assert.Throws<JsiiError>(() => so.RetrieveValueOfTheProperty());
Assert.Contains("Oh no, this is bad", exception.Message);
}

Expand All @@ -576,7 +576,7 @@ public void PropertyOverrides_Set_Throws()
{
SyncVirtualMethodsChild_Throws so = new SyncVirtualMethodsChild_Throws();

JsiiException exception = Assert.Throws<JsiiException>(() => so.ModifyValueOfTheProperty("Hii"));
JsiiError exception = Assert.Throws<JsiiError>(() => so.ModifyValueOfTheProperty("Hii"));
Assert.Contains("Exception from overloaded setter", exception.Message);
}

Expand Down Expand Up @@ -624,7 +624,7 @@ public void SyncOverrides_CallsDoubleAsyncMethodFails()
SyncOverrides obj = new SyncOverrides();
obj.CallAsync = true;

Assert.Throws<JsiiException>(() => obj.CallerIsMethod());
Assert.Throws<JsiiError>(() => obj.CallerIsMethod());
}

[Fact(DisplayName = Prefix + nameof(SyncOverrides_CallsDoubleAsyncPropertyGetterFails))]
Expand All @@ -633,7 +633,7 @@ public void SyncOverrides_CallsDoubleAsyncPropertyGetterFails()
SyncOverrides obj = new SyncOverrides();
obj.CallAsync = true;

Assert.Throws<JsiiException>(() => obj.CallerIsProperty);
Assert.Throws<JsiiError>(() => obj.CallerIsProperty);
}

[Fact(DisplayName = Prefix + nameof(SyncOverrides_CallsDoubleAsyncPropertySetterFails))]
Expand All @@ -642,7 +642,7 @@ public void SyncOverrides_CallsDoubleAsyncPropertySetterFails()
SyncOverrides obj = new SyncOverrides();
obj.CallAsync = true;

Assert.Throws<JsiiException>(() => obj.CallerIsProperty = 12);
Assert.Throws<JsiiError>(() => obj.CallerIsProperty = 12);
}

[Fact(DisplayName = Prefix + nameof(TestInterfaces))]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,12 +24,12 @@ public RuntimeTests()
_sut = new Services.Runtime(_nodeProcessMock);
}

[Fact(DisplayName = Prefix + nameof(ThrowsJsiiExceptionWhenResponseNotReceived))]
public void ThrowsJsiiExceptionWhenResponseNotReceived()
[Fact(DisplayName = Prefix + nameof(ThrowsJsiiErrorWhenResponseNotReceived))]
public void ThrowsJsiiErrorWhenResponseNotReceived()
{
_nodeProcessMock.StandardOutput.ReadLine().ReturnsNull();

var ex = Assert.Throws<JsiiException>(() => _sut.ReadResponse());
var ex = Assert.Throws<JsiiError>(() => _sut.ReadResponse());
Assert.Equal("Child process exited unexpectedly!", ex.Message);
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ out object? result
if (!converter.TryConvert(attribute.Parameters[paramIndex], requiredType, referenceMap,
request.Arguments![n], out var value))
{
throw new JsiiException($"Unable to convert {request.Arguments![n]} to {requiredType.Name}");
throw new JsiiError($"Unable to convert {request.Arguments![n]} to {requiredType.Name}");
}

if (attribute.Parameters[paramIndex].IsVariadic)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -576,7 +576,7 @@ private bool MakeProxy(Type interfaceType, bool force, [NotNullWhen(true)] out o
);
if (constructorInfo == null)
{
throw new JsiiException($"Could not find constructor to instantiate {proxyType.FullName}");
throw new JsiiError($"Could not find constructor to instantiate {proxyType.FullName}");
}

result = constructorInfo.Invoke(new object[]{ Reference.ForProxy() });
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ internal static void Load(Assembly assembly)
.FirstOrDefault(name => name.EndsWith(".tgz", StringComparison.InvariantCultureIgnoreCase));
if (tarballResourceName == null)
{
throw new JsiiException("Cannot find embedded tarball resource in assembly " + assembly.GetName(), null);
throw new JsiiError("Cannot find embedded tarball resource in assembly " + assembly.GetName(), null);
}

IServiceProvider serviceProvider = ServiceContainer.ServiceProvider;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,10 @@

namespace Amazon.JSII.Runtime
{
public sealed class JsiiException : Exception
public abstract class JsiiException : Exception
{
public ErrorResponse? ErrorResponse { get; }
public Type? ExceptionType { get; }

internal JsiiException() : base()
{
Expand All @@ -26,4 +27,26 @@ internal JsiiException(ErrorResponse response, Exception? innerException)
ErrorResponse = response;
}
}

public sealed class JsiiError : JsiiException
{
internal JsiiError() : base()
{
}

internal JsiiError(string message) : base(message)
{
}

internal JsiiError(string message, Exception? innerException)
: base(message, innerException)
{
}

internal JsiiError(ErrorResponse response, Exception? innerException)
: base(response, innerException)
{
}

}
}
Original file line number Diff line number Diff line change
@@ -1,8 +1,17 @@
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
{
Expand All @@ -17,5 +26,8 @@ public ErrorResponse(string error, string? stack = null)

[JsonProperty("stack", NullValueHandling = NullValueHandling.Ignore)]
public string? Stack { get; }

[JsonProperty("name", NullValueHandling = NullValueHandling.Ignore)]
public ErrorResponseName Name { get ; }
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ void SendRequest<TRequest>(TRequest requestObject)
}
catch (IOException exception)
{
throw new JsiiException("Unexpected error communicating with jsii-runtime", exception);
throw new JsiiError("Unexpected error communicating with jsii-runtime", exception);
}
}

Expand All @@ -94,7 +94,7 @@ TResponse ReceiveResponse<TResponse>()
}
catch (IOException exception)
{
throw new JsiiException("Unexpected error communicating with jsii-runtime", exception);
throw new JsiiError("Unexpected error communicating with jsii-runtime", exception);
}
}

Expand All @@ -109,7 +109,12 @@ TResponse TryDeserialize<TResponse>(string responseJson) where TResponse : class
{
ErrorResponse errorResponse = responseObject.ToObject<ErrorResponse>()!;

throw new JsiiException(errorResponse, null);
if (errorResponse.Name.Equals(ErrorResponseName.JsiiError))
{
throw new JsiiError(errorResponse, null);
}

throw new Exception(errorResponse.Error);
}

if (typeof(TResponse).IsAssignableFrom(typeof(HelloResponse)))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,25 +15,25 @@ internal interface ITypeCache
System.Type GetClassType(string fullyQualifiedName)
{
return TryGetClassType(fullyQualifiedName)
?? throw new JsiiException($"Unable to find class for jsii FQN \"{fullyQualifiedName}\"");
?? throw new JsiiError($"Unable to find class for jsii FQN \"{fullyQualifiedName}\"");
}

System.Type GetEnumType(string fullyQualifiedName)
{
return TryGetEnumType(fullyQualifiedName)
?? throw new JsiiException($"Unable to find enum for jsii FQN \"{fullyQualifiedName}\"");
?? throw new JsiiError($"Unable to find enum for jsii FQN \"{fullyQualifiedName}\"");
}

System.Type GetInterfaceType(string fullyQualifiedName)
{
return TryGetInterfaceType(fullyQualifiedName)
?? throw new JsiiException($"Unable to find interface for jsii FQN \"{fullyQualifiedName}\"");
?? throw new JsiiError($"Unable to find interface for jsii FQN \"{fullyQualifiedName}\"");
}

System.Type GetProxyType(string fullyQualifiedName)
{
return TryGetProxyType(fullyQualifiedName)
?? throw new JsiiException($"Unable to find proxy type for jsii FQN \"{fullyQualifiedName}\"");
?? throw new JsiiError($"Unable to find proxy type for jsii FQN \"{fullyQualifiedName}\"");
}

System.Type GetFrameworkType(TypeReference reference, bool isOptional);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ ConstructorInfo GetByRefConstructor()
ConstructorInfo? constructorInfo = type.GetConstructor(constructorFlags, null, new[] {typeof(ByRefValue)}, null);
if (constructorInfo == null)
{
throw new JsiiException($"Could not find constructor to initialize {type.FullName} with a {typeof(ByRefValue).FullName}");
throw new JsiiError($"Could not find constructor to initialize {type.FullName} with a {typeof(ByRefValue).FullName}");
}
return constructorInfo;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ public string ExtractResource(Assembly assembly, string resourceName, string bag
using var stream = assembly.GetManifestResourceStream(resourceName);
if (stream == null)
{
throw new JsiiException($"Cannot find embedded resource: {resourceName} in {String.Join(", ", assembly.GetManifestResourceNames())}", null);
throw new JsiiError($"Cannot find embedded resource: {resourceName} in {String.Join(", ", assembly.GetManifestResourceNames())}", null);
}

stream.CopyTo(output);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ public string ReadResponse()
var response = _nodeProcess.StandardOutput.ReadLine();
if (string.IsNullOrEmpty(response))
{
throw new JsiiException("Child process exited unexpectedly!");
throw new JsiiError("Child process exited unexpectedly!");
}

return response;
Expand Down

0 comments on commit 283dae5

Please sign in to comment.