Fix dotnet/extensions#8612 JSInterop throws truncated call stack

The fix for this is to use more ExceptionDispatchInfo! Basically
everwhere that we handle an exception is now an EDI. It's easy to pass
these around and they do the right thing as far as perserving the stack
in.

I de-factored this code a little bit to make all of this work, but it's
now pretty unambiguously correct upon inspection.

I started out wanting to get rid of the continue-with because we've
found that pretty error prone overall. However making this method
async-void started to ask more questions than it answered. What happens
if serialization of the exception fails?
\n\nCommit migrated from 0a27fd3ad6

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;
-        }
     }
 }

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)

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<SimpleJson.JsonArray>(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<SimpleJson.JsonArray>(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<TestJSRuntime> 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<string> AsyncThrowingMethod()
+            {
+                await Task.Yield();
+                throw new InvalidTimeZoneException();
+            }
+        }
+
         public class TestJSRuntime : JSInProcessRuntimeBase
         {
             private TaskCompletionSource<object> _nextInvocationTcs = new TaskCompletionSource<object>();