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

Document middlewares order (priority) #1458

Closed
AlexWayfer opened this issue Nov 10, 2022 · 8 comments
Closed

Document middlewares order (priority) #1458

AlexWayfer opened this issue Nov 10, 2022 · 8 comments
Labels
documentation info Generic question on how to use Faraday

Comments

@AlexWayfer
Copy link
Contributor

Basic Info

  • Faraday Version: 2.6.0
  • Ruby Version: 3.1

Issue description

Please, add more documentation about middlewares ordering and priority. With detailed explanations how hooks work, I think.

Steps to reproduce

I had a code like:

Faraday.new(...) do |faraday|
  faraday.request :json
  faraday.response :json

  faraday.response :parse_dates
end

It seems obviously for me that a response firstly being decoded from JSON, then dates will be parsed.

The gem for parsing dates BTW: https://github.com/AlexWayfer/faraday-parse_dates

But it didn't work, until I've change their order.

Why? I believe I've done everything according to middlewares documentation in the faraday-parse_dates gem.

At the https://lostisland.github.io/faraday/middleware/ I've found such documentation:

Swapping middleware means giving the other priority.

How, in which order, some examples and strict rules?

@iMacTia
Copy link
Member

iMacTia commented Nov 10, 2022

Hey @AlexWayfer, thanks for raising this, this is great feedback.
The short version of it is that requests traverse the middlewares top-down, while responses travel back bottom-up.
We've put this image in the docs that I thought was really good at explaining things:

That said, I understand this can still be confusing for some, so I'll think about another illustrative way to visualise this.
If you have any suggestions, they would be very welcome!

@AlexWayfer
Copy link
Contributor Author

I saw this image. It's confusing for my case because there are middlewares are for request and response, both.

For example, you can define Faraday::Response.register_middleware and not define Faraday::Middleware.register_middleware, so it'll not be suitable.

In a perfect case, I'm not sure if it's possible, I'd just turn the direction for response middlewares reverse. It'd be more intuitive I believe.

@iMacTia
Copy link
Member

iMacTia commented Nov 10, 2022

That is unfortunately not possible without compeltely re-engineering Faraday's internals 😅.
The middleware stack is currently built by nesting each middleware into each other, which is what makes things like the "retry middleware" possible, but I agree it's not super-easy to grasp.

It doesn't really matter if the middleware was registered as a request or a response one, the only thing that matter is how they're added to the stack.

Say you have the following:

Faraday.new(...) do |faraday|
  faraday.request :authorization
  faraday.response :json
  faraday.response :parse_dates
end

This will result into a middleware stack like the following:

authorization do
  # authorization request hook
  json do
    # json request hook
    parse_dates do
      # parse_dates request hook
      response = adapter.perform(request)
      # parse_dates response hook
    end
    # json response hook
  end
  # authorization response hook
end

Hence you can see that parse_dates is the LAST middleware processing the request, and the FIRST middleware processing the response

@iMacTia iMacTia added the info Generic question on how to use Faraday label Nov 10, 2022
@AlexWayfer
Copy link
Contributor Author

I see the architecture with call, on_request and on_complete.

And I don't understand why on_ have no super-methods. While call I believe has one, but "you normally don't need to override it".

We could have more flexible approach with the super methods for on_, not only call for both (request and response).

Or we should stick with single call, and everything before super is for request, after — response. I've done this in mine Flame Rack-based framework.

I see a kind of conflict between approaches here.

@AlexWayfer
Copy link
Contributor Author

But OK, with the single call + super we would have the same problems: later middleware process the request later and the response earlier.

Meh.

There are different types of middlewares, different processes and chains. I'd like to see the super for on_ methods instead of overriding call. I think we should move in this way.

Again, we can left the call, but I guess supers for on_ methods will help with the order… or not. I'm messed up.

Also we can try to change the order of adding middlewares depending on their types: https://github.com/lostisland/faraday/blob/85dfdf8/lib/faraday/middleware_registry.rb#L28

Not to add into the end (merge hashes), but prepend. Not sure it'll work for any middleware, but we can process the basic cases and left old logic for others.

@panozzaj
Copy link

I have been working with Faraday a decent amount, and the nested block example above made the diagram above finally click for me. Before, I would just reorder the middlewares until the behavior seemed right. Thanks for providing that example!

iMacTia added a commit that referenced this issue Dec 14, 2022
Add an example to showcase the rack-style nested structure generated by the rack builder.
Addresses #1458 (comment)
@iMacTia
Copy link
Member

iMacTia commented Dec 20, 2022

@panozzaj @AlexWayfer FYI -- I've incorporated your feedback into the docs.

You can see the result on the Middleware page

I'm closing this issue as we're not really planning any internal refactoring on how middleware works, that would be quite the breaking change 😄 !

@iMacTia iMacTia closed this as completed Dec 20, 2022
@jpriollaud
Copy link

Another example

Doesn't work:

faraday.use CustomErrorMiddleware
faraday.response :raise_error

Works:

faraday.response :raise_error
faraday.use CustomErrorMiddleware

Not intuitive in the slightest.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation info Generic question on how to use Faraday
Projects
None yet
Development

No branches or pull requests

4 participants