Shopify/smart_todo

Gem release check is failing with HTTP timeout

Opened this issue · 3 comments

We have a step on CI that checks TODOs and it's failing with HTTP timeout quite often:

/usr/local/ruby/lib/ruby/3.0.0/net/protocol.rb:43:in `ssl_socket_connect': Net::OpenTimeout (Net::OpenTimeout)
--
  | from /usr/local/ruby/lib/ruby/3.0.0/net/http.rb:1038:in `connect'
  | from /usr/local/ruby/lib/ruby/3.0.0/net/http.rb:970:in `do_start'
  | from /usr/local/ruby/lib/ruby/3.0.0/net/http.rb:959:in `start'
  | from /usr/local/ruby/lib/ruby/3.0.0/net/http.rb:1512:in `request'
  | from /usr/local/ruby/lib/ruby/3.0.0/net/http.rb:1270:in `get'
  | from /tmp/bundle/ruby/3.0.0/gems/smart_todo-1.2.0/lib/smart_todo/events/gem_release.rb:28:in `met?'
  | from /tmp/bundle/ruby/3.0.0/gems/smart_todo-1.2.0/lib/smart_todo/events.rb:38:in `gem_release'

I haven't found any information regarding SLO for rubygems API. But I think it would be helpful to allow to configure HTTP client and maybe even use faraday instead of standard ruby HTTP client.

Here is the code for the reference:

https://github.com/Shopify/smart_todo/blob/master/lib/smart_todo/events/gem_release.rb#L63

Thank you for the issue. Are you able to open a PR adding some configuration to the timeout of the client? I'd like to avoid adding faraday as a dependency of the gem since I don't think it is necessary just yet.

Sure, I can do it. Btw, I have just found there is similar code for slack API but it actually has timeouts:

https://github.com/Shopify/smart_todo/blob/master/lib/smart_todo/slack_client.rb#L30-L31

Do you think we have to use different configs for rubygems/slack or just provide generic HTTP client options? It could be handy to configure it separately but I think in the most of cases consumer of this code doesn't care. So I'd keep it simple with single config.

For the options itself we also have few options:

  • configure options as Hash;
  • allow to configure a lambda that should be executed after client is built;
  • allow to configure a lambda that should accept the URL and build an instance of Net::HTTP;

I'd prefer to keep it simple and use a Hash.

Single option as a Hash is a good start to me. If we need to build something more complicated users will ask for it.