Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

Major refactor of error handling and API #6

Merged
merged 0 commits into from Aug 22, 2014

Conversation

Projects
None yet
3 participants
Contributor

AndreasMadsen commented Mar 23, 2013

I realize that this is a very big change and understand if you don't want to merge it, just tell and I will publish my own module.

The issues that I found is:

  • If a request fails an error is throw in another tick, that makes it very difficult to catch
  • Some error are strings (http://www.devthought.com/2011/12/22/a-string-is-not-an-error/)
  • The callback is not the last argument, that is generally considered an antipattern in node.
  • The API lacks separation between the YQL parameters and request options
  • The internal logic do not handle error reports, this is not always a bad thing but something I would normally expect from a pure client.
  • The callback do not have an error argument, this is also an antipattern in node.
Owner

derek commented Mar 30, 2013

Hey Andreas, just came across this PR. I'll take a look at it in the next few days and get back to you. Thanks!

Contributor

emkay commented Jun 5, 2013

Hey @derek any update on this PR?

Owner

derek commented Jun 5, 2013

@emkay Whoops. This was filed when I was on vacation, so it pretty quickly escaped my attention. Looks like a nice contribution though. I'll pull it in.

@AndreasMadsen Thanks again!

@derek derek merged commit 82b254c into derek:master Aug 22, 2014

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment