lostisland / hurley

DEAD project, don't use
MIT License
177 stars 21 forks source link

What about the middleware #17

Open dasch opened 9 years ago

dasch commented 9 years ago

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

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

christhekeele commented 9 years ago

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

technoweenie commented 9 years ago

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 commented 9 years ago

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

technoweenie commented 9 years ago

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 commented 9 years ago

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 commented 9 years ago

@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 commented 9 years ago

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 commented 9 years ago
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 commented 9 years ago

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 commented 9 years ago

Oh interesting. I think we can arrange that.

technoweenie commented 9 years ago

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

lucasmazza commented 9 years ago

@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.