Can we have a syntactic sugar for `cancelToken.throwIfRequested`?

[inikulin]

I was just thinking about how cancel tokens paradigm can be applied to our existing code and it appears that we will need to call cancelToken.throwIfRequested nearly in all our async functions. This creates a lot of unecessary code noise. It would be nice to have an indicator that this particular function can be cancelled by passed token, e.g. :

async function foo (bar, cancelToken*) { // Just an example syntax
    // Implicitly calls cancelToken.throwIfRequesed(); on call
}

Going even farther: implicity call cancelToken.throwIfRequesed() after each expression/statement in function. But IMHO it's too crazy.

I know that this unlikely a good example of how this problem can be addressed, but anyway it would be nice if we will have some bikeshedding on this subject.

[domenic]

So in general (see #16) it is not clear when cancelToken.cancelIfRequested() would be useful. If you are working with already-cancelable async operations, you can just pass them the cancel token. See https://github.com/domenic/cancelable-promise/blob/master/Cancel%20Tokens.md#use-within-async-functions

It's basically only useful for an async function that does many uncancelable async things, and wants to provide cancelation opportunities in between each of them. I have a not-very-good example (see above link), but honestly it would be better served by either Promise.all (to do the work in parallel) or async generator functions (to do the work in series) where you could just call asyncGenerator.return(). I am still hoping (#16 again) someone can help supply me with an example where cancelToken.cancelIfRequested() is actually useful.

[inikulin]

It's basically only useful for an async function that does many uncancelable async things, and wants to provide cancelation opportunities in between each of them.

This. E.g. we have:

async function someFunc() {
      await doOneThing();
      await doAnotherThing();

      await Promise.all([doLots(), ofOther(), thing()]);

      await doSomethingElse();
}

We need either pass cancel token around (which is OK) but then we need to add throwIfCancel or even worse

if (cancelToken.requested) {
  cancel throw;
}

at the beginning of each function body then. This is annoying.

Another solution is to check cancel token before each call in someFunc which doesn't makes things better.

I have a not-very-good example (see above link)

Are you talking about generateABunchOfKeys example?

[domenic]

This. E.g. we have:

But what async things are those doWhatever functions doing, that is not cancelable? A real-world example would be great.

Are you talking about generateABunchOfKeys example?

Yes.

[inikulin]

But what async things are those doWhatever functions doing, that is not cancelable? A real-world example would be great.

We have a really complex logic that covers thousand lines of code that eventually ends up in single promise. I'll try to create simplified example tomorrow with clear head, because I feel sleepy right now.

[inikulin]

Example

We have an automated browser testing framework. Here is a simplified workflow:

async function runTests (testPaths, browserDescriptors, reporterPluginPath) {
    var [ tests, browsers ] = await createRunnableConfiguration(testPaths, browserDescriptors, reporterPluginPath);

    return await executeTask(tests, browsers);
}

async function createRunnableConfiguration (testPaths, browserDescriptors, reporterPluginPath) {
    var browsersSpawnInfo = await resolveBrowserInfo(browserDescriptors); // Browsers can be represented as aliases, paths, or CI server handles. 
                                                                          // Resolve them to spawn info as soon as possible to enable early errors.

    return Promise.all([getTests(testPaths), spawnBrowsers(browsersSpawnInfo)]);
}

async function resolveBrowserInfo (browserDescriptors) {
    return Promise.all(browserDescriptors.map(async descr => {
        if (descr.path)
            await checkBrowserExecutableExistence(descr.path);
        else
            await checkCIServerForBrowserExistence(descr.alias);
    }));
}

async function getTests (testPaths) {
    return Promise.all(testPaths.map(async path => {
        var fileContent = await fs.readFile(path);

        var content = babelCompileContent(fileContent);

        return evaluateTestFile(content);
    }));
}

async function spawnBrowsers (spawnInfo) {
    return Promise.all(spawnInfo.map(async info => {
        var browser = await spawnBrowser(info);

        await Promise.race([browserConnection(browser), timeout()]);
    }));
}

async function executeTask (tests, browsers) {
    for (const test of tests)
        await test.run();

    await browsers.close();
}

This is just extremely simplified sceleton of the actual code. Each function here also branches into many async function calls. But I've tried to highlight only top level heavy tasks there cancelation makes sense.

Even with this example here is how code will look like once we will add cancellation using tokens:

async function runTests (testPaths, browserDescriptors, reporterPluginPath, cancelToken) {
    var [ tests, browsers ] = await createRunnableConfiguration(testPaths, browserDescriptors, reporterPluginPath);

    return await executeTask(tests, browsers, cancelToken);
);
}

async function createRunnableConfiguration (testPaths, browserDescriptors, reporterPluginPath, cancelToken) {
    var browsersSpawnInfo = await resolveBrowserInfo(browserDescriptors, cancelToken); // Browsers can be represented as aliases, paths, or CI server handles. 
                                                                                       // Resolve them to spawn info as soon as possible to enable early errors.

    return Promise.all([getTests(testPaths, cancelToken), spawnBrowsers(browsersSpawnInfo, cancelToken)]);
}

async function resolveBrowserInfo (browserDescriptors, cancelToken) {
    return Promise.all(browserDescriptors.map(async descr => {
        cancelToken.throwIfRequested();

        if (descr.path)
            await checkBrowserExecutableExistence(descr.path);
        else
            await checkCIServerForBrowserExistence(descr.alias);
    }));
}

async function getTests (testPaths, cancelToken) {
    return Promise.all(testPaths.map(async path => {
        cancelToken.throwIfRequested();

        var fileContent = await fs.readFile(path);

        cancelToken.throwIfRequested();
        var content = babelCompileContent(fileContent);

        return evaluateTestFile(content);
    }));
}

async function spawnBrowsers (spawnInfo, cancelToken) {
    return Promise.all(spawnInfo.map(async info => {
        cancelToken.throwIfRequested();

        var browser = await spawnBrowser(info);

        await Promise.race([browserConnection(browser), timeout(), intervalCancelTokenCheck(cancelToken)]);
    }));
}

async function executeTask (tests, browsers, cancelToken) {
    for (const test of tests) {
        cancelToken.throwIfRequested();

        await test.run();
    }

    await browsers.close();
}

CancelTrigger idea

IMHO this is too noisy and it's really easy to shoot yorself in the foot. I was thinking about it recently and came to the following idea: what if instead of tokens which you need to pass around we will have cancel triggers which will be checked automatically and cancel whole consequent chain:

var rootTask = new Promise((resolve, reject) => {...});

function doSmthgWhenRootComplete (cancelTrigger) {
    return rootTask
        .then(() => {
            return doSomething();
        }, { cancelTrigger })
        .then(() => {
            return doSomethingElse();
        })
        .then(() => {
            return andElse();
        });
}

cancelTrigger.cancel();

which desugars to:

var rootTask = new Promise((resolve, reject) => {...});

function doSmthgWhenRootComplete (cancelTrigger) {
    return rootTask
        .then(() => {
            cancelTrigger.throwIfCancel();

            return doSomething();
        }, { cancelTrigger })
        .then(() => {
            cancelTrigger.throwIfCancel();

            return doSomethingElse();
        })
        .then(() => {
            cancelTrigger.throwIfCancel();

            return andElse();
        });
}

cancelTrigger.cancel();

With this approach we will have immutable promises due to down propagation, but behavior which is very close to promise.cancel(). And like in my OP post we can have sugar for this to use with async/await:

async function runTests (testPaths,
                         browserDescriptors,
                         reporterPluginPath,
                         cancelTrigger*) // <-- this tells that whole chain to which this async function expands can be cancelled by this trigger.
{ 

    var [ tests, browsers ] = await createRunnableConfiguration(testPaths, browserDescriptors, reporterPluginPath);

    return await executeTask(tests, browsers);
}

async function createRunnableConfiguration (testPaths, browserDescriptors, reporterPluginPath) {
    var browsersSpawnInfo = await resolveBrowserInfo(browserDescriptors); // Browsers can be represented as aliases, paths, or CI server handles.
                                                                          // Resolve them to spawn info as soon as possible to enable early errors.

    return Promise.all([getTests(testPaths), spawnBrowsers(browsersSpawnInfo)]);
}

async function resolveBrowserInfo (browserDescriptors) {
    return Promise.all(browserDescriptors.map(async descr => {
        if (descr.path)
            await checkBrowserExecutableExistence(descr.path);
        else
            await checkCIServerForBrowserExistence(descr.alias);
    }));
}

async function getTests (testPaths) {
    return Promise.all(testPaths.map(async path => {
        var fileContent = await fs.readFile(path);

        var content = babelCompileContent(fileContent);

        return evaluateTestFile(content);
    }));
}

async function spawnBrowsers (spawnInfo) {
    return Promise.all(spawnInfo.map(async info => {
        var browser = await spawnBrowser(info);

        await Promise.race([browserConnection(browser), timeout()]);
    }));
}

async function executeTask (tests, browsers) {
    for (const test of tests)
        await test.run();

    await browsers.close();
}

As you can see we will need only one small piece of code to make whole async function cancellable nearly on every step. I bet I'm missing something, but this approach come to my mind and I decided to share it with you.

[domenic]

Hmm. This is certainly interesting. I don't think I fully understand how to map between your example using * syntax and the example using cancel tokens more manually, though. For example, in your last example you do not pass the cancel token from runTests to executeTask.I can't see any way to make that work implicitly. (Unless you are saying all function calls inside a function with a * in its parameter list automatically get an extra argument passed to them!?)

I'm still generally fairly skeptical here, but if you could provide something that is more obviously a desugaring, it would be worth looking in to...

[inikulin]

I'm still generally fairly skeptical here, but if you could provide something that is more obviously a desugaring, it would be worth looking in to.

Yeah, I think I'll better provide POC and testbed to both clarify my idea and settle things in my own head. I hope I will have time this weekend.

[inikulin]

Sorry for the delay with this one. Have a really busy week :disappointed_relieved:

[bergus]

@inikulin Actually passing these "cancelTriggers" as an argument to the then call is exactly what my new proposal is doing with tokens. Feedback welcome!

[inikulin]

Oh, I completely forgot to comment about it. After some research I've figured out that "cancelTriggers" will break immutability, so it's not a case.

@domenic Regarding original request, I've seen await.cancelToken proposal somewhere in the issues here and this is exactly what I wanted.

[bergus]

@inikulin Can you expand on "After some research I've figured out that "cancelTriggers" will break immutability", please? The promises in my proposal are pretty much immutable, as you cannot change the token associated to them (that cancels them).

[inikulin]

@bergus

var p1 = runTaskChain1();
var p2 = runTaskChain2();

var trigger = new CancelTrigger();
p1.cancelTrigger = trigger;

var p3 = p2.then(() => doSomething());

p1.then(() => p2); // Now p2 can be canceled with the `trigger` 

cancelTrigger.request(); 
// Since now we can cancel p2 at some point, p3 will never be resolved, 

[bergus]

@inikulin Uh, I must have missed something. Nowhere in your posts above were you assigning a .cancelTrigger property to a promise/task instance? Also I don't understand why "Now p2 can be canceled with the trigger". Should the snippet desugar to

p1.then(() => {
    p1.cancelTrigger.throwIfCancel();
    return p2;
})

so that p2 might be prevented from being evaluated, but is never canceleld itself?

I meant the examples from this comment. With my proposal, the code would look like this:

var root = new Promise((resolve, reject) => {...});

function doSmthgWhenRootComplete (cancelToken) {
    return root
        .then(doSomething, null, cancelToken)
        .then(doSomethingElse, null, cancelToken)
        .then(andElse, null, cancelToken);
}

const {token, cancel} = CancelToken.source();
doSmthgWhenRootComplete(token);
// later:
cancel();

Here, root is never cancelled, but none of doSomething, doSomethingElse, andElse will be executed if the token is cancelled. I have to admit I missed that you were passing {cancelTrigger} only to the first then invocation and expected it to prevent any callbacks in the whole chain (whereas in my proposal you need to pass it alongside of every handler), but it seems the idea is the same. And as you stated:

With this approach we will have immutable promises due to down propagation, but behavior which is very close to promise.cancel()

[inikulin]

I have to admit I missed that you were passing {cancelTrigger} only to the first then invocation and expected it to prevent any callbacks in the whole chain (whereas in my proposal you need to pass it alongside of every handler), but it seems the idea is the same.

But unfortunately it's not solve problem discussed here, then.

[bergus]

unfortunately it's not solving the problem discussed here, then.

Hm, I still believe that passing the token alongside the callback is much much better than having to call token.throwIfCancel(); inside every callback (which is especially cumbersome when it introduces an otherwise not necessar function expression). Yes, for cancelling a whole chain with only one token we need mightier weapons

[domenic]

Excellent, I'll close this in favor of #23 then.