net-protocol icon indicating copy to clipboard operation
net-protocol copied to clipboard

Do OpenTimeout, ReadTimeout & WriteTimeout need to inherit from Timeout::Error?

Open jjb opened this issue 1 year ago • 0 comments

I believe the last use of the timeout() method was removed in 2008 76c2840

I believe the only use of the timeout library is OpenTimeout, ReadTimeout & WriteTimeout inheriting from Timeout::Error.

The test suite does not enforce this - I made them inherit from StandardError and the test suite still passes (although. n.b., it still passes with ReadTimeout removed, so maybe coverage isn't great).

I believe this was done for historical reasons. in the above-mentioned commit removing use of timeout, explicit raising of Timeout::Error (nee Timeout::TimeoutError) was added. After that, the ancestry was maintained when other changes were made. interesting commits: 43901df c0b1e4b 8e671df

I imagine this was done so that calling code which was expecting to rescue Timeout::Error wouldn't break.

Here's an example of client code rescuing Timeout::Error. It also rescues Net:ReadTimeout https://github.com/ruby/net-http/blob/705e7c0f65af0680bc3970063cbddf7889943184/lib/net/http.rb#L2358-L2362 - i'm guessing it was initially only Timeout::Error and the redundancy was added at some point

Would it be a good idea to phase out this dependency? My naive thinking is it could be fairly aggressive and land with ruby 3.4, since the high impact dependencies (ruby/net-*) are controllable. And also the change could be called out in release notes.

(sorry if this is already documented/discussed elsewhere)

jjb avatar Jul 10 '23 22:07 jjb