Separate timeout errors by type

Excon used to raise Excon::Error::Timeout for any type of timeout.
This commit splits this error into multiple specific ones, with the
following inheritance hierarchy:

Timeout
|_ReadTimeout
|_WriteTimeout
|_ConnectTimeout
  |_ConnectReadTimeout
  |_ConnectWriteTimeout

lib/excon/constants.rb

@@ -14,7 +14,7 @@ module Excon
   DEFAULT_RETRY_LIMIT = 4
 
   DEFAULT_RETRY_ERRORS = [
-    Excon::Error::Timeout,
+    Excon::Error::ConnectTimeout,
     Excon::Error::Socket,
     Excon::Error::HTTPStatus
   ].freeze

lib/excon/error.rb

@@ -48,9 +48,33 @@ def initialize(socket_error = Excon::Error.new)
 
     class InvalidHeaderKey < Error; end
     class InvalidHeaderValue < Error; end
-    class Timeout < Error; end
     class ResponseParse < Error; end
 
+    class Timeout < Error
+      def self.by_type(type, human_name)
+        case type
+        when :connect_read
+          Excon::Errors::ConnectReadTimeout.described_as(human_name)
+        when :connect_write
+          Excon::Errors::ConnectWriteTimeout.described_as(human_name)
+        when :read
+          Excon::Errors::ReadTimeout.described_as(human_name)
+        when :write
+          Excon::Errors::WriteTimeout.described_as(human_name)
+        end
+      end
+
+      def self.described_as(timeout_kind)
+        new("#{timeout_kind.to_s.tr('_', ' ')} timeout reached")
+      end
+    end
+
+    class ReadTimeout < Timeout; end
+    class WriteTimeout < Timeout; end
+    class ConnectTimeout < Timeout; end
+    class ConnectReadTimeout < ConnectTimeout; end
+    class ConnectWriteTimeout < ConnectTimeout; end
+
     class ProxyConnectionError < Error
       attr_reader :request, :response
 

lib/excon/socket.rb

@@ -96,7 +96,7 @@ def readline
             @socket.readline
           end
         rescue Timeout::Error
-          raise Excon::Errors::Timeout.new('read timeout reached')
+          raise Excon::Errors::ReadTimeout.described_as('read')
         end
       end
     end
@@ -364,6 +364,9 @@ def select_with_timeout(socket, type)
       if @data.include?(:deadline)
         request_timeout = request_time_remaining
 
+        # If request timeout is already reached, there's no need to proceed.
+        raise(Excon::Errors::Timeout.by_type(type, 'request')) if request_timeout <= 0
+
         # If the time remaining until the request times out is less than the timeout for the type of select,
         # use the time remaining as the timeout instead.
         if request_timeout < timeout
@@ -383,7 +386,7 @@ def select_with_timeout(socket, type)
         IO.select(nil, [socket], nil, timeout)
       end
 
-      select || raise(Excon::Errors::Timeout.new("#{timeout_kind} timeout reached"))
+      select || raise(Excon::Errors::Timeout.by_type(type, timeout_kind))
     end
 
     def unpacked_sockaddr
@@ -395,13 +398,9 @@ def unpacked_sockaddr
     end
 
     # Returns the remaining time in seconds until we reach the deadline for the request timeout.
-    # Raises an exception if we have exceeded the request timeout's deadline.
     def request_time_remaining
       now = Process.clock_gettime(Process::CLOCK_MONOTONIC)
       deadline = @data[:deadline]
-
-      raise(Excon::Errors::Timeout.new('request timeout reached')) if now >= deadline
-
       deadline - now
     end
   end

lib/excon/ssl_socket.rb

@@ -157,7 +157,7 @@ def initialize(data = {})
           @socket.connect
         end
       rescue Errno::ETIMEDOUT, Timeout::Error
-        raise Excon::Errors::Timeout.new('connect timeout reached')
+        raise Excon::Errors::ConnectTimeout.described_as('connect')
       end
 
       # verify connection

lib/excon/unix_socket.rb

@@ -15,7 +15,7 @@ def connect
           @socket.connect_nonblock(sockaddr)
         rescue Errno::EINPROGRESS
           unless IO.select(nil, [@socket], nil, @data[:connect_timeout])
-            raise(Excon::Errors::Timeout.new("connect timeout reached"))
+            raise Excon::Errors::ConnectTimeout.described_as('connect')
           end
           begin
             @socket.connect_nonblock(sockaddr)
@@ -29,7 +29,7 @@ def connect
             @socket.connect(sockaddr)
           end
         rescue Timeout::Error
-          raise Excon::Errors::Timeout.new('connect timeout reached')
+          raise Excon::Errors::ConnectTimeout.described_as('connect')
         end
       end
 

spec/requests/timeout_spec.rb

@@ -39,7 +39,7 @@
 
       it 'does not raise' do
         # raising a read timeout to keep tests fast
-        expect { conn.request(:path => '/timeout') }.to raise_error(Excon::Error::Timeout, 'read timeout reached')
+        expect { conn.request(:path => '/timeout') }.to raise_error(Excon::Error::ReadTimeout, 'read timeout reached')
       end
     end
   end
@@ -67,7 +67,7 @@
       let(:timeout) { 0.001 }
 
       it 'returns a request Excon::Error::Timeout' do
-        expect { conn.request(:path => '/timeout') }.to raise_error(Excon::Error::Timeout, 'request timeout reached')
+        expect { conn.request(:path => '/timeout') }.to raise_error(Excon::Error::ReadTimeout, 'request timeout reached')
       end
     end
 
@@ -76,7 +76,7 @@
       let(:timeout) { 5 }
 
       it 'returns a read Excon::Error::Timeout' do
-        expect { conn.request(:path => '/timeout') }.to raise_error(Excon::Error::Timeout, 'read timeout reached')
+        expect { conn.request(:path => '/timeout') }.to raise_error(Excon::Error::ReadTimeout, 'read timeout reached')
       end
     end
   end

tests/timeout_tests.rb

@@ -2,7 +2,7 @@
   with_rackup('timeout.ru') do
 
     [false, true].each do |nonblock|
-      tests("nonblock => #{nonblock} hits read_timeout").raises(Excon::Errors::Timeout) do
+      tests("nonblock => #{nonblock} hits read_timeout").raises(Excon::Errors::ReadTimeout) do
         connection = Excon.new('http://127.0.0.1:9292', :nonblock => nonblock)
         connection.request(:method => :get, :path => '/timeout', :read_timeout => 1)
       end