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

Show correct information in case of an error #41

Closed
wants to merge 2 commits into from

Conversation

boncey
Copy link

@boncey boncey commented Apr 16, 2014

Hi, I've noticed that if you hit an error with the API you don't get back the error message.
This is because the code is looking at the wrong field; error instead of errors.

I've changed this to use the correct field and to show warnings and response codes also.

boncey added 2 commits April 16, 2014 18:57
Show warnings if provided
Show response code
@weppos weppos self-assigned this Apr 16, 2014
@weppos
Copy link
Member

weppos commented Apr 16, 2014

Actually, the correct field is message. There should be no warning or errors.

error was used in the past and there are still a few occurrences. errors is supposed to host the array of fields that have an error attached, in case the error is a validation error.

I'm closing this PR since this is invalid. Feel free to submit a patch that uses the message field, if you want. Please make sure to attach some relevant test cases.

You may want to use the official fixtures to write the tests.

@weppos weppos closed this Apr 16, 2014
@boncey
Copy link
Author

boncey commented Apr 16, 2014

This is the response I am getting back from the server.

{"domain_name"=>"dailytwurly.com", "name"=>"www", "errors"=>[], "warning"=>"www.dailytwurly.com → 80.87.131.12 already exists so it was ignored."}

What am I missing?

@boncey
Copy link
Author

boncey commented Apr 16, 2014

Also, this has a response code of 422.
The code maps 406 to RecordExists - shouldn't it return a 406?

@weppos
Copy link
Member

weppos commented Apr 16, 2014

406 means Not Acceptable.

That specific response is buggy. It should return either:

  • a successful response with either a 200 or 204 (or)
  • an error response with a 400 and a message

I'll open a ticket internally to make sure the issue is fixed asap.

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