Consider changing callbacks to (err, result)?

[aseemk]

Hey there,

I'm new to this library, so apologies if this has been asked to death or is by design -- didn't find anything in a search besides #15 which briefly mentions it.

This library would play a lot nicer with Node async flow management tools and libraries if its callbacks followed the standard (err, result) convention. Have you considered that?

I know it'd be a backwards-incompatible change, so a major version bump would be warranted, but, without knowing history or context, seems like it would be a great change to make.

So to clarify, err would have to be null for all successful responses, and non-null for successful ones. Perhaps the result could get a statusCode property (or e.g. _statusCode) added to it if knowing the status code is helpful even in non-error cases.

What do you think? Thanks for consideration and great work otherwise!

[sintaxi]

I have considered this. Most of my libraries follow the error-first convention.

The problem is error-first does not translate to HTTP very well because it assumes only two possible outcomes and using it would make the application code much more challenging to write because without the status code you do not have content about what the problem is.

Imagine you are trying to make an operation against the API and you are not successful. With error-first you can only assume what the reason for the error is. Most would assume the reason is a validation message (400) that something in the input is wrong, when in reality it could be a number of different things. Here are the possible status codes...

400 Bad input parameter. Error message should indicate which one and why.
401 Bad or expired token. This can happen if the user or Dropbox revoked or expired an access token. To fix, you should re-authenticate the user.
403 Bad OAuth request (wrong consumer key, bad nonce, expired timestamp...). Unfortunately, re-authenticating the user won't help here.
404 File or folder not found at the specified path.
405 Request method not expected (generally should be GET or POST).
503 Your app is making too many requests and is being rate limited. 503s can trigger on a per-app or per-user basis.
507 User is over Dropbox storage quota.
5xx Server error. Check our ops Twitter feed @DropboxOps.

With error-first paradigm the application would have to somehow be able to detect the reason for the error and without the status code this very challenging. Dbox is designed to take away the ceremony of HTTP not remove HTTP out of the picture. API requests have a body, and status code gives you context of what that body represents. I think that is as simple as it can be made while still being a reliable tool.

[aseemk]

I definitely understand. I've used some libraries that have the error value convey the same information, e.g.:

var err = new Error(resp.statusCode + ' ' + resp.message);
err.statusCode = resp.statusCode;

Are you open to something like that?

[respectTheCode]

This would be a big improvement. As it is now we can't use libraries like async with dbox. What about error, status, results?

[jstroem]

hmm what about just put the status in the results so you have results = {status: '...', response: '...'} then there is always space for more as well without changing the cb-function stucture but only the argument giving to it?

[jeremycondon]

Why not write a quick wrapper class that implements the return codes the way you want them?

Thanks, Jeremy