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

Fix bug 47 handle html responses #48

Merged
merged 4 commits into from
Sep 17, 2019

Conversation

joaohornburg
Copy link
Contributor

This PR closes #47

In case of an HTML response by the API (which can happen when there is a major outage) the client was trying to parse a HTML as JSON, which resulted in raising JSON::ParserError. This made it impossible to know what the actual error is.

The proposed change here is to encapsulate the parse error and raise the correct exception, with the whole html body as the error message.

@joaohornburg joaohornburg added the WIP Work in progress label Aug 16, 2019
@joaohornburg joaohornburg mentioned this pull request Aug 16, 2019
@joaohornburg joaohornburg added bugfix Review me and removed WIP Work in progress labels Aug 16, 2019
@@ -13,6 +13,8 @@ def raise_error
raise error_class, array_of_errors.first if error_class < RDStation::Error

error_class.new(array_of_errors).raise_error
rescue JSON::ParserError => error
raise error_class, {'error_message' => response.body}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing spaces around the curly braces: { 'error_message' => response.body }

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instala o linter-rubocop nesse Atom aí! :D

@@ -13,6 +13,8 @@ def raise_error
raise error_class, array_of_errors.first if error_class < RDStation::Error

error_class.new(array_of_errors).raise_error
rescue JSON::ParserError => error
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since we are handling this error directly in the API response, in which cases that exception would be raised?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this will be raised when the body ain't a valid JSON. The most comon scenario for this to happen is when we have a downtime on the API.

@Seralto Seralto self-requested a review September 3, 2019 19:50
@joaohornburg joaohornburg merged commit e56a75d into master Sep 17, 2019
@m3nd3s m3nd3s deleted the fix-bug-47-handle-html-responses branch March 6, 2023 17:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Handle html errors.
3 participants