From 867453a06dfcffedb24cbd39bbb513773ce26377 Mon Sep 17 00:00:00 2001 From: Pranav K Date: Thu, 13 Jun 2019 21:54:05 -0700 Subject: [PATCH] Resolve the JSInterop call if deserialization fails (dotnet/extensions#1802) * Resolve the JSInterop call if deserialization fails \n\nCommit migrated from https://github.com/dotnet/extensions/commit/26cf0c9f4eb73237a460b6cc6a4912675869a0b5 --- src/JSInterop/JSInterop.slnf | 10 +++++++ .../ref/Microsoft.JSInterop.netstandard2.0.cs | 1 + .../src/DotNetDispatcher.cs | 3 +- .../Microsoft.JSInterop/src/JSException.cs | 9 ++++++ .../Microsoft.JSInterop/src/JSRuntimeBase.cs | 16 ++++++++--- .../test/DotNetDispatcherTest.cs | 18 ++++++++++++ .../test/JSRuntimeBaseTest.cs | 28 +++++++++++++++++++ 7 files changed, 79 insertions(+), 6 deletions(-) create mode 100644 src/JSInterop/JSInterop.slnf diff --git a/src/JSInterop/JSInterop.slnf b/src/JSInterop/JSInterop.slnf new file mode 100644 index 0000000000..c6cbd4cb3a --- /dev/null +++ b/src/JSInterop/JSInterop.slnf @@ -0,0 +1,10 @@ +{ + "solution": { + "path": "..\\..\\Extensions.sln", + "projects": [ + "src\\JSInterop\\Microsoft.JSInterop\\src\\Microsoft.JSInterop.csproj", + "src\\JSInterop\\Microsoft.JSInterop\\test\\Microsoft.JSInterop.Tests.csproj", + "src\\JSInterop\\Mono.WebAssembly.Interop\\src\\Mono.WebAssembly.Interop.csproj", + ] + } +} diff --git a/src/JSInterop/Microsoft.JSInterop/ref/Microsoft.JSInterop.netstandard2.0.cs b/src/JSInterop/Microsoft.JSInterop/ref/Microsoft.JSInterop.netstandard2.0.cs index 1db3385bd7..c2884dc64a 100644 --- a/src/JSInterop/Microsoft.JSInterop/ref/Microsoft.JSInterop.netstandard2.0.cs +++ b/src/JSInterop/Microsoft.JSInterop/ref/Microsoft.JSInterop.netstandard2.0.cs @@ -37,6 +37,7 @@ namespace Microsoft.JSInterop public partial class JSException : System.Exception { public JSException(string message) { } + public JSException(string message, System.Exception innerException) { } } public abstract partial class JSInProcessRuntimeBase : Microsoft.JSInterop.JSRuntimeBase, Microsoft.JSInterop.IJSInProcessRuntime, Microsoft.JSInterop.IJSRuntime { diff --git a/src/JSInterop/Microsoft.JSInterop/src/DotNetDispatcher.cs b/src/JSInterop/Microsoft.JSInterop/src/DotNetDispatcher.cs index 5ecd3506e0..d7fc99f484 100644 --- a/src/JSInterop/Microsoft.JSInterop/src/DotNetDispatcher.cs +++ b/src/JSInterop/Microsoft.JSInterop/src/DotNetDispatcher.cs @@ -227,11 +227,10 @@ namespace Microsoft.JSInterop catch { // Always dispose the JsonDocument in case of an error. - jsonDocument.Dispose(); + jsonDocument?.Dispose(); throw; } - static bool IsIncorrectDotNetObjectRefUse(JsonElement item, Type parameterType) { // Check for incorrect use of DotNetObjectRef at the top level. We know it's diff --git a/src/JSInterop/Microsoft.JSInterop/src/JSException.cs b/src/JSInterop/Microsoft.JSInterop/src/JSException.cs index 2929f69311..c2e4f1b7ae 100644 --- a/src/JSInterop/Microsoft.JSInterop/src/JSException.cs +++ b/src/JSInterop/Microsoft.JSInterop/src/JSException.cs @@ -17,5 +17,14 @@ namespace Microsoft.JSInterop public JSException(string message) : base(message) { } + + /// + /// Constructs an instance of . + /// + /// The exception message. + /// The inner exception. + public JSException(string message, Exception innerException) : base(message, innerException) + { + } } } diff --git a/src/JSInterop/Microsoft.JSInterop/src/JSRuntimeBase.cs b/src/JSInterop/Microsoft.JSInterop/src/JSRuntimeBase.cs index 3e39e42e9a..e2b3875a50 100644 --- a/src/JSInterop/Microsoft.JSInterop/src/JSRuntimeBase.cs +++ b/src/JSInterop/Microsoft.JSInterop/src/JSRuntimeBase.cs @@ -94,10 +94,18 @@ namespace Microsoft.JSInterop if (succeeded) { var resultType = TaskGenericsUtil.GetTaskCompletionSourceResultType(tcs); - var result = asyncCallResult != null ? - JsonSerializer.Parse(asyncCallResult.JsonElement.GetRawText(), resultType, JsonSerializerOptionsProvider.Options) : - null; - TaskGenericsUtil.SetTaskCompletionSourceResult(tcs, result); + try + { + var result = asyncCallResult != null ? + JsonSerializer.Parse(asyncCallResult.JsonElement.GetRawText(), resultType, JsonSerializerOptionsProvider.Options) : + null; + TaskGenericsUtil.SetTaskCompletionSourceResult(tcs, result); + } + catch (Exception exception) + { + var message = $"An exception occurred executing JS interop: {exception.Message}. See InnerException for more details."; + TaskGenericsUtil.SetTaskCompletionSourceException(tcs, new JSException(message, exception)); + } } else { diff --git a/src/JSInterop/Microsoft.JSInterop/test/DotNetDispatcherTest.cs b/src/JSInterop/Microsoft.JSInterop/test/DotNetDispatcherTest.cs index c65b4e4680..7854122feb 100644 --- a/src/JSInterop/Microsoft.JSInterop/test/DotNetDispatcherTest.cs +++ b/src/JSInterop/Microsoft.JSInterop/test/DotNetDispatcherTest.cs @@ -363,6 +363,24 @@ namespace Microsoft.JSInterop.Tests Assert.Contains(nameof(ThrowingClass.AsyncThrowingMethod), exception); }); + [Fact] + public Task BeginInvoke_ThrowsWithInvalidArgsJson_WithCallId() => WithJSRuntime(async jsRuntime => + { + // Arrange + var callId = "123"; + var resultTask = jsRuntime.NextInvocationTask; + DotNetDispatcher.BeginInvoke(callId, thisAssemblyName, "InvocableStaticWithParams", default, "not json"); + + await resultTask; // This won't throw, it sets properties on the jsRuntime. + + // Assert + using var jsonDocument = JsonDocument.Parse(jsRuntime.LastInvocationArgsJson); + var result = jsonDocument.RootElement; + Assert.Equal(callId, result[0].GetString()); + Assert.False(result[1].GetBoolean()); // Fails + Assert.Contains("JsonReaderException: '<' is an invalid start of a value.", result[2].GetString()); + }); + Task WithJSRuntime(Action testCode) { return WithJSRuntime(jsRuntime => diff --git a/src/JSInterop/Microsoft.JSInterop/test/JSRuntimeBaseTest.cs b/src/JSInterop/Microsoft.JSInterop/test/JSRuntimeBaseTest.cs index 82126836d8..4e0818219b 100644 --- a/src/JSInterop/Microsoft.JSInterop/test/JSRuntimeBaseTest.cs +++ b/src/JSInterop/Microsoft.JSInterop/test/JSRuntimeBaseTest.cs @@ -5,6 +5,7 @@ using System; using System.Collections.Generic; using System.Linq; using System.Text.Json; +using System.Threading.Tasks; using Microsoft.JSInterop.Internal; using Xunit; @@ -86,6 +87,33 @@ namespace Microsoft.JSInterop.Tests Assert.Equal("This is a test exception", ((JSException)task.Exception.InnerException).Message); } + [Fact] + public async Task CanCompleteAsyncCallsWithErrorsDuringDeserialization() + { + // Arrange + var runtime = new TestJSRuntime(); + + // Act/Assert: Tasks not initially completed + var unrelatedTask = runtime.InvokeAsync("unrelated call", Array.Empty()); + var task = runtime.InvokeAsync("test identifier", Array.Empty()); + Assert.False(unrelatedTask.IsCompleted); + Assert.False(task.IsCompleted); + using var jsonDocument = JsonDocument.Parse("\"Not a string\""); + + // Act/Assert: Task can be failed + runtime.OnEndInvoke( + runtime.BeginInvokeCalls[1].AsyncHandle, + /* succeeded: */ true, + new JSAsyncCallResult(jsonDocument, jsonDocument.RootElement)); + Assert.False(unrelatedTask.IsCompleted); + + var jsException = await Assert.ThrowsAsync(() => task); + Assert.IsType(jsException.InnerException); + + // Verify we've disposed the JsonDocument. + Assert.Throws(() => jsonDocument.RootElement.Type); + } + [Fact] public void CannotCompleteSameAsyncCallMoreThanOnce() {