Skip to content
New issue

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

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Basic error handling for API responses added #7

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

inadarei
Copy link

I saw that error-handling for the project is still TBD.

This pull request does not handle all and any kind of errors, but at least it throws some additional info if the API responds with HTTP 400 or 404 or 500.

Basically, I had a malformed list of init parameters and was getting "calling .map() on undefined" instead of a helpful message, so I think it would help in similar cases :)

If you could merge the pull and re-publish npm, it would be super!

Thanks for the project!

@seebees
Copy link
Owner

seebees commented Mar 10, 2013

Let me take a look. I'll do some wirk and merge stuff tomorow.
On Mar 9, 2013 7:30 PM, "Irakli Nadareishvili" [email protected]
wrote:

I saw that error-handling for the project is still TBD.

This pull request does not handle all and any kind of errors, but at least
it throws some additional info if the API responds with HTTP 400 or 404 or
500.

Basically, I had a malformed list of init parameters and was getting
"calling .map() on undefined" instead of a helpful message, so I think it
would help in similar cases :)

If you could merge the pull and re-publish npm, it would be super!

Thanks for the project!

You can merge this Pull Request by running

git pull https://github.com/inadarei/ironmq master

Or view, comment on, or merge it at:

#7
Commit Summary

  • Adding some basic error-handling for API requests

File Changes

Patch Links:

@inadarei
Copy link
Author

Awesome! Many thanks.

@seebees
Copy link
Owner

seebees commented Mar 16, 2013

@inadarei So the only thing is I don't want to throw. Because the callback my not be wrapped in a domain.

What would you think of something like this:

    if (response.statusCode > 399) {
      var errMsg = util.format("%s %s %s %s %d %s %j \n", "API request to Iron.io failed." 
                              , "\nRequest URL:"
                              , response.request.uri.href
                              , "\nResponse Code: "
                              , response.statusCode
                              , "\nError Message: "
                              , JSON.parse(response.body))

      cb(response.statusCode, errMsg)
    }

@inadarei
Copy link
Author

inadarei commented Jun 2, 2014

Yes. Returning error rather than throwing sounds much better. I don't remember why my pull request was throwing an error, tbh. It was a while ago :)

The code you posted looks better

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants