-
Notifications
You must be signed in to change notification settings - Fork 18
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
added InvalidRequestTokenError error #253
Conversation
11e8fd9
to
4deebad
Compare
if parsed_body.is_a?(Hash) && parsed_body[:type] == INVALID_REQUEST_TOKEN | ||
return Castle::InvalidRequestTokenError | ||
end | ||
rescue JSON::ParserError |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we want to assume our API can return an invalid JSON here? Perhaps we could catch JSON::ParserError
in the call
method instead.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is also used for the legacy endpoints, I wanted to keep the functionality unchanged if that error type value is not present
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Legacy APIs respond with the same format for 422 errors
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
not sure about eg impersonation endpoints, I prefer to keep that somehow covered
lib/castle/core/process_response.rb
Outdated
end | ||
end | ||
|
||
Castle::InvalidParametersError |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would be great to add a proper error message to these exceptions. Or is this going to be a different PR? :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍 good point
No description provided.