From f768cf7a0e1e9a355b0b99daf1ffadacd893971e Mon Sep 17 00:00:00 2001 From: HoneyryderChuck Date: Wed, 15 Sep 2021 22:34:18 +0100 Subject: [PATCH 1/2] Improving API compatibility and error checking in responses * `Response#error`, which, coupled with `ErrorResponse#error`, allows for `if response.error` kind of conditional; * `Response#raise_for_status` now returns the response when no error is raise (for method chaining); Closes #153 --- lib/httpx/adapters/datadog.rb | 2 +- lib/httpx/response.rb | 10 ++++++++-- sig/response.rbs | 5 +++-- test/error_response_test.rb | 6 ++++++ test/response_test.rb | 18 +++++++++++++----- 5 files changed, 31 insertions(+), 10 deletions(-) diff --git a/lib/httpx/adapters/datadog.rb b/lib/httpx/adapters/datadog.rb index cfd2b700..0e5b28e4 100644 --- a/lib/httpx/adapters/datadog.rb +++ b/lib/httpx/adapters/datadog.rb @@ -64,7 +64,7 @@ module Datadog def finish(response) return unless @span - if response.respond_to?(:error) + if response.is_a?(::HTTPX::ErrorResponse) @span.set_error(response.error) else @span.set_tag(Datadog::Ext::HTTP::STATUS_CODE, response.status.to_s) diff --git a/lib/httpx/response.rb b/lib/httpx/response.rb index 0a76d558..788f58f5 100644 --- a/lib/httpx/response.rb +++ b/lib/httpx/response.rb @@ -64,10 +64,16 @@ module HTTPX end # :nocov: - def raise_for_status + def error return if @status < 400 - raise HTTPError, self + HTTPError.new(self) + end + + def raise_for_status + return self unless (err = error) + + raise err end def json(options = nil) diff --git a/sig/response.rbs b/sig/response.rbs index eb7a0916..235e7bba 100644 --- a/sig/response.rbs +++ b/sig/response.rbs @@ -1,6 +1,8 @@ module HTTPX interface _Response - def raise_for_status: () -> void + def raise_for_status: () -> self + + def error: () -> StandardError? end class Response @@ -89,7 +91,6 @@ module HTTPX @options: Options attr_reader request: Request - attr_reader error: Exception def status: () -> (Integer | _ToS) diff --git a/test/error_response_test.rb b/test/error_response_test.rb index f157d038..0c8fdbcd 100644 --- a/test/error_response_test.rb +++ b/test/error_response_test.rb @@ -10,6 +10,12 @@ class ErrorResponseTest < Minitest::Test assert r1.status == "wow" end + def test_error_response_error + error = RuntimeError.new("wow") + r1 = ErrorResponse.new(request_mock, error, {}) + assert r1.error == error + end + def test_error_response_raise_for_status some_error = Class.new(RuntimeError) r1 = ErrorResponse.new(request_mock, some_error.new("wow"), {}) diff --git a/test/response_test.rb b/test/response_test.rb index 44ef97ab..e64c0e31 100644 --- a/test/response_test.rb +++ b/test/response_test.rb @@ -28,16 +28,24 @@ class ResponseTest < Minitest::Test assert resource.body == "data", "body should have been updated" end - def test_raise_for_status + def test_response_error r1 = Response.new(request, 200, "2.0", {}) - r1.raise_for_status + assert r1.error.nil? + r2 = Response.new(request, 404, "2.0", {}) + assert !r2.error.nil? + assert r2.error.is_a?(HTTPError) + end + + def test_response_raise_for_status + r1 = Response.new(request, 200, "2.0", {}) + assert r1.raise_for_status == r1 r2 = Response.new(request, 302, "2.0", {}) - r2.raise_for_status + assert r2.raise_for_status == r2 r3 = Response.new(request, 404, "2.0", {}) - error = assert_raises(HTTPX::HTTPError) { r3.raise_for_status } + error = assert_raises(HTTPError) { r3.raise_for_status } assert error.status == 404 r4 = Response.new(request, 500, "2.0", {}) - error = assert_raises(HTTPX::HTTPError) { r4.raise_for_status } + error = assert_raises(HTTPError) { r4.raise_for_status } assert error.status == 500 end From a31a315e080f87937821e76a5bc0630299832463 Mon Sep 17 00:00:00 2001 From: HoneyryderChuck Date: Wed, 15 Sep 2021 23:12:56 +0100 Subject: [PATCH 2/2] deprecating ErrorResponse#status --- lib/httpx/response.rb | 1 + 1 file changed, 1 insertion(+) diff --git a/lib/httpx/response.rb b/lib/httpx/response.rb index 788f58f5..9677bf88 100644 --- a/lib/httpx/response.rb +++ b/lib/httpx/response.rb @@ -317,6 +317,7 @@ module HTTPX end def status + warn ":#{__method__} is deprecated, use :error.message instead" @error.message end