This project is archived and is in readonly mode.

#2947 ✓resolved
Ben

ActiveResource timeout doesn't set open_timeout on underlying Net:HTTP object

Reported by Ben | July 24th, 2009 @ 10:41 AM | in 2.x

ActiveResource provides a timeout variable: "Active Resource relies on HTTP to access RESTful APIs and as such is inherently susceptible to slow or unresponsive servers. In such cases, your Active Resource method calls could \timeout. You can control the amount of time before Active Resource times out with the timeout variable. "

However, the timeout value only sets the read_timeout on the underlying Net:HTTP object, not the open_timeout. This means that if a hostname resolves for an ActiveResource request, but there's no server to respond (or the server doesn't respond), the timeout value set in the ActiveResource subclass will not be adhered to. Modifying the ActiveResource::Connection class to also set the http object's open_timeout value should resolve the issue.

Comments and changes to this ticket

  • Michael Koziarski

    Michael Koziarski August 3rd, 2009 @ 06:06 AM

    • Tag changed from activeresource to activeresource, bugmash
  • Josh Nichols

    Josh Nichols August 8th, 2009 @ 11:46 PM

    +1 on this. read_timeout helps when the connection is open and is slow to serve up its goods, but does nothing about a slow response when opening in the first place.

  • pjammer

    pjammer August 9th, 2009 @ 02:10 AM

    I can't create a patch, however i dug into this and here is what i've found.

    We need to simply add the open_timeout to accept the timeout variable, so no additional functionality/arg's need to be changed in a person's app code. If they used timeout before, we simply take that same variable and apply it to Net::HTTP#opne_timeout, which is the variable you want to use if there is NO connection to a server. it appears the read_timeout variable is applied if you get a response but then nothing comes back, which as you can tell, is different then from being shut out of the server altogether.

    Here is the code.

    Line 139ish... 
    http.open_timeout = @timeout ||= 60 #default to 60 seconds. currently default is never.
    

    I've used the rdocs for Net::HTTP to verify the open_timeout method here:

    open_timeout [RW] Seconds to wait until connection is opened. If the HTTP object cannot open a connection in this many seconds, it raises a TimeoutError exception.

    Net::HTTP open_timeout doesn't have a default value however, it's set to nil == forever, so we set it to 60 seconds, to match the read_timeout variable's default in Net::HTTP.

  • Josh Nichols

    Josh Nichols August 9th, 2009 @ 07:45 AM

    Attached a patch to deal with this.

    There's a couple of complications. Mainly, the 'http' method is what is building the Net::HTTP object. This would be fine, except that that lib/active_resource/http_mock overrides this entirely to use itself (ActiveResource::HttpMock). Considering this is kind of a public API, I didn't want to change the 'http' method, so instead, I created a 'build_http' method which handles the actual construction. I made a few tests which test that things happen sanely during build_http, including setting open_timeout.

    I think some additional tests could be added for building the http object, but this should be enough for this particular issue.

  • Repository

    Repository August 9th, 2009 @ 10:29 AM

    • State changed from “new” to “resolved”

    (from [a0caad5255ed120192755fce10960a38b53c056d]) Setting connection timeout also affects Net::HTTP open_timeout.

    [#2947 state:resolved] http://github.com/rails/rails/commit/a0caad5255ed120192755fce10960a...

  • Repository
  • Jeremy Kemper

    Jeremy Kemper August 9th, 2009 @ 10:29 AM

    • Tag changed from activeresource, bugmash to activeresource

    Josh, rebased your patch against the proxy change and updated the test to not need a mock.

Create your profile

Help contribute to this project by taking a few moments to create your personal profile. Create your profile »

<h2 style="font-size: 14px">Tickets have moved to Github</h2>

The new ticket tracker is available at <a href="https://github.com/rails/rails/issues">https://github.com/rails/rails/issues</a>

People watching this ticket

Attachments

Referenced by

Pages