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

raise Oktakit::Error.from_response(response) fails with TypeError #59

Open
arharovets opened this issue Jun 3, 2022 · 0 comments · May be fixed by #60
Open

raise Oktakit::Error.from_response(response) fails with TypeError #59

arharovets opened this issue Jun 3, 2022 · 0 comments · May be fixed by #60

Comments

@arharovets
Copy link

arharovets commented Jun 3, 2022

Hi there,

I've encountered an unexpected behavior when trying to raise Oktakit::Error.from_response(response) causing TypeError - exception object expected with oktakit version 0.3.1.

Steps to reproduce

okta_client = Oktakit.new(token: token, api_endpoint: api_endpoint)
response, http_status = okta_client.get_user('[email protected]')

# response
#-> {:errorCode=>"E0000007", :errorSummary=> "Not found: Resource not found: [email protected] (User)", :errorLink=>"E0000007", :errorId=>"oaeLRic8zbhTBiJ81eJnWTQUg", :errorCauses=>[]}
# response.class
#-> Sawyer::Resource

raise Oktakit::Error.from_response(response) unless http_status == 200
#->TypeError - exception object expected:
#-> ... trace ...
###
response = {:errorCode=>"E0000007", :errorSummary=> "Not found: Resource not found: [email protected] (User)", :errorLink=>"E0000007", :errorId=>"oaeLRic8zbhTBiJ81eJnWTQUg", :errorCauses=>[]}
###

# lib/oktakit/error.rb
module Oktakit
  # Custom error class for rescuing from all Okta errors
  class Error < StandardError
    # Returns the appropriate Oktakit::Error subclass based
    # on status and response message
    #
    # @param [Hash] response HTTP response
    # @return [Oktakit::Error]
    def self.from_response(response)
      status = response[:status].to_i # nil.to_i = 0
      if (klass = error(status)) # this block returns nil
        klass.new(response)
      end
    end
    ...
    
    def build_error_message
      return nil if @response.nil?

      message =  "#{@response[:method].to_s.upcase} " # no corresponding attribute
      message << redact_url(@response[:url].to_s) + ': ' # same here
      message << "#{@response[:status]} - " # and same here
      message << response_message.to_s unless response_message.nil?
      message
    end
    ...

It looks like the structure of the response object for errors has changed (documentation link).

arharovets added a commit to arharovets/oktakit that referenced this issue Jun 13, 2022
This fixes [Shopify#59](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.
arharovets added a commit to arharovets/oktakit that referenced this issue Jun 14, 2022
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.
@arharovets arharovets linked a pull request Jun 17, 2022 that will close this issue
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 a pull request may close this issue.

1 participant