Skip to content

Commit

Permalink
fix(dotnet/runtime): Incorrect callback response format (#286)
Browse files Browse the repository at this point in the history
The response for a callback completion that resulted in an error should use the `err` field
name and not the `error` field name as was initially done. Additionally, the program flow
did not propagate all errors back to the `jsii-kernel`, causing false-pass on the compliance
test suite, as expectations were incorrectly set up to expect `RuntimeException` instead
of `JsiiException`.

Fixes #285
  • Loading branch information
RomainMuller authored Oct 30, 2018
1 parent 100f5ad commit 1b851e1
Show file tree
Hide file tree
Showing 5 changed files with 17 additions and 18 deletions.
8 changes: 4 additions & 4 deletions fetch-dotnet-snk.sh
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ function echo_usage() {
echo -e "\tDOTNET_STRONG_NAME_SECRET_ID=<The name (i.e. production/my/key) or ARN of the secret containing the .snk file.>"
}

if [ -z ${DOTNET_STRONG_NAME_ENABLED:-} ]; then
if [ -z "${DOTNET_STRONG_NAME_ENABLED:-}" ]; then
echo "Environment variable DOTNET_STRONG_NAME_ENABLED is not set. Skipping strong-name signing."
exit 0
fi
Expand All @@ -21,19 +21,19 @@ echo "Retrieving SNK..."
apt update -y
apt install jq -y

if [ -z ${DOTNET_STRONG_NAME_ROLE_ARN:-} ]; then
if [ -z "${DOTNET_STRONG_NAME_ROLE_ARN:-}" ]; then
echo "Strong name signing is enabled, but DOTNET_STRONG_NAME_ROLE_ARN is not set."
echo_usage
exit 1
fi

if [ -z ${DOTNET_STRONG_NAME_SECRET_REGION:-}]; then
if [ -z "${DOTNET_STRONG_NAME_SECRET_REGION:-}" ]; then
echo "Strong name signing is enabled, but DOTNET_STRONG_NAME_SECRET_REGION is not set."
echo_usage
exit 1
fi

if [ -z ${DOTNET_STRONG_NAME_SECRET_ID:-} ]; then
if [ -z "${DOTNET_STRONG_NAME_SECRET_ID:-}" ]; then
echo "Strong name signing is enabled, but DOTNET_STRONG_NAME_SECRET_ID is not set."
echo_usage
exit 1
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,10 +19,10 @@ public CompleteRequest(string callbackId, string error = null, object result = n
[JsonProperty("cbid")]
public string CallbackId { get; }

[JsonProperty("error", NullValueHandling = NullValueHandling.Ignore)]
[JsonProperty("err", NullValueHandling = NullValueHandling.Ignore)]
public string Error { get; }

[JsonProperty("result", NullValueHandling = NullValueHandling.Ignore)]
public object Result { get; }
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -462,8 +462,8 @@ public void AsyncOverrides_OverrideThrows()
{
AsyncVirtualMethodsChild obj = new AsyncVirtualMethodsChild();

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

class SyncVirtualMethodsChild_Set_CallsSuper : SyncVirtualMethods
Expand Down Expand Up @@ -531,8 +531,8 @@ public void PropertyOverrides_Get_Throws()
{
SyncVirtualMethodsChild_Throws so = new SyncVirtualMethodsChild_Throws();

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

[Fact(DisplayName = Prefix + nameof(PropertyOverrides_Set_CallsSuper))]
Expand All @@ -549,8 +549,8 @@ public void PropertyOverrides_Set_Throws()
{
SyncVirtualMethodsChild_Throws so = new SyncVirtualMethodsChild_Throws();

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

[Fact(DisplayName = Prefix + nameof(PropertyOverrides_Interfaces))]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,14 +29,13 @@ out object result
}
catch (TargetInvocationException e)
{
throw e.InnerException;
}
catch (TargetParameterCountException)
{
throw;
// An exception was thrown by the method being invoked
error = e.InnerException.ToString();
return null;
}
catch (Exception e)
{
// An exception was thrown while preparing the request or processing the result
error = e.ToString();
return null;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -251,7 +251,7 @@ Func<IClient, object[], InvokeResponse> invokeFunc
)
{
IServiceProvider serviceProvider = ServiceContainer.ServiceProvider;
IClient client = serviceProvider.GetRequiredService<IClient>(); ;
IClient client = serviceProvider.GetRequiredService<IClient>();
IJsiiToFrameworkConverter converter = serviceProvider.GetRequiredService<IJsiiToFrameworkConverter>();
IReferenceMap referenceMap = serviceProvider.GetRequiredService<IReferenceMap>();

Expand Down

0 comments on commit 1b851e1

Please sign in to comment.