Assign Net::HTTPRequest to exceptions

With the changes made in [2ea5401][], the underlying
[Net::HTTPRequest][] instance is available to the connection throughout
the lifetime of the request. With that instance available, exceptions
raised during the request-response handshake can be constructed with the
`Net::HTTPRequest` instance.

Access to the `Net::HTTPRequest` from code that rescues from those
exceptions is  valuable:

* for debugging purposes
* for observability and logging
* for retrying requests, etc.

However, the exception classes are documented and therefore their
constructors are part of the public API. Inserting a `request`
positional argument is a breaking change. To account for that, this
commit includes deprecations for subclasses that call `super` from
within `#initialize` to instruct them on changing the signature to
account for the `Net::HTTPRequest` instance.

[2ea5401]: 2ea5401
[Net::HTTPRequest]: https://docs.ruby-lang.org/en/master/Net/HTTPRequest.html

Author: seanpdoyle

Gemfile

@@ -18,6 +18,7 @@ gem "rubocop-performance"
 gem "rubocop-rails"
 gem "rubocop-rails-omakase"
 
+gem "minitest", "< 6"
 gem "minitest-bisect"
 
 gemspec

lib/active_resource/base.rb

@@ -1509,7 +1509,7 @@ def save
     # of the <tt>before_*</tt> callbacks throw +:abort+ the action is
     # cancelled and <tt>save!</tt> raises ActiveResource::ResourceInvalid.
     def save!
-      save || raise(ResourceInvalid.new(self))
+      save || raise(ResourceInvalid.new(nil, self))
     end
 
     ##

lib/active_resource/connection.rb

@@ -136,52 +136,52 @@ def request(method, path, *arguments)
           payload[:request]     = request
           payload[:result]      = http.request(request)
         end
-        handle_response(result)
+        handle_response(request, result)
       rescue Timeout::Error => e
-        raise TimeoutError.new(e.message)
+        raise TimeoutError.new(request, e.message)
       rescue OpenSSL::SSL::SSLError => e
-        raise SSLError.new(e.message)
+        raise SSLError.new(request, e.message)
       rescue Errno::ECONNREFUSED => e
-        raise ConnectionRefusedError.new(e.message)
+        raise ConnectionRefusedError.new(request, e.message)
       end
 
       # Handles response and error codes from the remote service.
-      def handle_response(response)
+      def handle_response(request, response)
         case response.code.to_i
         when 301, 302, 303, 307
-          raise(Redirection.new(response))
+          raise(Redirection.new(request, response))
         when 200...400
           response
         when 400
-          raise(BadRequest.new(response))
+          raise(BadRequest.new(request, response))
         when 401
-          raise(UnauthorizedAccess.new(response))
+          raise(UnauthorizedAccess.new(request, response))
         when 402
-          raise(PaymentRequired.new(response))
+          raise(PaymentRequired.new(request, response))
         when 403
-          raise(ForbiddenAccess.new(response))
+          raise(ForbiddenAccess.new(request, response))
         when 404
-          raise(ResourceNotFound.new(response))
+          raise(ResourceNotFound.new(request, response))
         when 405
-          raise(MethodNotAllowed.new(response))
+          raise(MethodNotAllowed.new(request, response))
         when 409
-          raise(ResourceConflict.new(response))
+          raise(ResourceConflict.new(request, response))
         when 410
-          raise(ResourceGone.new(response))
+          raise(ResourceGone.new(request, response))
         when 412
-          raise(PreconditionFailed.new(response))
+          raise(PreconditionFailed.new(request, response))
         when 422
-          raise(ResourceInvalid.new(response))
+          raise(ResourceInvalid.new(request, response))
         when 429
-          raise(TooManyRequests.new(response))
+          raise(TooManyRequests.new(request, response))
         when 451
-          raise(UnavailableForLegalReasons.new(response))
+          raise(UnavailableForLegalReasons.new(request, response))
         when 401...500
-          raise(ClientError.new(response))
+          raise(ClientError.new(request, response))
         when 500...600
-          raise(ServerError.new(response))
+          raise(ServerError.new(request, response))
         else
-          raise(ConnectionError.new(response, "Unknown response code: #{response.code}"))
+          raise(ConnectionError.new(request, response, "Unknown response code: #{response.code}"))
         end
       end
 

lib/active_resource/exceptions.rb

@@ -2,9 +2,19 @@
 
 module ActiveResource
   class ConnectionError < StandardError # :nodoc:
-    attr_reader :response
+    attr_reader :request, :response
 
-    def initialize(response, message = nil)
+    def initialize(request, response = nil, message = nil)
+      if request.is_a?(Net::HTTPResponse) && (response.is_a?(String) || response.nil?)
+        ActiveResource.deprecator.warn(<<~WARN)
+          ConnectionError subclasses must be constructed with a request. Call super with a Net::HTTPRequest instance as the first argument.
+        WARN
+
+        message = response
+        response, request = request, nil
+      end
+
+      @request  = request
       @response = response
       @message  = message
     end
@@ -13,6 +23,7 @@ def to_s
       return @message if @message
 
       message = +"Failed."
+      message << "  Request = #{request.method} #{request.uri}." if request.respond_to?(:method) && request.respond_to?(:uri)
       message << "  Response code = #{response.code}." if response.respond_to?(:code)
       message << "  Response message = #{response.message}." if response.respond_to?(:message)
       message
@@ -21,22 +32,47 @@ def to_s
 
   # Raised when a Timeout::Error occurs.
   class TimeoutError < ConnectionError
-    def initialize(message)
+    def initialize(request, message = nil)
+      if request.is_a?(String)
+        ActiveResource.deprecator.warn(<<~WARN)
+          TimeoutError subclasses must be constructed with a request. Call super with a Net::HTTPRequest instance as the first argument.
+        WARN
+
+        message, request = request, nil
+      end
+
+      @request = request
       @message = message
     end
     def to_s; @message ; end
   end
 
   # Raised when a OpenSSL::SSL::SSLError occurs.
   class SSLError < ConnectionError
-    def initialize(message)
+    def initialize(request, message = nil)
+      if request.is_a?(String)
+        ActiveResource.deprecator.warn(<<~WARN)
+          SSLError subclasses must be constructed with a request. Call super with a Net::HTTPRequest instance as the first argument.
+        WARN
+
+        message, request = request, nil
+      end
+
+      @request = request
       @message = message
     end
     def to_s; @message ; end
   end
 
   # Raised when a Errno::ECONNREFUSED occurs.
-  class ConnectionRefusedError < Errno::ECONNREFUSED; end
+  class ConnectionRefusedError < Errno::ECONNREFUSED
+    attr_reader :request
+
+    def initialize(request, message)
+      @request = request
+      super(message)
+    end
+  end
 
   # 3xx Redirection
   class Redirection < ConnectionError # :nodoc:

test/cases/connection_test.rb

@@ -243,7 +243,8 @@ def test_timeout
     @http = mock("new Net::HTTP")
     @conn.expects(:http).returns(@http)
     @http.expects(:request).raises(Timeout::Error, "execution expired")
-    assert_raise(ActiveResource::TimeoutError) { @conn.get("/people_timeout.json") }
+    error = assert_raise(ActiveResource::TimeoutError) { @conn.get("/people_timeout.json") }
+    assert_kind_of Net::HTTPRequest, error.request
   end
 
   def test_setting_timeout
@@ -285,21 +286,24 @@ def test_ssl_error
     http = Net::HTTP.new("")
     @conn.expects(:http).returns(http)
     http.expects(:request).raises(OpenSSL::SSL::SSLError, "Expired certificate")
-    assert_raise(ActiveResource::SSLError) { @conn.get("/people/1.json") }
+    error = assert_raise(ActiveResource::SSLError) { @conn.get("/people/1.json") }
+    assert_kind_of Net::HTTPRequest, error.request
   end
 
   def test_handle_econnrefused
     http = Net::HTTP.new("")
     @conn.expects(:http).returns(http)
     http.expects(:request).raises(Errno::ECONNREFUSED, "Failed to open TCP connection")
-    assert_raise(ActiveResource::ConnectionRefusedError) { @conn.get("/people/1.json") }
+    error = assert_raise(ActiveResource::ConnectionRefusedError) { @conn.get("/people/1.json") }
+    assert_kind_of Net::HTTPRequest, error.request
   end
 
   def test_handle_econnrefused_with_backwards_compatible_error
     http = Net::HTTP.new("")
     @conn.expects(:http).returns(http)
     http.expects(:request).raises(Errno::ECONNREFUSED, "Failed to open TCP connection")
-    assert_raise(Errno::ECONNREFUSED) { @conn.get("/people/1.json") }
+    error = assert_raise(Errno::ECONNREFUSED) { @conn.get("/people/1.json") }
+    assert_kind_of Net::HTTPRequest, error.request
   end
 
   def test_auth_type_can_be_string
@@ -378,7 +382,7 @@ def assert_redirect_raises(code)
     end
 
     def handle_response(response)
-      @conn.__send__(:handle_response, response)
+      @conn.__send__(:handle_response, nil, response)
     end
 
     def decode(response)