Better Node.JS conventions for errors

[vvo]

Hi,

It seems most methods from this module are behaving this way:

index.saveObject(obj, cb);

function cb(err, res) {
  // success: err = falsy, res = algoliasearch response
  // error: err = truthy, res = actual Error object
}

While in Node.JS, most of the modules, including core API, will do it this way:

index.saveObject(obj, cb);

function cb(err, res) {
  // success: err = falsy, res = algoliasearch response
  // error: err = Error object, res = undefined
}

There are many reasons to do it the Node.JS way like not messing up with Node.JS developers interested in your product and showing them you know Node.JS!

But another interesting usecase is that algoliasearch-client-node would be Promiseable.

Promises are a great way to handle asynchronous flow control in JavaScript and will be the de-facto way to handle complex asynchronous code very soon for most JavaScript developers.

Right now we cannot easily wrap algoliasearch-client-node with Promises because the error object.

Using https://github.com/then/promise which is a Promise polyfill we should be able to:

var saveObject = Promise.denodeify(index.saveObject.bind(index));

saveObject(obj)
  .then(fn) // then called when everything is ok
  .catch(fn) // catch called when first param is an Error object

At the moment, doing this will result in having catch called with "true" and you can't have any other info on the error.

Note that my usecase is not the point, following Node.JS conventions on errors is the point.

Thanks

[speedblue]

You are right, we will change it to follow conventions and keep backward compatibility.
The best approach is probably to return the error object in both err & res attribute of the callback. In this case it would be backward compatible and follow the convention, is it correct?

[vvo]

yes it would do the trick and breaks nothing.

not common to have two Err in the err callback but no big deal it won't break.

[vvo]

the only case where it can break is if people do if err === true.

Also you can always bump major and people upgrade to major if they want to. It all depends on the number of clients using the module right now.

I understand that a business-related node module should try to not break compat yes.

[vvo]

We solved this with the release of https://github.com/algolia/algoliasearch-client-js V3