Task.chain cancellation semantics

[javamonn]

I have a series of tasks that are chained together so that they execute on after another. At some point, I may wish to stop the execution of the series of tasks, so I call cancel on the TaskExecution object I get from calling run on the chained tasks. As expected, the onCancelled callback fires on the listener attached to the TaskExecution object. However, The onCancelled callback on the tasks themselves don't appear to fire, and the tasks continue to execute.

Steps to reproduce

https://runkit.com/javamonn/5a65014051c1e2001226dd28

Expected behaviour

When I call cancel on the ExecutionObject of several tasks that have been chained together and ran, I would expect the onCancelled callback of the currently executing task in that series to be called or isCancelled to be set, and that tasks that are chained later in the series to not execute at all, as the task did not resolve successfully.

Observed behaviour

You can see the attached logs on the runkit example. When I call cancel on the ExecutionObject, the attached listener fires the onCancelled object correctly, but the task series continues executing, onCancelled is not called and isCancelled is not set, and the currently executing task at the time of cancellation ends up throwing an error when it (incorrectly) tries to resolve itself.

Environment

N / A, attached runkit.

Additional information

I'm not sure if this is simply a misunderstanding of the semantics of cancelling a series of tasks that are chained together. If so, I apologize for the spurious issue!

[robotlolita]

The semantics aren't well defined (which is not a good thing, I have to take the time to define them properly), but cancelling the execution should cancel all of the pending tasks in a chain. There seems to be an error in the propagation when part of the chain is already settled, though.

If the cancellation happens before task A settles, then all tasks in the chain are cancelled. If task A settles first, then cancellation of all other tasks in the chain is skipped.

I'll take a better look at why this is happening in February, since I have to finish preparing a talk for a conference next week.

[yelouafi]

@robotlolita

In the chain method

https://github.com/origamitower/folktale/blob/81e13a0060f5b9524114036ccfe4a47dc22ae736/packages/base/source/concurrency/task/_task.js#L46

I think you need to sync the value of execution with the currently executing task since it's no longer valid once the first task is resolved

chain(transformation) {
    return new Task(resolver => {
-     const execution = this.run();
+    let execution = this.run();
      resolver.onCancelled(() => execution.cancel());

      execution.listen({
        onCancelled: resolver.cancel,
        onRejected:  resolver.reject,
        onResolved:  value => {
-         transformation(value).run().listen({
+        execution = transformation(value).run()
+        execution.listen({
            onCancelled: resolver.cancel,
            onRejected:  resolver.reject,
            onResolved:  resolver.resolve
          });
        }
      });
    });
  }

[robotlolita]

@yelouafi it's a bit more complicated than that because propagation of cancellation isn't a concept yet in tasks, and users should be able to use tasks without having to worry about these low-level details.

(the execution is always valid, btw, and onCancelled isn't called if the task is already settled. This is part of the problem)

[robotlolita]

@javamonn Can you try upgrading to 2.2.0-alpha1 and see if it solves your problem? npm i folktale@next should do it.

[javamonn]

@robotlolita 2.2.0-alpha1 does resolve the issues I was seeing. Thanks for the patch!