2

I have a working system to produce errors and send them to be used by Active Admin.

For example in Active admin, for a specific page of our CMS, the page might execute :

url_must_be_accessible("http://www.exmaple.com", field_url_partner, "URL for the partner")

And this uses the code below to send to the Active Admin Editor different type of errors notifications:

module UrlHttpResponseHelper

  def url_must_be_accessible(url, target_field, field_name)      
    if url
      url_response_code = get_url_http_response(url).code.to_i
      case url_response_code
      when -1                                                         
        # DNS issue; website does not exist; 
        errors.add(target_field,
          "#{field_name}: DNS Problem -> #{url} website does not exist")
      when 200
        return 
      when 304
         return
      else
        errors.add(target_field,
          "#{field_name}: #{url} sends #{url_response_code} response code")
      end
    end
  end

  def get_url_http_response(url)  
    uri = URI.parse(URI.encode(url))
    request = Net::HTTP.get_response(uri)
    return request
  rescue Errno::ECONNREFUSED, SocketError => e
    OpenStruct.new(code: -1)
  end

end

In local mode, this worked great! But in production, we're on Heroku and when a request pn Heroku goes beyond 30 seconds like if you try on this link "http://www.exmaple.com", the app crashes with a "H12 error".

I'd like to add to the code above two things - timeouts: i think i need both read_timeout and open_timeout and that the read_timeout + open_timeout should be < to 30 seconds, with let's take some security , better < 25seconds

  • if the request is still "going" after 25 seconds, then avoid reaching 30seconds by giving up/dropping the request

  • and catch this "we dropped the request intentionnally because risk of timeout" by sending a notification to the user. I'd like to use my current system with somehting along the lines of:

     rescue Errno::ECONNREFUSED, SocketError => e
        OpenStruct.new(code: -7) # = some random number
      end
    
    case url_response_code
    when -7
      errors.add(target_field,
              "#{field_name}: We tried to reach #{url} but this takes too long and risks crashing the app. please check the url and try again.")
    

How can I create a code like -1 but another one to rescue this "timeout"/"drop the request attempt" that I myself enforce.

Tried but nothing works. I don't manage to create the code for catch and drop this request if reaches 25 seconds...

Mathieu
  • 4,587
  • 11
  • 57
  • 112
  • Did you try to wrap your `get_url_http_response(url)` with Timeout.timeout { }` block? – nattfodd Feb 05 '18 at 08:27
  • no did not know of this method! looks very relevant to my need indeed. will check it out and see how to use it. I'd also need to catch the fatc timeout makes the request "drop" to send a notification with my current system (see question) – Mathieu Feb 05 '18 at 09:17

1 Answers1

4

That's not very beautiful solution (see: https://medium.com/@adamhooper/in-ruby-dont-use-timeout-77d9d4e5a001), but I believe you still can use it here, because you only have one thing happening inside opposite to the example in the link, where multiple actions could cause non-obvious behavior:

def get_url_http_response(url)
  uri = URI.parse(URI.encode(url))
  request = Timeout.timeout(25) { Net::HTTP.get_response(uri) }
  return request
rescue Errno::ECONNREFUSED, SocketError => e
  OpenStruct.new(code: -1)
rescue Timeout::Error
  # return here anything you want
end
nattfodd
  • 1,790
  • 1
  • 17
  • 35
  • hehe also stumbled on this post 2 mins ago:) and they say in comments that even if I was opting for get_response read_timeout or write_timeout parameters, ti would still use in the background Timeout "Note that Net::HTTP is still using timeout for open_timeout, as seen in https://github.com/ruby/rub... ... at least the read_timeout is implemented better now" – Mathieu Feb 05 '18 at 09:24
  • 1
    it worked! thanks, will allott points in 22 hours as it's blocked until then by SO – Mathieu Feb 05 '18 at 09:44