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

POST command should return error, response, body #50

Open
roopesh opened this issue Jan 27, 2017 · 7 comments
Open

POST command should return error, response, body #50

roopesh opened this issue Jan 27, 2017 · 7 comments

Comments

@roopesh
Copy link

roopesh commented Jan 27, 2017

It's pretty standard to return at least an error in a callback function. I've modified it locally to give me the error, response, and body. Either way I have to interpret it but this way I have access to the response.statusCode and the body.response.result.

And trying to parse the body regardless of error and always trying to return the data regardless of result doesn't work in failure cases.

@mseminatore
Copy link
Owner

@roopesh Can you elaborate on your issue a little bit further? All API's take a standard Node-style callback which returns (error, data) to the callback function. Are you asking to return the body instead of the response? And are you asking to return an error or data but not both?

@roopesh
Copy link
Author

roopesh commented Jan 30, 2017

So when I send in an invalid token I don't get an error object back. In fact the node instance was creating because there wasn't a response in the data. The only way to know what was happening was to get the response and check the statusCode. I could see the 401.

So in my local version I return error, response, and body to interpret the call's output.

@roopesh
Copy link
Author

roopesh commented Jan 30, 2017

Oops. The node instance was crashing.

@mseminatore
Copy link
Owner

@roopesh Returning (error, body, response) would be non-standard behavior for a Node library so that is a non-starter. However body is already fully contained within response so only response would need to be returned.

Changing the return parameters to the node-back would be a breaking change to the API and so per semver rules that cannot be considered for 2.x. I will however consider it for 3.x. Please feel free to elaborate on what you'd like to see if it is different than what I just described.

@roopesh
Copy link
Author

roopesh commented Jan 30, 2017

That works for me. I'll update mine to return response, too, and see if it works.

@roopesh
Copy link
Author

roopesh commented Jan 30, 2017

That works. I'm now just getting the error, response back from the POST (I'm going to change the GET to the same thing) and I can get to the body from there.

Thank you!

@mseminatore
Copy link
Owner

@roopesh You are welcome! Hope the library works for your needs.

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

No branches or pull requests

2 participants