Skip to content

Commit

Permalink
Pass HTTP status to Oktakit::Error.from_response() [description below]
Browse files Browse the repository at this point in the history
This fixes #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.
  • Loading branch information
arharovets committed Jun 14, 2022
1 parent 1c58db5 commit ff1c609
Show file tree
Hide file tree
Showing 3 changed files with 115 additions and 22 deletions.
42 changes: 22 additions & 20 deletions lib/oktakit/error.rb
Original file line number Diff line number Diff line change
Expand Up @@ -6,11 +6,12 @@ class Error < StandardError
#
# @param [Hash] response HTTP response
# @return [Oktakit::Error]
def self.from_response(response)
status = response[:status].to_i
if (klass = error(status))
klass.new(response)
end
def self.from_response(response, status)
status = status.to_i

return if response.nil? || status.nil? || status.zero?

(klass = error(status)) ? klass.new(response, status) : nil
end

def self.error(status)
Expand All @@ -33,8 +34,10 @@ def self.error(status)
end
end

def initialize(response = nil)
def initialize(response = nil, status = nil)
@response = response
@status = status

super(build_error_message)
end

Expand All @@ -54,18 +57,16 @@ def data

private

attr_reader :response, :status

def parse_data
body = @response[:body]
return if body.empty?
return body unless body.is_a?(String)
return '' if response.empty?
return response unless response.is_a?(String)

headers = @response[:response_headers]
headers = response[:response_headers]
content_type = headers && headers[:content_type] || ''
if content_type =~ /json/
Sawyer::Agent.serializer.decode(body)
else
body
end

content_type =~ /json/ ? Sawyer::Agent.serializer.decode(body) : body
end

def response_message
Expand All @@ -78,12 +79,13 @@ def response_message
end

def build_error_message
return nil if @response.nil?
message = ''

message << "#{response[:method].to_s.upcase} " unless response[:method].nil?
message << "#{redact_url(response[:url].to_s)} : " unless response[:url].nil?
message << status.to_s
message << " - #{response_message}" unless response_message.nil? || response_message.empty?

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

Expand Down
4 changes: 2 additions & 2 deletions lib/oktakit/response/raise_error.rb
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,8 @@ module Response
class RaiseError < Faraday::Response::Middleware
private

def on_complete(response)
if (error = Oktakit::Error.from_response(response))
def on_complete(response, status)
if (error = Oktakit::Error.from_response(response, status))
raise error
end
end
Expand Down
91 changes: 91 additions & 0 deletions spec/error_spec.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,91 @@
require "spec_helper"

describe Oktakit::Error do
describe "#from_response(response)" do
context "success" do
subject(:from_response) { described_class.from_response(okta_response, 404) }

context "with response and status" do
let(:okta_response) do
{
errorCode: "00000000",
errorSummary: "Not found: Resource not found: [email protected] (User)",
errorLink: "E0000007",
errorId: "00000000",
errorCauses: [],
}
end

it "returns an instance of the described class" do
expect(from_response.class).to(eq(Oktakit::NotFound))
end

it "returns a meaningful error message" do
expected_message = "404 - Not found: Resource not found: [email protected] (User)"

expect(from_response.message).to(eq(expected_message))
end
end

context "with missing response" do
let(:okta_response) { {} }

it "returns an instance of the described class" do
expect(from_response.class).to(eq(Oktakit::NotFound))
end

it "returns a meaningful error message" do
expect(from_response.message).to(eq("404"))
end
end

context "with a different response format" do
let(:okta_response) do
{
status: 404,
method: "POST",
url: "http://example.com/api/v1",
response_headers: {
content_type: "json",
},
body: {
errorCode: "E0000007",
errorSummary: "Not found: Resource not found: [email protected] (User)",
errorLink: "E0000007",
errorId: "oaeLRic8zbhTBiJ81eJnWTQUg",
errorCauses: [],
},
}
end

it "returns an instance of the described class" do
expect(from_response.class).to(eq(Oktakit::NotFound))
end

it "returns a meaningful error message" do
expected_message = "POST http://example.com/api/v1 : 404"

expect(from_response.message).to(eq(expected_message))
end
end
end

context "with errors" do
context "with missing status" do
subject(:from_response) { described_class.from_response(okta_response, 0) }

let(:okta_response) do
{
errorCode: "00000000",
errorSummary: "Not found: Resource not found: [email protected] (User)",
errorLink: "E0000007",
errorId: "00000000",
errorCauses: [],
}
end

it { is_expected.to(be_nil) }
end
end
end
end

0 comments on commit ff1c609

Please sign in to comment.