Task.fromNodeBack is not handling cancellation

[cdoublev]

All of the Folktale task primitives should be dealing with cancellation.

Task.fromNodeBack currently doesn't prevent resolving|rejecting when the task is cancelled.

Steps to reproduce

Given:

const Task = require('folktale/concurrency/task')

const delay = Task.fromNodeback((result, error, cb) =>
    setTimeout(() => cb(error ? result : null, result)), 0)

This works:

Task.waitAll([delay(1, false), delay(2, true)])
   .orElse(e => Task.rejected(console.log('Error: ', e)))
   .run() // => Error 2

And this don't:

Task.waitAll([delay(1, false), delay(2, true), delay(3, true)])
   .orElse(e => Task.rejected(console.log('Error: ', e)))
   .run() // => Error 2
  // throw new Error('Only pending deferreds can be ' + description + ', this deferred is already ' + description + '.');

Expected behaviour

Last example shouldn't throw an error (Task.fromNodeback should check if task is already cancelled).

Observed behaviour

The error Only pending deferreds can be ' + description + ', this deferred is already ' + description + '.' is thrown.

Environment

• OS: Vagrant Debian Box
• JavaScript VM: node v11.6.0
• Folktale version: 2.3.1

Additional information

Related issue (comment): #153 (comment)
Related file: https://github.com/origamitower/folktale/blob/master/packages/base/source/conversions/nodeback-to-task.js#L25

[robotlolita]

@cdoublev can you install the beta release and check if it works for you?

$ npm install folktale@next

[cdoublev]

Yes, my own (rejection) tests stay green (without yelling about uncaught errors) after updating with @next and reverting all waitAll or and() usages from .apply(). Thank you for fixing/completing my tests. It perfectly answers my question asked in this comment. 👍