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

What about the middleware #17

Open
dasch opened this issue Apr 10, 2015 · 12 comments
Open

What about the middleware #17

dasch opened this issue Apr 10, 2015 · 12 comments

Comments

@dasch
Copy link

dasch commented Apr 10, 2015

I've got an extensive middleware stack used in my production Faraday code, including

  • HTTP caching
  • retry logic
  • instrumentation (ActiveSupport::Notifications)
  • circuit breaker logic
  • automatic Avro decoding
  • error mapping

Having a unified interface for this in Faraday was nice. Are you planning on offering the same kind of functionality in Hurley?

@christhekeele
Copy link

Better understanding the middleware implementation/equivalent in Hurley is definitely staying my hand here, too.

@technoweenie
Copy link
Member

Check out the callbacks section: https://github.com/lostisland/hurley#client-callbacks

"before" callbacks are like Faraday request middleware. "after" callbacks are like Faraday response middleware.

If you've already seen these, what is it missing to implement your desired use cases?

@christhekeele
Copy link

More third-party support for some of the old faraday plugins than anything–sorry, should have been clearer.

@technoweenie
Copy link
Member

Oh, you're asking about middleware being bundled with Hurley. Well, of the items that @dasch mentioned, the only ones I can see ever bundling in Hurley are retry logic and instrumentation. Maybe error mapping.

@dasch
Copy link
Author

dasch commented Apr 16, 2015

I think it may just be a matter of what you call things, but I see middleware as purpose-specific objects with a well-defined interface that can be dropped into your stack. Callbacks, not so much. Also, some of my middlewares only conditionally call the upstream, i.e. circuit breakers, caching, etc. From perusing the README that doesn't seem to be allowed in Hurley. Maybe an around_call callback, e.g.

client.around_call :cache do |request, upstream|
  cache(request.url) do # muy simplificado
    upstream.call(request) # this would return the response
  end
end

Some integrations are really difficult to do without support for blocks.

@lucasmazza
Copy link

@dasch I've managed to implement the caching logic (sort of as a proof of concept) as a proxy class rather than using callbacks - https://github.com/plataformatec/hurley-http-cache.

@dasch
Copy link
Author

dasch commented Apr 16, 2015

That actually looks great! @technoweenie would you consider that a good pattern? Would it make sense to add a stack-like syntax for connection layering?

@technoweenie
Copy link
Member

client = Hurley::Client.new
client.connection = Hurley::HttpCache.new

This isn't a bad pattern, but not the one I had in mind (though I think it's best option considering how Hurley::Client#call is implemented today. I think my ideal is:

client = Hurley::Client.new
client.before_call(Hurley::HttpCache.new)
client.before_call(Hurley::CircuitBreaker.new)
client.after_call(Hurley::ErrorMapper.new)
client.connection = something # net/http, typhoeus, whatever

If request callbacks are allowed to return response objects, they could skip the connection. Given the stack above, the HTTP adapter won't be called unless it's uncached and the circuit breaker is not tripped. The error mapper "after" callback will then raise exceptions based on the status codes (if I'm interpreting how your error map might work correctly).

@lucasmazza
Copy link

being able to halt the request chain by returning a response object would help a lot to implement the caching as before/after callbacks, although I'm not sure how we could support ETag revalidation through callbacks. Would make sense for callbacks to have access to the connection object too?

@technoweenie
Copy link
Member

Oh interesting. I think we can arrange that.

@technoweenie
Copy link
Member

I implemented both in #27. How's that look?

@lucasmazza
Copy link

@technoweenie looks promising, I'll look into updating the cache gem to use callbacks instead of wrapping the connection object on top of the callback-early-exit.

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

4 participants