diff --git a/src/JSInterop/Microsoft.JSInterop/src/DotNetDispatcher.cs b/src/JSInterop/Microsoft.JSInterop/src/DotNetDispatcher.cs index 0f346bbfdd..ac936e670a 100644 --- a/src/JSInterop/Microsoft.JSInterop/src/DotNetDispatcher.cs +++ b/src/JSInterop/Microsoft.JSInterop/src/DotNetDispatcher.cs @@ -7,6 +7,7 @@ using System.Collections.Concurrent; using System.Collections.Generic; using System.Linq; using System.Reflection; +using System.Runtime.ExceptionServices; using System.Threading.Tasks; namespace Microsoft.JSInterop @@ -72,8 +73,10 @@ namespace Microsoft.JSInterop ? null : jsRuntimeBaseInstance.ArgSerializerStrategy.FindDotNetObject(dotNetObjectId); + // Using ExceptionDispatchInfo here throughout because we want to always preserve + // original stack traces. object syncResult = null; - Exception syncException = null; + ExceptionDispatchInfo syncException = null; try { @@ -81,45 +84,38 @@ namespace Microsoft.JSInterop } catch (Exception ex) { - syncException = ex; + syncException = ExceptionDispatchInfo.Capture(ex); } // If there was no callId, the caller does not want to be notified about the result - if (callId != null) + if (callId == null) { - // Invoke and coerce the result to a Task so the caller can use the same async API - // for both synchronous and asynchronous methods - var task = CoerceToTask(syncResult, syncException); - - task.ContinueWith(completedTask => + return; + } + else if (syncException != null) + { + // Threw synchronously, let's respond. + jsRuntimeBaseInstance.EndInvokeDotNet(callId, false, syncException); + } + else if (syncResult is Task task) + { + // Returned a task - we need to continue that task and then report an exception + // or return the value. + task.ContinueWith(t => { - try + if (t.Exception != null) { - var result = TaskGenericsUtil.GetTaskResult(completedTask); - jsRuntimeBaseInstance.EndInvokeDotNet(callId, true, result); + var exception = t.Exception.GetBaseException(); + jsRuntimeBaseInstance.EndInvokeDotNet(callId, false, ExceptionDispatchInfo.Capture(exception)); } - catch (Exception ex) - { - ex = UnwrapException(ex); - jsRuntimeBaseInstance.EndInvokeDotNet(callId, false, ex); - } - }); - } - } - private static Task CoerceToTask(object syncResult, Exception syncException) - { - if (syncException != null) - { - return Task.FromException(syncException); - } - else if (syncResult is Task syncResultTask) - { - return syncResultTask; + var result = TaskGenericsUtil.GetTaskResult(task); + jsRuntimeBaseInstance.EndInvokeDotNet(callId, true, result); + }, TaskScheduler.Current); } else { - return Task.FromResult(syncResult); + jsRuntimeBaseInstance.EndInvokeDotNet(callId, true, syncResult); } } @@ -175,9 +171,10 @@ namespace Microsoft.JSInterop { return methodInfo.Invoke(targetInstance, suppliedArgs); } - catch (Exception ex) + catch (TargetInvocationException tie) when (tie.InnerException != null) { - throw UnwrapException(ex); + ExceptionDispatchInfo.Capture(tie.InnerException).Throw(); + throw null; // unreachable } } @@ -285,15 +282,5 @@ namespace Microsoft.JSInterop return loadedAssemblies.FirstOrDefault(a => a.GetName().Name.Equals(assemblyName, StringComparison.Ordinal)) ?? throw new ArgumentException($"There is no loaded assembly with the name '{assemblyName}'."); } - - private static Exception UnwrapException(Exception ex) - { - while ((ex is AggregateException || ex is TargetInvocationException) && ex.InnerException != null) - { - ex = ex.InnerException; - } - - return ex; - } } } diff --git a/src/JSInterop/Microsoft.JSInterop/src/JSRuntimeBase.cs b/src/JSInterop/Microsoft.JSInterop/src/JSRuntimeBase.cs index 09379396cf..d18bc7f4fe 100644 --- a/src/JSInterop/Microsoft.JSInterop/src/JSRuntimeBase.cs +++ b/src/JSInterop/Microsoft.JSInterop/src/JSRuntimeBase.cs @@ -3,6 +3,7 @@ using System; using System.Collections.Concurrent; +using System.Runtime.ExceptionServices; using System.Threading; using System.Threading.Tasks; @@ -80,6 +81,10 @@ namespace Microsoft.JSInterop { resultOrException = resultOrException.ToString(); } + else if (!success && resultOrException is ExceptionDispatchInfo edi) + { + resultOrException = edi.SourceException.ToString(); + } // We pass 0 as the async handle because we don't want the JS-side code to // send back any notification (we're just providing a result for an existing async call) diff --git a/src/JSInterop/Microsoft.JSInterop/test/DotNetDispatcherTest.cs b/src/JSInterop/Microsoft.JSInterop/test/DotNetDispatcherTest.cs index 93ef9a2498..e3f91a6fd0 100644 --- a/src/JSInterop/Microsoft.JSInterop/test/DotNetDispatcherTest.cs +++ b/src/JSInterop/Microsoft.JSInterop/test/DotNetDispatcherTest.cs @@ -285,6 +285,54 @@ namespace Microsoft.JSInterop.Tests Assert.Equal(2468, resultDto2.IntVal); }); + + [Fact] + public Task CanInvokeSyncThrowingMethod() => WithJSRuntime(async jsRuntime => + { + // Arrange + + // Act + var callId = "123"; + var resultTask = jsRuntime.NextInvocationTask; + DotNetDispatcher.BeginInvoke(callId, thisAssemblyName, nameof(ThrowingClass.ThrowingMethod), default, default); + + await resultTask; // This won't throw, it sets properties on the jsRuntime. + + // Assert + var result = Json.Deserialize(jsRuntime.LastInvocationArgsJson); + Assert.Equal(callId, result[0]); + Assert.False((bool)result[1]); // Fails + + // Make sure the method that threw the exception shows up in the call stack + // https://github.com/aspnet/AspNetCore/issues/8612 + var exception = (string)result[2]; + Assert.Contains(nameof(ThrowingClass.ThrowingMethod), exception); + }); + + [Fact] + public Task CanInvokeAsyncThrowingMethod() => WithJSRuntime(async jsRuntime => + { + // Arrange + + // Act + var callId = "123"; + var resultTask = jsRuntime.NextInvocationTask; + DotNetDispatcher.BeginInvoke(callId, thisAssemblyName, nameof(ThrowingClass.AsyncThrowingMethod), default, default); + + await resultTask; // This won't throw, it sets properties on the jsRuntime. + + // Assert + var result = Json.Deserialize(jsRuntime.LastInvocationArgsJson); + Assert.Equal(callId, result[0]); + Assert.False((bool)result[1]); // Fails + + // Make sure the method that threw the exception shows up in the call stack + // https://github.com/aspnet/AspNetCore/issues/8612 + var exception = (string)result[2]; + Assert.Contains(nameof(ThrowingClass.AsyncThrowingMethod), exception); + }); + + Task WithJSRuntime(Action testCode) { return WithJSRuntime(jsRuntime => @@ -413,6 +461,22 @@ namespace Microsoft.JSInterop.Tests public int IntVal { get; set; } } + public class ThrowingClass + { + [JSInvokable] + public static string ThrowingMethod() + { + throw new InvalidTimeZoneException(); + } + + [JSInvokable] + public static async Task AsyncThrowingMethod() + { + await Task.Yield(); + throw new InvalidTimeZoneException(); + } + } + public class TestJSRuntime : JSInProcessRuntimeBase { private TaskCompletionSource _nextInvocationTcs = new TaskCompletionSource();