Experimental - Task to Promise conversion #1567

Xicy · 2023-06-28T13:34:24Z

lahma · 2023-06-28T20:35:40Z

Great initiative here too. I don't have strong opinions, but might open a can of worms when people would start submitting issues with corner cases - but of course they can be referred to create a PR to improve things. It seems that this would handle the most requested cases anyway.

/cc @twop

twop · 2023-06-28T23:28:03Z

Jint/Runtime/Interop/DelegateWrapper.cs

+                if (!task.IsCompleted)
+                {
+                    // Task.Wait has the potential of inlining the task's execution on the current thread; avoid this.
+                    ((IAsyncResult) task).AsyncWaitHandle.WaitOne();


Does this mean that JS thread will be blocked until task is resolved?

Right, when promise needs to resolve, block to js thread until resolved.

I'm not sure but Evalutate function in unit tests, only process the script. It does not block until call RunAvailableContinuations

Yeah, I think that is the main problem that I encountered with the Task mapping too.

Tasks (plural in general case) can be resolved out of order at any time on any thread. But in JS it is one thread that collects continuations in the order they came in, in other words non blocking at any time. So I would probably advocate to use that as a guiding principal, and to my understanding the current code is not aligned with that, or I'm missing something obv?

my understanding the current code is not aligned with that

As per your explanation, yes code does not align. Maybe EventLoop works on another thread that can be closer to working asynchrony.

By the way, I don't have experience with that. If you lead the way, I try to enhance the code.

Edit:
PS: When I thought about works in another thread, It may occurs an error, because the engine does not thread-safe.

lahma · 2023-07-01T18:21:33Z

At the same time I see great value in this but do you folks see possible bad drawbacks (via bug reports and hard to track errors)? This would create a reasonable bridge (AFAIK) to allow common patterns and interoperability. Of course bug reports can be closed with "please send a PR to fix instead", but there's some fundamental impossibility - that might not be fair.

VelvetToroyashi · 2023-08-17T09:53:25Z

Wanted to poke at this to see where things were in terms of progress; this is great, and I'm excited to seeing these changes be implemented, however in my attempt at testing it, I've noticed a strange quirk.

Take the following C# code:

var engine = new Engine().SetValue("asyncMethod", () => Task.Delay(2000));

And in JS:

(
  async () => // Workaround for no top-level async/await
  {
    await asyncMethod();
  }
)();

When calling engine.Execute with that script, it runs as expected; the code is delayed for 2 seconds, and that's that.

However, if the delegate is instead made async, and awaits the task that represents the work:

async () => await Task.Delay(2000)

The promise is resolved immediately which is unexpected behavior, especially considering that on the surface, practically identical. My initial suspicion was it was something to do with how the task is handled, but I suspect this may be down to something deeper in the TPL implementation? I'll take a deeper look at this later in the day, but an interesting behavior nonetheless.

6 AM EDIT:

Instant-resolving issue seems to be fixed now? I don't believe I've touched anything, but now I'm concerned that I cannot reproduce this consistently; earlier, any time the delegate was asynchronous, the promise resolved instantly. No longer an issue.

Interestingly, something that remains consistent is that a Task-returning delegate (() => <Task>) returns undefined, whereas an async delegate async () => returns TaskVoidResult, though in theory both should return undefined or similar?

using Jint;

var engine = new Engine(s => s.Strict());

engine.SetValue("log", (Action<object>)Console.WriteLine);
engine.SetValue("taskDelegate", () => Task.Delay(100));
engine.SetValue("asyncDelegate", async () => await Task.Delay(100));
engine.Execute
(
    """
    (
        async () => 
        {
            let td = await taskDelegate();
            let ad = await asyncDelegate();
            log(`Task-delegate returns undefined: ${td === undefined}`);
            log(`Async-delegate returns undefined: ${ad === undefined}`);
        }
    )();
    """
);

Produces:

Task-delegate returns undefined: true
Async-delegate returns undefined: false

8/18 edit:

Turns out this is due to how the TPL works, and should be a relatively easy fix. I'll open a pull against this PR's branch momentarily.

Xicy · 2023-08-21T10:14:59Z

@VelvetToroyashi, Thank you also here :),
Is there any chance to check failed test cases?

VelvetToroyashi · 2023-08-21T10:16:50Z

@Xicy I can run the tests locally here in a second

Jint/Runtime/Interop/DelegateWrapper.cs

ejsmith · 2023-08-22T15:13:29Z

This looks awesome! Can't wait to get this ability. I was wondering if we can await a C# task from the a script... would it be possible to freeze and serialize the state of the JS engine at that point and resume it at a later time? I'm thinking of long running workflow type scenarios where there is a call out from the script to wait for some event to occur. That is implemented in the form of an await inside of the script that is calling a C# async method. That async method could then serialize engine state, wait some undefined amount of time and when that event occurs it would deserialize the engine state and resume the script execution. Thoughts?

Could potentially use this to provide the long running task support:
https://github.com/Azure/durabletask

sebastienros · 2023-08-22T16:07:59Z

@ejsmith maybe not rely on Jint as the core but use it as a way to run an activity in the workflow. Check Elsa Workflows or the one in Orchard Core (Else is coming from there) which are using Jint to run customizable tasks.

ejsmith · 2023-08-22T16:37:17Z

@ejsmith maybe not rely on Jint as the core but use it as a way to run an activity in the workflow. Check Elsa Workflows or the one in Orchard Core (Else is coming from there) which are using Jint to run customizable tasks.

Yeah, I'm going to look into that as well. Was just thinking it would be really nice and simpler to just have a script that did the entire workflow.

sebastienros · 2023-08-22T16:43:46Z

@ejsmith why not after all, currently Elsa/OrchardCore are using a Json description of it. Now I am starting to understand your vision. And being able to serialize the whole state and continue is something that would be helpful in other scenarios (it has bugged us many times). Even with some constraints like assuming the global objects are immutable so they don't have to be part of the serialization. Or that we can't even store state and it's up to the workflow engine to store what it needs and rehydrate it when it's resumed (custom C# bindings, state objects, ...).

lahma · 2023-08-22T16:53:42Z

I think that the easiest way to handle "game state" is to have a separate state object that can be JSON serialized. JavaScript engine state is far too complex to serialize successfully as one can mutate so many things (functions etc).

sebastienros · 2023-08-22T17:03:26Z

@lahma exactly, that's what I meant by "it's up to the workflow engine to store what it needs and rehydrate it"

So the remaining problem is to be able to stop the engine, maybe with a custom API call invoked from a C# callback, which would return some kind of serializable code pointer, and they another call to "resume" a script instead of "execute". I have no idea how it could work with the current implementation. Meaning that I believe that is technically impossible without changing how a script is currently processed.

ejsmith · 2023-08-22T18:08:37Z

I think that the easiest way to handle "game state" is to have a separate state object that can be JSON serialized. JavaScript engine state is far too complex to serialize successfully as one can mutate so many things (functions etc).

Yeah, I can see that. I'd think you'd have to restrict it to no global state and just serialize the call stack and variables passed to those stacks and then be able to jump back to that stack. But I'm sure that it's way more complicated than that. 🙂

ejsmith · 2023-08-22T18:12:36Z

@lahma exactly, that's what I meant by "it's up to the workflow engine to store what it needs and rehydrate it"

So the remaining problem is to be able to stop the engine, maybe with a custom API call invoked from a C# callback, which would return some kind of serializable code pointer, and they another call to "resume" a script instead of "execute". I have no idea how it could work with the current implementation. Meaning that I believe that is technically impossible without changing how a script is currently processed.

Thanks! I'm going to dive into that.

I currently have a really good scripting setup for custom logic and actions was just thinking that it would be really nice to just add something like await waitForDays(12) right into the script and various other kinds of awaits for different triggers. I think Elsa will work really well too. Just didn't want to get into having to do some sort of designer for building workflows. Does Elsa have a web designer?

sebastienros · 2023-08-22T18:30:50Z

Else does have a web designer.

sfmskywalker · 2023-09-27T17:46:24Z

Yep: https://github.com/elsa-workflows/elsa-studio

It even has an installation guide.

sfmskywalker · 2023-09-27T18:07:25Z

This looks awesome! Can't wait to get this ability. I was wondering if we can await a C# task from the a script... would it be possible to freeze and serialize the state of the JS engine at that point and resume it at a later time? I'm thinking of long running workflow type scenarios where there is a call out from the script to wait for some event to occur. That is implemented in the form of an await inside of the script that is calling a C# async method. That async method could then serialize engine state, wait some undefined amount of time and when that event occurs it would deserialize the engine state and resume the script execution. Thoughts?

Could potentially use this to provide the long running task support: https://github.com/Azure/durabletask

This is super interesting to me. I've been experimenting with a custom DSL that reads like JS, but one where keywords and statements translate to actual workflow activities, making it easy to describe a long running process that reads sequentially. As an example:

variable age = GetInput("age")";

if age > 16 then
{
    print("Enjoy your driver's license! And be careful!");
    event("crash");
    print("I told you to be careful -_-");
}
else
    print("Enjoy your bicycle!");
                        
print("Goodbye.");

In the above pseudo code, if/else keywords translate to Elsa's If activity where the first body becomes a Sequence activity for the Then property and the else body becomes a Sequence activity for the Else property. print translates to the WriteLine activity,

The relevant part here is the event function, which translates to the Event activity, which is a blocking activity that puts the workflow in a suspended state, and gets resumed when the application triggers the "crash" event.

But there are two significant limitations with this DSL for the moment:

It's not finished
It doesn't support all of the scripting stuff that a real JavaScript interpreter does.

So, I think that the idea to actually use real JavaScript and have it yield to the workflow engine, and have that resume the script at some point, is brilliant. If we can somehow pull this off, I might not have to write a custom DSL in the first place to be able to define a workflow in a (limited) scripting language. Instead, an entire workflow, both long-and short-running, could be defined in JS 🤯

And although this could be supported inside a JS activity running in a workflow, I think it would make just as much sense, if not more, to also have the ability to just write .js files to define long running processes - which is the point of the custom DSL.

I have no clue what it would take to make Jint support something like this (being able to yield back to the host app mid-script and then return), but let's keep marinating on it 🤔

twop · 2023-09-27T18:19:04Z

I think the best way to achieve that in JS without async/await is to use generators (look at return(v) method)
https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Operators/yield*#method_forwarding

A couple of libraries in JS ecosystem use that to build custom async DSL:

Hope that helps, note using generators + yield might be a different lift for embedders

sfmskywalker · 2023-09-27T18:31:58Z

yield sounds like it could be a perfect fit. If I understand correctly, then it might be used like this from a RunJavaScritpt activity:

const foo = 'bar";
yield*; // Yields back to the RunJavaScript activity, which will now create a bookmark. In most samples I've seen, yield* is used to generate data (e.g. yield* 42), but in our case, we just want to yield control back. Maybe it would be nice to make that explicit, like yield* suspend or whatever.
const someInput = getInput(); // Input would be provided by the application resuming the JS bookmark.
return someInput;

sfmskywalker · 2023-09-27T18:46:44Z

And I wonder, if that works, then could we also yield control from within a custom JS function? So that we could write a long-running workflow in pure JS:

const approveSignalUrl = generateSignalUrl("approve");
const rejectSignalUrl = generateSignalUrl("reject");

await sendEmail("approver@acme.com", `<a href="${approveSignalUrl}">Approve</a> or <a href="${rejectSignalUrl}">Reject</a>`);

if(signalReceived("approve")) // signalReceived should yield control back to engine.
{
   await sendEmail("contributor@acme.com", "Great job!");
}

if(signalReceived("reject"))
{
   await sendEmail("contributor@acme.com", "You and your work suck!");
}

In C#, signalReceived would somehow be able to tell Jint to yield, e.g.:

engine.SetValue("signalReceived", () => engine.YieldMagic("signalReceived"));

But that might be too farfetched. Perhaps the following approach might be more realistic, where the yield* keyword is returned in front of the suspending activity:

if(yield* signalReceived("approve")) // signalReceived should yield control back to engine.
{
   await sendEmail("contributor@acme.com", "Great job!");
}

if(yield* signalReceived("reject"))
{
   await sendEmail("contributor@acme.com", "You and your work suck!");
}

Here, the engine would understand that the SignalReceived activity is to be scheduled next, and when that gets resumed, it knows from the bookmark from where to resume JS execution.

sfmskywalker · 2023-09-27T18:51:15Z

Looking at the sample script, it would be important that the yield* keyword used in this way doesn't stop running the remaining code in order to also create a bookmark for the "reject" signal.

That got complicated real fast 😅

ejsmith · 2023-09-27T21:10:28Z

Yeah, I would personally prefer this type of long running workflow system over a visual designer. As we talked about before, the thing we'd need to make this possible is the ability for a Jint engine instance to be paused, all the callstack and state to be serialized, then later be able to come back and deserialize that callstack into a new Jint engine instance and resume from the await statement where it left off. So it would look like this:

const approveSignalUrl = generateSignalUrl("approve");
const rejectSignalUrl = generateSignalUrl("reject");

await sendEmail("approver@acme.com", `<a href="${approveSignalUrl}">Approve</a> or <a href="${rejectSignalUrl}">Reject</a>`);

// await a signal to awaken the script, tell it which signals we care about
let response = await signalRecieved("approve", "reject");
// signalRecieved is a C# async method that freezes the jint engine, serializes it
// and persists the state somewhere. C# code waits for one of those URLs to be hit
// which then triggers C# code that rehydrates the JINT engine callstack and
// resumes with a response saying which signal was triggered.

if (response.signal == "approve")
   await sendEmail("contributor@acme.com", "Great job!");
} else
   await sendEmail("contributor@acme.com", "You and your work suck!");
}

ejsmith · 2023-09-27T21:12:18Z

BTW, all of this first requires this Task to Promise conversion PR to be accepted which we have totally hi-jacked. Sorry @Xicy! @lahma what are the chances this PR is going to be accepted?

ExecutionCanceledException used for canceled operations

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

lahma · 2023-10-11T16:30:15Z

@Xicy thank you for the PR, looks good and kudos for the test coverage. I've rebased against main and it still seems to have some problems running against full .NET framework, could you please take a look?

Xicy · 2023-10-11T16:33:01Z

@Xicy thank you for the PR, looks good and kudos for the test coverage. I've rebased against main and it still seems to have some problems running against full .NET framework, could you please take a look?

First of all, I'm sorry for your loss. I'll try to take care of it next weekend.

Update:

I fixed the error, but I couldn't actually test it, because I changed my work stack dotnet to PHP and I didn't dotnet environment Could you review the PR @lahma

lahma · 2023-10-12T04:58:27Z

Thank you!

twop reviewed Jun 28, 2023

View reviewed changes

Xicy mentioned this pull request Jul 31, 2023

Any guidance on how to use await / top level await #1606

Closed

VelvetToroyashi mentioned this pull request Aug 18, 2023

Fix void-Task coallescence Xicy/jint#1

Merged

VelvetToroyashi reviewed Aug 21, 2023

View reviewed changes

Jint/Runtime/Interop/DelegateWrapper.cs Outdated Show resolved Hide resolved

VelvetToroyashi reviewed Aug 21, 2023

View reviewed changes

Jint/Runtime/Interop/DelegateWrapper.cs Outdated Show resolved Hide resolved

lahma mentioned this pull request Oct 11, 2023

How do I make an "async/await" operator for Jint? #755

Closed

Xicy added 2 commits October 11, 2023 19:20

Task methods convert to Promise internaly

a0a0c0f

added new test case from issue 514-1507127509

b8001b5

Xicy and others added 3 commits October 11, 2023 19:20

Readablity increased

86d3ce0

ExecutionCanceledException used for canceled operations

@VelvetToroyashi solvings

5b996d5

lahma force-pushed the feature/task-promise-conversion branch from 6c23400 to 5b996d5 Compare October 11, 2023 16:20

Workaround check

802fbfa

lahma approved these changes Oct 12, 2023

View reviewed changes

lahma merged commit c905f53 into sebastienros:main Oct 12, 2023
3 checks passed

wazzamatazz mentioned this pull request Jan 17, 2024

Add ValueTask and ValueTask<T> to Promise conversion #1744

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Experimental - Task to Promise conversion #1567

Experimental - Task to Promise conversion #1567

Xicy commented Jun 28, 2023

lahma commented Jun 28, 2023 •

edited

twop Jun 28, 2023

Xicy Jun 29, 2023

twop Jun 29, 2023

Xicy Jun 29, 2023 •

edited

lahma commented Jul 1, 2023

VelvetToroyashi commented Aug 17, 2023 •

edited

Xicy commented Aug 21, 2023

VelvetToroyashi commented Aug 21, 2023

ejsmith commented Aug 22, 2023 •

edited

sebastienros commented Aug 22, 2023

ejsmith commented Aug 22, 2023

sebastienros commented Aug 22, 2023

lahma commented Aug 22, 2023

sebastienros commented Aug 22, 2023

ejsmith commented Aug 22, 2023

ejsmith commented Aug 22, 2023

sebastienros commented Aug 22, 2023

sfmskywalker commented Sep 27, 2023

sfmskywalker commented Sep 27, 2023

twop commented Sep 27, 2023 •

edited

sfmskywalker commented Sep 27, 2023 •

edited

sfmskywalker commented Sep 27, 2023 •

edited

sfmskywalker commented Sep 27, 2023

ejsmith commented Sep 27, 2023 •

edited

ejsmith commented Sep 27, 2023

lahma commented Oct 11, 2023

Xicy commented Oct 11, 2023 •

edited

lahma commented Oct 12, 2023

Experimental - Task to Promise conversion #1567

Experimental - Task to Promise conversion #1567

Conversation

Xicy commented Jun 28, 2023

lahma commented Jun 28, 2023 • edited

twop Jun 28, 2023

Choose a reason for hiding this comment

Xicy Jun 29, 2023

Choose a reason for hiding this comment

twop Jun 29, 2023

Choose a reason for hiding this comment

Xicy Jun 29, 2023 • edited

Choose a reason for hiding this comment

lahma commented Jul 1, 2023

VelvetToroyashi commented Aug 17, 2023 • edited

6 AM EDIT:

8/18 edit:

Xicy commented Aug 21, 2023

VelvetToroyashi commented Aug 21, 2023

ejsmith commented Aug 22, 2023 • edited

sebastienros commented Aug 22, 2023

ejsmith commented Aug 22, 2023

sebastienros commented Aug 22, 2023

lahma commented Aug 22, 2023

sebastienros commented Aug 22, 2023

ejsmith commented Aug 22, 2023

ejsmith commented Aug 22, 2023

sebastienros commented Aug 22, 2023

sfmskywalker commented Sep 27, 2023

sfmskywalker commented Sep 27, 2023

twop commented Sep 27, 2023 • edited

sfmskywalker commented Sep 27, 2023 • edited

sfmskywalker commented Sep 27, 2023 • edited

sfmskywalker commented Sep 27, 2023

ejsmith commented Sep 27, 2023 • edited

ejsmith commented Sep 27, 2023

lahma commented Oct 11, 2023

Xicy commented Oct 11, 2023 • edited

Update:

lahma commented Oct 12, 2023

lahma commented Jun 28, 2023 •

edited

Xicy Jun 29, 2023 •

edited

VelvetToroyashi commented Aug 17, 2023 •

edited

ejsmith commented Aug 22, 2023 •

edited

twop commented Sep 27, 2023 •

edited

sfmskywalker commented Sep 27, 2023 •

edited

sfmskywalker commented Sep 27, 2023 •

edited

ejsmith commented Sep 27, 2023 •

edited

Xicy commented Oct 11, 2023 •

edited