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

Pass HTTP status to Oktakit::Error.from_response() #60

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

arharovets
Copy link

This fixes #59 by
modifying Oktakit::Error.from_response() method to accept and
process client response and HTTP status.

The reason behind these changes:

response, http_status = client.get_user('[email protected]')

This usage method is listed in the repo's README and works in most
cases. However, it separates response from HTTP status. response is a
Sawyer::Resource object and doesn't contain any data about the request
(HTTP status, URL, method or headers). It only has a Hash with Okta
error code, error summary, internal error link, error id and error
causes, which causes a TypeError if we try to
raise Oktakit::Error.from_response(response) unless http_status == 200
in case the user is not found.

With these changes implemented, we will gain the ability to process
different response formats without causing errors on the client side.

This fixes Shopify#59 by
modifying `Oktakit::Error.from_response()` method to accept and
process client response and HTTP status.

The reason behind these changes:
```ruby
response, http_status = client.get_user('[email protected]')
```
This usage method is listed in the repo's README and works in most
cases. However, it separates response from HTTP status. `response` is a
`Sawyer::Resource` object and doesn't contain any data about the request
(HTTP status, URL, method or headers). It only has a Hash with Okta
error code, error summary, internal error link, error id and error
causes, which causes a `TypeError` if we try to
`raise Oktakit::Error.from_response(response) unless http_status == 200`
in case the user is not found.

With these changes implemented, we will gain the ability to process
different response formats without causing errors on the client side.
@ghost ghost added the cla-needed label Jun 17, 2022
@ghost ghost removed the cla-needed label Jun 28, 2022
@arharovets arharovets force-pushed the pass_status_to_error branch from 020579f to ff1c609 Compare June 28, 2022 11:33
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.

raise Oktakit::Error.from_response(response) fails with TypeError
1 participant