Bug #21803: `Addrinfo#connect_internal` should raise `IO::TimeoutError` on user-specified timeouts

• Status: Open
• Assignee: -
• Target version: -
• Backport: 3.2: UNKNOWN, 3.3: UNKNOWN, 3.4: UNKNOWN, 4.0: UNKNOWN

[shioimm (Misaki Shioi)]

Background & Proposal

Addrinfo#connect_internal is a core internal method that is called by both
Addrinfo#connect and Addrinfo#connect_from.
It receives a user-specified timeout value from these methods and currently raises
Errno::ETIMEDOUT when that timeout expires.

https://github.com/ruby/ruby/blame/1c07158203f00d020841f2400002f6e9b62d1318/ext/socket/lib/socket.rb#L65

  def connect_internal(local_addrinfo, timeout=nil) # :yields: socket
    sock = Socket.new(self.pfamily, self.socktype, self.protocol)
    begin
      sock.ipv6only! if self.ipv6?
      sock.bind local_addrinfo if local_addrinfo
      if timeout
        case sock.connect_nonblock(self, exception: false)
        when 0 # success or EISCONN, other errors raise
          break
        when :wait_writable
          sock.wait_writable(timeout) or
            raise Errno::ETIMEDOUT, "user specified timeout for #{self.ip_address}:#{self.ip_port}"
        end while true
      else
        sock.connect(self)
      end

However, this timeout is not caused by a system call returning Errno::ETIMEDOUT.
It is a Ruby-level timeout triggered by explicitly waiting with a user-specified
timeout value.
This distinction was discussed in another issue:
Introduce general IO#timeout and IO#timeout= for blocking operations)

After that discussion, Support IO#timeout for rsock_connect introduced a change so that rsock_connect() raises IO::TimeoutError
when a user-specified timeout expires.

In addition, TCPSocket.new and Socket.tcp were updated to align with this behavior, so that they raise IO::TimeoutError when a user-specified timeout occurs:
Socket.tcp and TCPSocket.new raise IO::TimeoutError with user specified timeout

However, the implementation of Socket.tcp still partially relies on
Addrinfo#connect_internal, specifically in cases where a synchronous connection
attempt is made to a single destination.
As a result, depending on the execution path, a user-specified timeout may raise
either IO::TimeoutError or Errno::ETIMEDOUT.
This inconsistency is undesirable. Therefore, I propose that Addrinfo#connect_internal should raise IO::TimeoutError (instead of Errno::ETIMEDOUT) when a user-specified timeout expires, to make the behavior consistent across the socket APIs.

[byroot (Jean Boussier)]

For what it's worth, changing the exception class is a breaking change (unless IO::TimeoutError is made to inherit from Errno::ETIMEDOUT, but it already inherits from IOError).

For instance https://github.com/redis-rb/redis-client/pull/278 is a regression caused by such change in Ruby 4.0.

[shioimm (Misaki Shioi)]

As you pointed out, changing the exception classes raised by TCPSocket.new and Socket.tcp does have user-facing impact, and perhaps this change should have been approached more carefully.
However, I believe that the fact that Socket.tcp still has cases where a user-specified timeout results in Errno::ETIMEDOUT, even after that change, is itself a source of confusion. That is the reason I opened this issue.
As an alternative approach, it might be possible for Socket.tcp to rescue Errno::ETIMEDOUT raised from Addrinfo#connect_internal and re-raise it as IO::TimeoutError.

[mame (Yusuke Endoh)]

• Related to Feature #18630: Introduce general `IO#timeout` and `IO#timeout=` for blocking operations. added