Abstracting IO timeout settings

[straight-shoota]

IO::Evented defines read_timeout and write_timeout properties. They are not specific to the evented IO implementation. Any other implementation would use such timeouts, as well.
For win32's overlapped IO implementation, I copied the identical definitions from IO::Evented:

crystal/src/io/evented.cr

Lines 18 to 48 in ef3dc9f

	# Returns the time to wait when reading before raising an `IO::TimeoutError`.
	def read_timeout : Time::Span?
	@read_timeout
	end
	# Sets the time to wait when reading before raising an `IO::TimeoutError`.
	def read_timeout=(timeout : Time::Span?) : ::Time::Span?
	@read_timeout = timeout
	end
	# Sets the number of seconds to wait when reading before raising an `IO::TimeoutError`.
	def read_timeout=(read_timeout : Number) : Number
	self.read_timeout = read_timeout.seconds
	read_timeout
	end
	# Returns the time to wait when writing before raising an `IO::TimeoutError`.
	def write_timeout : Time::Span?
	@write_timeout
	end
	# Sets the time to wait when writing before raising an `IO::TimeoutError`.
	def write_timeout=(timeout : Time::Span?) : ::Time::Span?
	@write_timeout = timeout
	end
	# Sets the number of seconds to wait when writing before raising an `IO::TimeoutError`.
	def write_timeout=(write_timeout : Number) : Number
	self.write_timeout = write_timeout.seconds
	write_timeout
	end

Besides, these methods are part of the public API (of Socket and IO::FileDescriptor), so they should really be identical across different IO implementations.

Thus, I propose to abstract them to a dedicated module IO::Timeout which would define those two ivars and their getters and setters. Socket and IO::FileDescriptor should directly include this module.

Additionally, there are very similar methods in HTTP::Client. The only difference is that the values are stored as Float instead of Time::Span. But that could easily be adjusted to use the same module. The external API is already identical.

crystal/src/http/client.cr

Lines 269 to 313 in ef3dc9f

	# Sets the number of seconds to wait when reading before raising an `IO::TimeoutError`.
	#
	# ```
	# require "http/client"
	#
	# client = HTTP::Client.new("example.org")
	# client.read_timeout = 1.5
	# begin
	# response = client.get("/")
	# rescue IO::TimeoutError
	# puts "Timeout!"
	# end
	# ```
	def read_timeout=(read_timeout : Number)
	@read_timeout = read_timeout.to_f
	end
	# Sets the read timeout with a `Time::Span`, to wait when reading before raising an `IO::TimeoutError`.
	#
	# ```
	# require "http/client"
	#
	# client = HTTP::Client.new("example.org")
	# client.read_timeout = 5.minutes
	# begin
	# response = client.get("/")
	# rescue IO::TimeoutError
	# puts "Timeout!"
	# end
	# ```
	def read_timeout=(read_timeout : Time::Span)
	self.read_timeout = read_timeout.total_seconds
	end
	# Sets the write timeout - if any chunk of request is not written
	# within the number of seconds provided, `IO::TimeoutError` exception is raised.
	def write_timeout=(write_timeout : Number)
	@write_timeout = write_timeout.to_f
	end
	# Sets the write timeout - if any chunk of request is not written
	# within the provided `Time::Span`, `IO::TimeoutError` exception is raised.
	def write_timeout=(write_timeout : Time::Span)
	self.write_timeout = write_timeout.total_seconds
	end

Note: HTTP::Client stores these values instead of directly delegating to the underlying socket, because a client can use different socket instances under the hood (when it gets closed, it can re-open a new one for example).

There are also related connect_timeout and dns_timeout properties on HTTP::Client which don't exist on Socket. They're only methods parameters there.

For the record, IO::Timeout was the previous name of IO::TimeoutError. This could potentially be a reason for confusion. But the error type was renamed in 0.34.0 (#8885) without a deprecation period and I assume that's far enough away that it shouldn't prevent the name being repurposed.

[yxhuvud]

I agree that it should not belong to Evented but I am not certain there is a need of an abstraction. It is just two accessors with a default value after all.

[straight-shoota]

With the overloads for different argument types, it still totals to six different method definitions. IMO at that point an abstraction could be valuable.

[straight-shoota]

An additional argument: These methods are part of the public API. They should be portable and documented in a single place.

#10766 even requires an implementation that is independent of the async IO backend that can be selected at runtime.