fork with fixes, up-to-date queue, improved error handling

[kapouer]

Hi, i've started a fork to mainly improve error handling (with tests) https://github.com/kapouer/chainit

also with updated dependency on queue 3.

There are some changes that you might not like:

• removal of timeout for adding depth-0 tasks, see commits for details
• removal of specific code that has nothing to do with chainit itself (i.e. webdriver conditions)
• removal of custom error messages (not really needed since error handling is better)

Feel free to merge/cherry-pick commits ! Jérémy.

[kapouer]

Hello again, chainit3@3.1.0 somewhat now provides a way to carry on a state object along the calls. This allows stuff like .load().wait('ready').html(cb).unload().load().wait('idle') to work. Checkout webkitgtk for an example usage.

[christian-bromann]

@kapouer thanks dude!

I guess WebdriverIO needs the specific conditions to work properly (customCb and custom error messages). We tried once to update to v2 but reverted since it seems to be really bad when it comes to performance. Did you experienced any performance leaks so far?

[kapouer]

I've removed suspicious uses of setTimeout - i don't see why there would be performance issues. Here's some example of a (simple) state machine https://github.com/kapouer/node-webkitgtk/blob/master/webkitgtk.js#L28

[vvo]

Hi @kapouer I missed all your commits and comments because I was not watching this repository. Next time try to mention @vvo it works well.

You seem to really have nailed it.

I think the statemachine could be in hiw own project. I don't understand why we need it but ok.

Here's what we can do:

• I give you push access to this repository on both github and npm
• You bump chainit to 3.0 and we merge projects

What do you think?

You have done an amazing work. I worked hard on this library but somehow got lost at some point, writing async flow control libraries is a brain heater !

removal of timeout for adding depth-0 tasks, see commits for details

Can't find the commit

[vvo]

Here you go, you have push access to chainit on both github and npmjs.

Why not submit a big PR and then you can bump to 3.x bye

[kapouer]

Hi, cool, thank you. I agree to the merge.

The state machine is a simple thing actually, and it allows https://github.com/kapouer/node-webkitgtk/commit/b129601a5e56e664d0b916362b86e50772f5aedf to take care of the order in which methods are called. A bit tricky but on the chainit side it's generic code - not tied to my module at all.

removal of timeout for adding depth-0 tasks, see commits for details https://github.com/kapouer/chainit3/commit/cdc65b772d93dbf48696733222fb0a2ddac8959a

[vvo]

@kapouer

obj
  .method1()
  .methodError()
  .notactuallycalled(function(err) {
    // the error that happened in methodError
    // but the method "notactuallycalled" is not called !
    console.error(err);
  });

This is a bit tricky and not the best way to do it.

We can't be in the notactuallycalled callback and say that the method was not called. Too hard to grasp on a regular basis.

I do ont have a solution right now for this.

[kapouer]

Agreed - the semantic is wrong, but it's actually very useful - you are assured the last handler get the error. I like that tradeoff very much.

[kapouer]

Maybe adding a tail function like:

obj
  .method1()
  .methodError()
  .notactuallycalled()
  .finished(function(err) {
    // the error that happened in methodError
    // the method "notactuallycalled" is not called
    console.error(err);
  });

[vvo]

Yes but it means .finished can't be a method from the API.

What do you think of: chainit.on(obj, 'error', fn)?

[kapouer]

if that allows to define a "catchall" method on the chainable object instance, that's good enough. BTW i can't think of any other meaningful scenario... maybe simply allowing a custom method name for catching errors is enough ?

[kapouer]

I'm not so fund of the catchall method. To me, chainable without callback is like a one method call: .call(that).or().do(it, cb) has the same semantics as .allinone(that, it, cb) Not providing a callback for the .call() method means i don't care what happens here, just do it as far as it is possible. Another way to see this is to consider .call().or().do = myfunc so of course, any error happening in call or or just propagates to the callback of myfunc. From a user point of view, it also is very handy, for it makes chainability and keep the ability to deal with errors in the usual idiomatic way - and in one place.

[vvo]

I am not sure I understand all what you are saying.

I get you do not like the catchall method but then I don't see if you are proposing another solution.

What I can say is only that

obj
  .method1()
  .methodError()
  .notactuallycalled(function(err) {
    // the error that happened in methodError
    // but the method "notactuallycalled" is not called !
    console.error(err);
  });

Should not be, any callback inside a method means the outer method was called. Right?

[kapouer]

Not necessarily right... what i explain is that chainable API comes with different semantics than plan old methods. Either it's chainable API or it falls down to another category - promise-like API.

[vvo]

chainit(myApi, {
  error: fn
});

?

[kapouer]

With that you catch the error all right, but not in the place you usually do. Usually you get errors in a callback, not in a listener.

My point is that when you write

obj.method1(params1).method2(params2, cb)

You actually mean

obj.method12(params1, params2, cb)

So if there was an error in method1, you really expect it to come back in cb(err). Wether method2 was called or not has no importance at all here, the callback you provide is a way out of the current chain. If you need a more detailed failure handling, you just have to register a callback for each method you call, like

obj.method1(cb1).method2(cb2)

With your proposal, you have no info about where it failed, while in this design, you actually control where you want to handle failures (after each methods, after key methods, at the end, etc...), and you handle them in the same code where you register your callbacks, not in some other place.

[vvo]

If you do not catch errors in method1 it means you do not care about them, thus you get an error in the global error callback.

method2 callback is method2 callback, it should not be method1 callback "when there's an error" and method2 callback when everything is fine. That's way too complex, it might fits your needs but it's hard to grasp and even harder to explain.

About my proposal: you do have info where it failed, at least you can have info, it all depends the error submitter, how it does to send an error.

[kapouer]

Well that's probably why i did fork on this : i don't think it's too complex - on the contrary, i think it's simpler ! In your proposal you have to setup an additionnal listener that stands out of the code where it is used. If you don't you can simply miss errors (or you find them logged on console, which is not very nice either). In what i propose, whatever you do you get the errors in the callback - you don't have to think about it. You should try it in your code, it is really simpler to deal with in userland. But i agree to disagree on this, let's not merge this into vvo/chainit and i'll keep my fork maintained separately. That doesn't prevent several commits to be merged of course.

[vvo]

If nobody listen for the global error and you did not catch the error (no callback) you can always throw the error.

I understand you prefer dealing with errors the way you did in chainit3.