Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Request timeout #40

Open
PlugIN73 opened this issue Oct 3, 2013 · 9 comments
Open

Request timeout #40

PlugIN73 opened this issue Oct 3, 2013 · 9 comments

Comments

@PlugIN73
Copy link

PlugIN73 commented Oct 3, 2013

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
Copy link

👍 to @PlugIN73 question

@mwunsch
Copy link
Owner

mwunsch commented Oct 4, 2013

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
Copy link

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
Copy link
Author

PlugIN73 commented Oct 4, 2013

@jbmeerkat It's look great!

@mwunsch
Copy link
Owner

mwunsch commented Oct 4, 2013

@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
Copy link

@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
Copy link
Author

PlugIN73 commented Oct 4, 2013

I think like so:

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

@PlugIN73
Copy link
Author

PlugIN73 commented Oct 4, 2013

@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
Copy link
Owner

mwunsch commented Oct 4, 2013

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants