Browse Source

Add experimental support for Task to Promise conversion (#1567)

This commit fixes an issue where an `async Task` (be it a method or a delegate)
would end up being marshalled directly to JS, giving a `Task<VoidTaskResult>`
to the user, instead of `undefined`, which is what is returned for
"synchronous tasks", i.e. any Task-returning invokable function that does
not generate an async state machine of its own (that is to say, any function
that returns `Task`, not `async Task`.

This commit fixes the issue by checking if a Task's result is equal to
`VoidTaskResult`, which is an internal type used by the runtime to indicate
a void-returning Task, such as that from an `async Task` method/delegate

---------

Co-authored-by: Velvet Toroyashi <[email protected]>
Umut Akkaya 1 year ago
parent
commit
c905f53c99
2 changed files with 122 additions and 2 deletions
  1. 68 0
      Jint.Tests/Runtime/AwaitTests.cs
  2. 54 2
      Jint/Runtime/Interop/DelegateWrapper.cs

+ 68 - 0
Jint.Tests/Runtime/AwaitTests.cs

@@ -10,4 +10,72 @@ public class AsyncTests
         result = result.UnwrapIfPromise();
         Assert.Equal("1", result);
     }
+
+    [Fact]
+    public void ShouldTaskConvertedToPromiseInJS()
+    {
+        Engine engine = new();
+        engine.SetValue("callable", Callable);
+        var result = engine.Evaluate("callable().then(x=>x*2)");
+        result = result.UnwrapIfPromise();
+        Assert.Equal(2, result);
+
+        static async Task<int> Callable()
+        {
+            await Task.Delay(10);
+            Assert.True(true);
+            return 1;
+        }
+    }
+
+    [Fact]
+    public void ShouldTaskCatchWhenCancelled()
+    {
+        Engine engine = new();
+        CancellationTokenSource cancel = new();
+        cancel.Cancel();
+        engine.SetValue("token", cancel.Token);
+        engine.SetValue("callable", Callable);
+        engine.SetValue("assert", new Action<bool>(Assert.True));
+        var result = engine.Evaluate("callable(token).then(_ => assert(false)).catch(_ => assert(true))");
+        result = result.UnwrapIfPromise();
+        static async Task Callable(CancellationToken token)
+        {
+            await Task.FromCanceled(token);
+        }
+    }
+
+    [Fact]
+    public void ShouldTaskCatchWhenThrowError()
+    {
+        Engine engine = new();
+        engine.SetValue("callable", Callable);
+        engine.SetValue("assert", new Action<bool>(Assert.True));
+        var result = engine.Evaluate("callable().then(_ => assert(false)).catch(_ => assert(true))");
+
+        static async Task Callable()
+        {
+            await Task.Delay(10);
+            throw new Exception();
+        }
+    }
+
+    [Fact]
+    public void ShouldTaskAwaitCurrentStack()
+    {
+        //https://github.com/sebastienros/jint/issues/514#issuecomment-1507127509
+        Engine engine = new();
+        string log = "";
+        engine.SetValue("myAsyncMethod", new Func<Task>(async () =>
+        {
+            await Task.Delay(1000);
+            log += "1";
+        }));
+        engine.SetValue("myAsyncMethod2", new Action(() =>
+        {
+            log += "2";
+        }));
+        engine.Execute("async function hello() {await myAsyncMethod();myAsyncMethod2();} hello();");
+        Assert.Equal("12", log);
+    }
 }

+ 54 - 2
Jint/Runtime/Interop/DelegateWrapper.cs

@@ -130,13 +130,65 @@ namespace Jint.Runtime.Interop
 
             try
             {
-                return FromObject(Engine, _d.DynamicInvoke(parameters));
+                var result = _d.DynamicInvoke(parameters);
+                if (result is not Task task)
+                {
+                    return FromObject(Engine, result);
+                }
+                return ConvertTaskToPromise(task);
             }
             catch (TargetInvocationException exception)
             {
-                ExceptionHelper.ThrowMeaningfulException(_engine, exception);
+                ExceptionHelper.ThrowMeaningfulException(Engine, exception);
                 throw;
             }
         }
+
+        private JsValue ConvertTaskToPromise(Task task)
+        {
+            var (promise, resolve, reject) = Engine.RegisterPromise();
+            task = task.ContinueWith(continuationAction =>
+            {
+                if (continuationAction.IsFaulted)
+                {
+                    reject(FromObject(Engine, continuationAction.Exception));
+                }
+                else if (continuationAction.IsCanceled)
+                {
+                    reject(FromObject(Engine, new ExecutionCanceledException()));
+                }
+                else
+                {
+                    // Special case: Marshal `async Task` as undefined, as this is `Task<VoidTaskResult>` at runtime
+                    // See https://github.com/sebastienros/jint/pull/1567#issuecomment-1681987702
+                    if (Task.CompletedTask.Equals(continuationAction))
+                    {
+                        resolve(FromObject(Engine, JsValue.Undefined));
+                        return;
+                    }
+
+                    var result = continuationAction.GetType().GetProperty(nameof(Task<object>.Result));
+                    if (result is not null)
+                    {
+                        resolve(FromObject(Engine, result.GetValue(continuationAction)));
+                    }
+                    else
+                    {
+                        resolve(FromObject(Engine, JsValue.Undefined));
+                    }
+                }
+            });
+
+            Engine.AddToEventLoop(() =>
+            {
+                if (!task.IsCompleted)
+                {
+                    // Task.Wait has the potential of inlining the task's execution on the current thread; avoid this.
+                    ((IAsyncResult) task).AsyncWaitHandle.WaitOne();
+                }
+            });
+
+            return promise;
+        }
     }
 }