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

New feature: RaiseError #792

Merged
merged 3 commits into from
Nov 11, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions lib/http/chainable.rb
Original file line number Diff line number Diff line change
Expand Up @@ -243,6 +243,7 @@ def nodelay
# * instrumentation
# * logging
# * normalize_uri
# * raise_error
# @param features
def use(*features)
branch default_options.with_features(features)
Expand Down
3 changes: 3 additions & 0 deletions lib/http/errors.rb
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,9 @@ class ResponseError < Error; end
# Requested to do something when we're in the wrong state
class StateError < ResponseError; end

# When status code indicates an error
class StatusError < ResponseError; end

# Generic Timeout error
class TimeoutError < Error; end

Expand Down
3 changes: 2 additions & 1 deletion lib/http/feature.rb
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ def on_error(_request, _error); end

require "http/features/auto_inflate"
require "http/features/auto_deflate"
require "http/features/logging"
require "http/features/instrumentation"
require "http/features/logging"
require "http/features/normalize_uri"
require "http/features/raise_error"
23 changes: 23 additions & 0 deletions lib/http/features/raise_error.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
# frozen_string_literal: true

module HTTP
module Features
class RaiseError < Feature
def initialize(ignore: [])
super()

@ignore = ignore
end

def wrap_response(response)
if response.code >= 400 && [email protected]?(response.code)
raise HTTP::StatusError, "Unexpected status code #{response.code}"
end

response
Copy link
Member

Choose a reason for hiding this comment

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

WDYT about:

Suggested change
if response.code >= 400 && !@ignore.include?(response.code)
raise HTTP::StatusError, "Unexpected status code #{response.code}"
end
response
return response if response.code < 400
return response if @ignore.include?(response.code)
raise HTTP::StatusError, response

Then changing HTTP::StatusError to:

   class StatusError < ResponseError
     attr_reader :response

     def initialize(response)
       @response = response

       super("Unexpected status code #{response.code}")
     end
   end

end

HTTP::Options.register_feature(:raise_error, self)
end
end
end
62 changes: 62 additions & 0 deletions spec/lib/http/features/raise_error_spec.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
# frozen_string_literal: true

RSpec.describe HTTP::Features::RaiseError do
subject(:feature) { described_class.new(ignore: ignore) }

let(:connection) { double }
let(:status) { 200 }
let(:ignore) { [] }

describe "#wrap_response" do
subject(:result) { feature.wrap_response(response) }

let(:response) do
HTTP::Response.new(
version: "1.1",
status: status,
headers: {},
connection: connection,
request: HTTP::Request.new(verb: :get, uri: "https://example.com")
)
end

context "when status is 200" do
it "returns original request" do
expect(result).to be response
end
end

context "when status is 399" do
let(:status) { 399 }

it "returns original request" do
expect(result).to be response
end
end

context "when status is 400" do
let(:status) { 400 }

it "raises" do
expect { result }.to raise_error(HTTP::StatusError, "Unexpected status code 400")
end
end

context "when status is 599" do
let(:status) { 599 }

it "raises" do
expect { result }.to raise_error(HTTP::StatusError, "Unexpected status code 599")
end
end

context "when error status is ignored" do
let(:status) { 500 }
let(:ignored) { [500] }
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
let(:ignored) { [500] }
let(:ignore) { [500] }


it "raises" do
expect { result }.to raise_error(HTTP::StatusError, "Unexpected status code 500")
Copy link
Member

Choose a reason for hiding this comment

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

shouldn't it not raise an error if 500 is ignored?

end
end
end
end
Loading