-
Notifications
You must be signed in to change notification settings - Fork 29
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
Handle html errors. #47
Comments
Hi @omar10594 thanks for submitting this issue. We'll take a look. |
@omar10594 are you able to reproduce this error? What is the HTTP response? You can see the raw HTTPS this way: https://gist.github.com/joaohornburg/27eb021709cda0bc4a38 |
Hi, I will try to log the response the next time we get a similar error, the last time we get an error of this type was at August 9, looks like they were having a problem in some of their services (http://status.rdstation.com.br/pages/history/54da143e240e245d4d000009#5d4ad79041918319d7d13f78) |
Hi, at our company we used your library to add contacts to our RD Station account when they signup in our site, sometimes the api responds with an error, but it's an html error so we are getting a JSON parse error.
I think this type of errors occurs when the api has a problem (status.rdstation.com.br)
Expected Behavior
Should raise a valid error
Current Behavior
Raises a
JSON::ParserError
Failure Information
This is an example of the body when we get a html error response.
For this example as it is a 504 code error, should raise a
RDStation::Error::ServerError
Solution
I think here the status code could be checked before parse the body.
rdstation-ruby-client/lib/rdstation/api_response.rb
Lines 3 to 8 in b719623
or for example
return JSON.parse(response.body) if response.code.between?(200, 299)
And do something similar in the error handler class to avoid parse the response there, or add a rescue block to return an empty errors array when the body could not be parsed.
With our logs we only can see the exception, not the request, so I'm not sure about the headers that it includes, but maybe the content type could be checked to know if it is a json body or not.
The text was updated successfully, but these errors were encountered: