Merge pull request #50 from machty/better-cancelation

No longer handle cancelation in catch blocks, add didCancel

addon/-task-instance.js

@@ -1,6 +1,31 @@
 import Ember from 'ember';
 import { createObservable, yieldableSymbol } from './utils';
 
+const TASK_CANCELATION_NAME = 'TaskCancelation';
+
+/**
+ * Returns true if the object passed to it is a TaskCancelation error.
+ * If you call `someTask.perform().catch(...)` or otherwise treat
+ * a {@linkcode TaskInstance} like a promise, you may need to
+ * handle the cancelation of a TaskInstance differently from
+ * other kinds of errors it might throw, and you can use this
+ * convenience function to distinguish cancelation from errors.
+ *
+ * ```js
+ * click() {
+ *   this.get('myTask').perform().catch(e => {
+ *     if (!didCancel(e)) { throw e; }
+ *   });
+ * }
+ * ```
+ *
+ * @param {Object} error the caught error, which might be a TaskCancelation
+ * @returns {Boolean}
+ */
+export function didCancel(e) {
+  return e && e.name === TASK_CANCELATION_NAME;
+}
+
 function forwardToInternalPromise(method) {
   return function(...args) {
     this._userWillHandlePromise = true;
@@ -21,6 +46,10 @@ const SUCCESS     = "success";
 const ERROR       = "error";
 const CANCELATION = "cancel";
 
+const RESUME_NEXT   = "next";
+const RESUME_THROW  = "throw";
+const RESUME_RETURN = "return";
+
 /**
   A `TaskInstance` represent a single execution of a
   {@linkcode Task}. Every call to {@linkcode Task#perform} returns
@@ -206,7 +235,7 @@ let taskInstanceAttrs = {
     }
 
     let error = new Error("TaskCancelation");
-    error.name = "TaskCancelation";
+    error.name = TASK_CANCELATION_NAME;
     error.taskInstance = this;
 
     this._finalize(error, CANCELATION);
@@ -215,7 +244,7 @@ let taskInstanceAttrs = {
       // eagerly advance index so that pending promise resolutions
       // are ignored
       this._index++;
-      this._proceed(this._index, error, 'throw');
+      this._proceed(this._index, error, RESUME_RETURN);
     }
   },
 
@@ -257,7 +286,13 @@ let taskInstanceAttrs = {
 
   _hasResolved: false,
 
-  _finalize(value, completion) {
+  _finalize(value, _completion) {
+    let completion = _completion;
+
+    if (didCancel(value)) {
+      completion = CANCELATION;
+    }
+
     this.set('isFinished', true);
 
     switch (completion) {
@@ -312,7 +347,7 @@ let taskInstanceAttrs = {
   _takeStep(index, nextValue, method) {
     if (index !== this._index) { return; }
 
-    let { done, value, error } = this._takeSafeStep(nextValue, method || 'next');
+    let { done, value, error } = this._takeSafeStep(nextValue, method || RESUME_NEXT);
 
     if (error) {
       this._finalize(value, ERROR);
@@ -334,7 +369,11 @@ let taskInstanceAttrs = {
     this._disposable = observable.subscribe(v => {
       this._proceedOrFinalize(done, index, v);
     }, error => {
-      this._proceed(index, error, 'throw');
+      if (didCancel(error)) {
+        this._proceed(index, error, RESUME_RETURN);
+      } else {
+        this._proceed(index, error, RESUME_THROW);
+      }
     }, () => {
       // TODO: test, and figure out what it means to yield
       // something that completes without producing a value.

addon/index.js

@@ -1,6 +1,7 @@
 import Ember from 'ember';
 import { isGeneratorIterator, createObservable } from './utils';
 import { TaskProperty } from './-task-property';
+import { didCancel } from './-task-instance';
 import { TaskGroupProperty } from './-task-group';
 import EventedObservable from './-evented-observable';
 import { subscribe } from './-subscribe';
@@ -131,6 +132,7 @@ export {
   enqueue,
   maxConcurrency,
   cancelOn,
-  performOn
+  performOn,
+  didCancel
 };
 

tests/dummy/app/components/my-button/component.js

@@ -1,5 +1,7 @@
 import Ember from 'ember';
 
+import { didCancel } from 'ember-concurrency';
+
 export default Ember.Component.extend({
   click() {
     let val = this.attrs.action(3, 4);
@@ -9,7 +11,7 @@ export default Ember.Component.extend({
         throw new Error("returned value wasn't 10");
       }
     }).catch(e => {
-      if (e.name !== 'TaskCancelation') {
+      if (!didCancel(e)) {
         throw e;
       }
     });

tests/unit/error-handling-test.js

@@ -56,9 +56,8 @@ test("parent task canceled by restartable policy: no errors", function(assert) {
 });
 
 test("parent task perform attempt canceled by drop policy: no errors", function(assert) {
-  assert.expect(5);
+  assert.expect(1);
 
-  let childCancelationCount = 0;
   let childDefer;
   let Obj = Ember.Object.extend({
     parent: task(function * () {
@@ -70,8 +69,7 @@ test("parent task perform attempt canceled by drop policy: no errors", function(
       try {
         yield childDefer.promise;
       } catch(e) {
-        assert.equal(e.name, 'TaskCancelation');
-        childCancelationCount++;
+        assert.ok(false);
       }
     }),
   });
@@ -82,17 +80,14 @@ test("parent task perform attempt canceled by drop policy: no errors", function(
     obj.get('parent').perform(1);
   });
   assert.ok(childDefer);
-  assert.equal(childCancelationCount, 0);
 
   Ember.run(() => {
     obj.get('parent').perform(2);
   });
-  assert.equal(childCancelationCount, 0);
 
   Ember.run(() => {
     obj.get('parent').cancelAll();
   });
-  assert.equal(childCancelationCount, 1);
 });
 
 

tests/unit/task-instance-test.js

@@ -341,8 +341,8 @@ test("yielding to other tasks: child task gets canceled", function(assert) {
           args: [],
         })._start();
         taskInstance1.then(shouldNotGetCalled);
-        let value = yield taskInstance1;
-        assert.equal(value, 123);
+        yield taskInstance1;
+        assert.ok(false);
       },
       args: [],
     })._start();
@@ -355,7 +355,7 @@ test("yielding to other tasks: child task gets canceled", function(assert) {
     assert.equal(e.taskInstance, taskInstance1);
   }
 
-  assert.equal(taskInstance0.get('state'), 'finished'); // TODO: this is up for debate
+  assert.equal(taskInstance0.get('state'), 'canceled');
   assert.equal(taskInstance1.get('state'), 'canceled');
 
   Ember.run(null, defer.resolve, "naw");
@@ -578,5 +578,48 @@ test("in a hierarchy of child task performs, a bubbling cancel should not be con
   Ember.run(taskInstance0, 'cancel');
 });
 
+test("task cancelation should skip over catch blocks within task functions", function(assert) {
+  assert.expect(1);
+
+  let taskInstance0;
+  Ember.run(() => {
+    TaskInstance.create({
+      fn: function * () {
+        try {
+          yield TaskInstance.create({
+            fn: function * () {
+              try {
+                taskInstance0 = TaskInstance.create({
+                  fn: function * () {
+                    try {
+                      yield Ember.RSVP.defer().promise;
+                      assert.ok(false, "one");
+                    } catch(e) {
+                      assert.ok(false, "one catch");
+                    }
+                  },
+                  args: [],
+                })._start();
+                yield taskInstance0;
+                assert.ok(false, "two");
+              } catch(e) {
+                assert.ok(false, "two catch");
+              }
+            },
+            args: [],
+          })._start();
+          assert.ok(false, "three");
+        } catch(e) {
+          assert.ok(false, "three catch");
+        }
+      },
+      args: [],
+    })._start().catch(e => {
+      assert.equal(e.name, 'TaskCancelation');
+    });
+  });
+
+  Ember.run(taskInstance0, 'cancel');
+});