Improve correctness of Future.hook

Closes #234

README.md

@@ -1347,31 +1347,22 @@ withConnection(
 .fork(console.error, console.log);
 ```
 
-In the case that a hooked Future is *cancelled* after the resource was acquired,
-`dispose` will be executed and immediately cancelled. This means that rejections
-which may happen during this disposal are **silently ignored**. To ensure that
-resources are disposed during cancellation, you might synchronously dispose
-resources in the `cancel` function of the disposal Future:
-
-<!-- eslint-disable no-unused-vars -->
-```js
-var closeConnection = conn => Future((rej, res) => {
-
-  //We try to dispose gracefully.
-  conn.flushGracefully(err => {
-    if(err === null){
-      conn.close();
-      res();
-    }else{
-      rej(err);
-    }
-  });
-
-  //On cancel, we force dispose.
-  return () => conn.close();
-
-});
-```
+When a hooked Future is cancelled while acquiring its resource, nothing else
+will happen. However, once the resource has been acquired, the `dispose`
+Future is guaranteed to run.
+
+In the case where the consumption Future settles by resolving, rejecting the
+disposal is forked and awaited. If the disposal Future rejects or encounters
+an exception, it will replace the state of the consumption Future. Otherwise
+rejection reason or resolution value of the consumption Future will determine
+the final outcome.
+
+In the case where an exception is encountered during consumption, the disposal
+Future will run, but its results are ignored.
+
+In the case where the consumption or disposal Future is cancelled from outside,
+the disposal Future will start and/or finish its computation, but the results
+will be ignored.
 
 #### finally
 

src/hook.js

@@ -1,3 +1,5 @@
+/* eslint no-param-reassign:0 */
+
 import {Core, isFuture} from './core';
 import {noop, show, showf, partial1, partial2} from './internal/fn';
 import {isFunction} from './internal/is';
@@ -33,22 +35,30 @@ Hook.prototype = Object.create(Core);
 Hook.prototype._interpret = function Hook$interpret(rec, rej, res){
 
   var _this = this, _acquire = this._acquire, _dispose = this._dispose, _consume = this._consume;
-  var cancel, cancelAcquire = noop, cancelConsume = noop, resource, value, cont = noop;
+  var cancel, cancelConsume = noop, resource, value, cont = noop;
 
   function Hook$done(){
     cont(value);
   }
 
+  function Hook$reject(x){
+    rej(x);
+  }
+
   function Hook$consumptionException(e){
-    rec(someError('trying to consume resources for a hooked Future', e, _this.toString()));
+    var rec_ = rec;
+    cont = noop;
+    rej = noop;
+    rec = noop;
+    Hook$dispose();
+    rec_(someError('trying to consume resources for a hooked Future', e, _this.toString()));
   }
 
   function Hook$disposalException(e){
     rec(someError('trying to dispose resources for a hooked Future', e, _this.toString()));
   }
 
   function Hook$dispose(){
-    cancel = noop;
     var disposal;
     try{
       disposal = _dispose(resource);
@@ -58,13 +68,20 @@ Hook.prototype._interpret = function Hook$interpret(rec, rej, res){
     if(!isFuture(disposal)){
       return Hook$disposalException(invalidDisposal(disposal, _dispose, resource));
     }
-    cancel = disposal._interpret(Hook$disposalException, rej, Hook$done);
+    disposal._interpret(Hook$disposalException, Hook$reject, Hook$done);
+    cancel = Hook$cancelDisposal;
   }
 
-  function Hook$cancelConsuption(){
+  function Hook$cancelConsumption(){
     cancelConsume();
     Hook$dispose();
-    cancel();
+    Hook$cancelDisposal();
+  }
+
+  function Hook$cancelDisposal(){
+    cont = noop;
+    rec = noop;
+    rej = noop;
   }
 
   function Hook$consumptionRejected(x){
@@ -81,7 +98,6 @@ Hook.prototype._interpret = function Hook$interpret(rec, rej, res){
 
   function Hook$consume(x){
     resource = x;
-    cancel = Hook$cancelConsuption;
     var consumption;
     try{
       consumption = _consume(resource);
@@ -91,15 +107,15 @@ Hook.prototype._interpret = function Hook$interpret(rec, rej, res){
     if(!isFuture(consumption)){
       return Hook$consumptionException(invalidConsumption(consumption, _consume, resource));
     }
+    cancel = Hook$cancelConsumption;
     cancelConsume = consumption._interpret(
       Hook$consumptionException,
       Hook$consumptionRejected,
       Hook$consumptionResolved
     );
   }
 
-  cancelAcquire = _acquire._interpret(rec, rej, Hook$consume);
-
+  var cancelAcquire = _acquire._interpret(rec, Hook$reject, Hook$consume);
   cancel = cancel || cancelAcquire;
 
   return function Hook$fork$cancel(){ cancel() };

test/5.hook.test.js

@@ -70,11 +70,11 @@ describe('hook()', function(){
     });
 
     it('crashes when the second function throws', function(){
-      var m = hook(F.resolved, function(){ throw F.resolved }, function(){ throw U.error });
+      var m = hook(F.resolved, function(){ return F.resolved }, function(){ throw U.error });
       return U.assertCrashed(m, new Error(
         'Error came up while trying to consume resources for a hooked Future:\n' +
         '  Intentional error for unit testing\n\n' +
-        '  In: Future.hook(Future.of("resolved"), function (){ throw F.resolved }, function (){ throw U.error })\n'
+        '  In: Future.hook(Future.of("resolved"), function (){ return F.resolved }, function (){ throw U.error })\n'
       ));
     });
 
@@ -152,25 +152,27 @@ describe('hook()', function(){
       setTimeout(cancel, 25);
     });
 
-    it('cancels dispose appropriately', function(done){
-      var dispose = Future(function(){ return function(){ return done() } });
+    it('does not cancel disposal', function(done){
+      var dispose = Future(function(){ return function(){ return done(U.error) } });
       var cancel =
         hook(F.resolved, function(){ return dispose }, function(){ return of('consume') })
         ._interpret(done, U.failRej, U.failRes);
       setTimeout(cancel, 10);
+      setTimeout(done, 50);
     });
 
-    it('cancels delayed dispose appropriately', function(done){
-      var dispose = Future(function(){ return function(){ return done() } });
+    it('does not cancel delayed dispose', function(done){
+      var dispose = Future(function(){ return function(){ return done(U.error) } });
       var cancel =
         hook(F.resolved, function(){ return dispose }, function(){ return F.resolvedSlow })
         ._interpret(done, U.failRej, U.failRes);
-      setTimeout(cancel, 25);
+      setTimeout(cancel, 50);
+      setTimeout(done, 100);
     });
 
-    it('immediately runs and cancels the disposal Future when cancelled after acquire', function(done){
+    it('runs the disposal Future when cancelled after acquire', function(done){
       var cancel =
-        hook(F.resolved, function(){ return Future(function(){ return function(){ return done() } }) }, function(){ return F.resolvedSlow })
+        hook(F.resolved, function(){ return Future(function(){ done() }) }, function(){ return F.resolvedSlow })
         ._interpret(done, U.failRej, U.failRes);
       setTimeout(cancel, 10);
     });