mwunsch / weary

A framework and DSL for building RESTful web service clients
MIT License
480 stars 23 forks source link

Request timeout #40

Open PlugIN73 opened 11 years ago

PlugIN73 commented 11 years ago

Hello! It's possible to configure request timeout? For example - http://www.ruby-doc.org/stdlib-1.9.3/libdoc/net/http/rdoc/Net/HTTP.html#method-i-read_timeout-3D

jbmeerkat commented 11 years ago

:+1: to @PlugIN73 question

mwunsch commented 11 years ago

Consider Rack::Timeout (https://github.com/kch/rack-timeout) or another similar Rack based solution.

If that doesn't work, I can work on building in a Weary::Middleware to accommodate.

jbmeerkat commented 11 years ago

maybe add special dsl for this and pass to Net::HTTP?

class GithubRepo < Weary::Client
  domain "https://api.github.com"
  open_timeout 1
  read_timeout 1

  get :get, "/repos/{user}/{repo}"
end

I think it`s better use native ruby capabilities than add additional middlewares what do you think @mwunsch ?

PlugIN73 commented 11 years ago

@jbmeerkat It's look great!

mwunsch commented 11 years ago

@jbmeerkat Ideally, the solution would fit all the request adapters. It's not immediately clear to me what those two methods do when using Excon for example. Weary was built with the Rack middleware model expressly for this purpose. I'd much rather see something like:

class GithubRepo < Weary::Client
  use Weary::Middleware::Timeout, 1
end

Or something to that end. If another middleware can accomplish the same goal, more power to it. What could happen is that the Weary Timeout middleware can just attach some data to the request env that the adapters can read from.

The problem with dipping down into Net::HTTP directly is that it's not exactly clear what should happen when the TimeoutError is thrown -- how does Weary react to that?

jbmeerkat commented 11 years ago

@mwunsch maybe something like this

  get :get, "/repos/{user}/{repo}" do |resource|
    resource.on_timeout [] # or anything else what we need to return if request fails
  end

and if no on_timeout use default behavior with 504 status code

PlugIN73 commented 11 years ago

I think like so:

get :get, "/repos/{user}/{repo}" do |resource|
rescue TimeoutError -> e
  #user logic
end
PlugIN73 commented 11 years ago

@mwunsch I find Timeout middleware in your code. Does it work? Can we use such syntax:

class GithubRepo < Weary::Client
  use Weary::Middleware::Timeout, 1
end

?

mwunsch commented 11 years ago

Once a Timeout middleware is built, it can be used in that way, yes.

You can use Rack::Timeout and see if it accomplishes what you want.