bagpipes.flow(..) - support last argument - callback(err,ctx)

[osher]

Hi.

This PR is not ready - I need to _discuss_ with you few points first.

I created this branch to create a PR from it, I cloned it and I wanted to add tests.

Then I saw this inside, which completely confused me:

  it('should throw errors if no onError handler', function(done) {
    var userFittingsDirs = [ path.resolve(__dirname, './fixtures/fittings') ];
    var pipe = [ 'error' ];
    var bagpipes = Bagpipes.create({ pipe: pipe }, { userFittingsDirs: userFittingsDirs });
    var context = {};
    (function() {
      bagpipes.play(bagpipes.getPipe('pipe'), context)
    }).should.throw.error;
    done();
  });

The part that confused me is should.throw.error. IMHO - there's trouble.

The troubles can be tested if you change test/fixtures/fittings/emit.js
from:

'use strict';

module.exports = function create() {
  return function emit(context, cb) {
    cb(null, 'test');
  }
};

to:

'use strict';

module.exports = function create() {
  return function emit(context, cb) {
    process.nextTick(function() {
      cb(null, 'test');
    })
  }
};

In this case - the tests that come with the repo fail, which suggests that as tests they do not represent real-world usage, and that's pitty 😞

What would you propose?

[osher]

I committed tests I started to add - I have a problem with the last case:

   when provided with a done(err,ctx) callback
      and the pipe flow succeeds
        √ should pass context to the done
      and the pipe flow fails
        1) should pass context to the done

That should test line 69 in:

53| Bagpipes.prototype.play = function play(pipe, context, done) {
54| 
55|   if (typeof context === 'function') {
56|      done = context;
57|      context = undefined;
58|  }
59|  assert(typeof context === 'undefined' || typeof context === 'object', 'context must be an object');
60|  assert(typeof done === 'undefined' || typeof done === 'function' && done.length == 2, 'end-61| callback must be a function that expects 2 argument - error, and the context');
61|  
62|  if (!context) { context = {}; }
63|
64|  var pw = pipeworks();
65|  pw.fit(function(context, next) { pipe.siphon(context, next); });
66|  if (context._finish) { pw.fit(context._finish); }
67|  if (done) { 
68|      pw.fit( function done_success(ctx, _) { done(null, ctx) } );
69|      pw.fault( function done_fault(ctx, err) { done (err, ctx) } );
70|  }
71|  
72|  pw.flow(context);
73|};

Any advise will be appreciated

[osher]

I hope I found the cause - Pipeworks#siphon does not pass the faultPipe.

kevinswiber/pipeworks#10

[theganyo]

Aha. I see this is probably in response to your issue here: #13. (And my issue here: #11.)

I like this. Simple and direct. Hopefully the pipeworks issue can be addressed and we can move forward on this.

[osher]

Did you see Kevin's answer and question to you?
kevinswiber/pipeworks#10 (comment)