Rework exception classes in various small ways.

- Re-parent RestClient::Exceptions::Timeout under
  RestClient::RequestTimeout. Ideally these would be separate, since
  HTTP 408 is completely unrelated to network timeouts. However, making
  Timeout a subclass of RequestTimeout will preserve backwards
  compatibility while still making it possible to distinguish between
  the cases for users who want to.

- Make all HTTP status exception classes subclasses of
  RestClient::RequestFailed. Previously there were exceptions carved out
  for HTTP 304, 401, and 404, which for some reason directly subclassed
  the parent class of RequestFailed, ExceptionWithResponse. I don't know
  what purpose this served.

history.md

@@ -9,11 +9,17 @@ This release is largely API compatible, but makes several breaking changes.
   For example, `Content-Type: text/plain; charset=EUC-JP` will return a String
   encoded with `Encoding::EUC_JP`.
 - Change exceptions raised on request timeout. Instead of
-  RestClient::RequestTimeout (which is still used for HTTP 408), network
-  timeouts will now raise either RestClient::Exceptions::ReadTimeout or
-  RestClient::Exceptions::OpenTimeout, both of which inherit from
-  RestClient::Exceptions::Timeout. This class also makes the original wrapped
-  exception available as `#original_exception`.
+  `RestClient::RequestTimeout` (which is still used for HTTP 408), network
+  timeouts will now raise either `RestClient::Exceptions::ReadTimeout` or
+  `RestClient::Exceptions::OpenTimeout`, both of which inherit from
+  `RestClient::Exceptions::Timeout`. For backwards compatibility, this still
+  inherits from `RestClient::RequestTimeout` so existing uses will still work.
+  This may change in a future major release. These new timeout classes also
+  make the original wrapped exception available as `#original_exception`.
+- Unify request exceptions under `RestClient::RequestFailed`, which still
+  inherits from `ExceptionWithResponse`. Previously, HTTP 304, 401, and 404
+  inherited directly from `ExceptionWithResponse` rather than from
+  `RequestFailed`. Now _all_ HTTP status code exceptions inherit from both.
 - Rename `:timeout` to `:read_timeout`. When `:timeout` is passed, now set both
   `:read_timeout` and `:open_timeout`.
 - Change default HTTP Accept header to `*/*`

lib/restclient/exceptions.rb

@@ -100,20 +100,16 @@ module RestClient
       @response.body if @response
     end
 
-    def inspect
-      "#{message}: #{http_body}"
-    end
-
     def to_s
-      inspect
+      message
     end
 
     def message
-      @message || self.class.default_message
+      @message || default_message
     end
 
-    def self.default_message
-      self.name
+    def default_message
+      self.class.name
     end
   end
 
@@ -124,7 +120,7 @@ module RestClient
   # The request failed with an error code not managed by the code
   class RequestFailed < ExceptionWithResponse
 
-    def message
+    def default_message
       "HTTP status code #{http_code}"
     end
 
@@ -141,11 +137,30 @@ module RestClient
   module Exceptions
     # Map http status codes to the corresponding exception class
     EXCEPTIONS_MAP = {}
+  end
+
+  # Create HTTP status exception classes
+  STATUSES.each_pair do |code, message|
+    klass = Class.new(RequestFailed) do
+      send(:define_method, :default_message) {"#{http_code ? "#{http_code} " : ''}#{message}"}
+    end
+    klass_constant = const_set(message.delete(' \-\''), klass)
+    Exceptions::EXCEPTIONS_MAP[code] = klass_constant
+  end
+
+  # Backwards compatibility. "Not Found" is the actual text in the RFCs.
+  ResourceNotFound = NotFound
+
+  module Exceptions
+    # We have to split the Exceptions module like we do here because the
+    # EXCEPTIONS_MAP is under Exceptions, but we depend on
+    # RestClient::RequestTimeout below.
 
     # Base class for request timeouts.
+    #
     # NB: Previous releases of rest-client would raise RequestTimeout both for
     # HTTP 408 responses and for actual connection timeouts.
-    class Timeout < RestClient::Exception
+    class Timeout < RestClient::RequestTimeout
       def initialize(message=nil, original_exception=nil)
         super(nil, nil)
         self.message = message if message
@@ -156,7 +171,7 @@ module RestClient
     # Timeout when connecting to a server. Typically wraps Net::OpenTimeout (in
     # ruby 2.0 or greater).
     class OpenTimeout < Timeout
-      def self.default_message
+      def default_message
         'Timed out connecting to server'
       end
     end
@@ -164,25 +179,12 @@ module RestClient
     # Timeout when reading from a server. Typically wraps Net::ReadTimeout (in
     # ruby 2.0 or greater).
     class ReadTimeout < Timeout
-      def self.default_message
+      def default_message
         'Timed out reading data from server'
       end
     end
   end
 
-  STATUSES.each_pair do |code, message|
-
-    # Compatibility
-    superclass = ([304, 401, 404].include? code) ? ExceptionWithResponse : RequestFailed
-    klass = Class.new(superclass) do
-      send(:define_method, :message) {"#{http_code ? "#{http_code} " : ''}#{message}"}
-    end
-    klass_constant = const_set message.delete(' \-\''), klass
-    Exceptions::EXCEPTIONS_MAP[code] = klass_constant
-  end
-
-  # Backwards compatibility. "Not Found" is the actual text in the RFCs.
-  ResourceNotFound = NotFound
 
   # The server broke the connection prior to the request completing.  Usually
   # this means it crashed, or sometimes that your network connection was

spec/unit/exceptions_spec.rb

@@ -84,4 +84,19 @@ describe "backwards compatibility" do
   it 'aliases RestClient::NotFound as ResourceNotFound' do
     RestClient::ResourceNotFound.should eq RestClient::NotFound
   end
+
+  it 'subclasses NotFound from RequestFailed, ExceptionWithResponse' do
+    RestClient::NotFound.should be < RestClient::RequestFailed
+    RestClient::NotFound.should be < RestClient::ExceptionWithResponse
+  end
+
+  it 'subclasses timeout from RestClient::RequestTimeout, RequestFailed, EWR' do
+    RestClient::Exceptions::OpenTimeout.should be < RestClient::Exceptions::Timeout
+    RestClient::Exceptions::ReadTimeout.should be < RestClient::Exceptions::Timeout
+
+    RestClient::Exceptions::Timeout.should be < RestClient::RequestTimeout
+    RestClient::Exceptions::Timeout.should be < RestClient::RequestFailed
+    RestClient::Exceptions::Timeout.should be < RestClient::ExceptionWithResponse
+  end
+
 end

spec/unit/request_spec.rb

@@ -383,6 +383,19 @@ describe RestClient::Request, :include_helpers do
     lambda { @request.transmit(@uri, 'req', nil) }.should raise_error(RestClient::Exceptions::OpenTimeout)
   end
 
+  it "uses correct error message for ReadTimeout",
+     :if => defined?(Net::ReadTimeout) do
+    @http.stub(:request).and_raise(Net::ReadTimeout)
+    lambda { @request.transmit(@uri, 'req', nil) }.should raise_error(RestClient::Exceptions::ReadTimeout, 'Timed out reading data from server')
+  end
+
+  it "uses correct error message for OpenTimeout",
+     :if => defined?(Net::OpenTimeout) do
+    @http.stub(:request).and_raise(Net::OpenTimeout)
+    lambda { @request.transmit(@uri, 'req', nil) }.should raise_error(RestClient::Exceptions::OpenTimeout, 'Timed out connecting to server')
+  end
+
+
   it "class method execute wraps constructor" do
     req = double("rest request")
     RestClient::Request.should_receive(:new).with(1 => 2).and_return(req)