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

Allow OnAfterResponse ResponseMiddleware to run on all paths #356

Open
lggomez opened this issue Jul 21, 2020 · 7 comments
Open

Allow OnAfterResponse ResponseMiddleware to run on all paths #356

lggomez opened this issue Jul 21, 2020 · 7 comments

Comments

@lggomez
Copy link
Contributor

lggomez commented Jul 21, 2020

As of now, custom response middlewares are only executed on the happy path of a request (that is, no error and with notParseResponse == false due to them being in the same list as the internal Resty middlewares for response handling

It is desirable that the user defined middlewares are executed on all paths and let the response handling to the user (as user defined request middlewares work now)

@zhong-wx
Copy link

zhong-wx commented Sep 3, 2020

Due to middlewares cannot run on all paths, I have to design a new Middleware layer base on Wrapping Transport. QAQ

@jeevatkm
Copy link
Member

jeevatkm commented Sep 4, 2020

@lggomez I believe you're referring to response middlewares correct? If so what is the behavior you're trying to propose, do you mind showcasing via PR?

@lggomez
Copy link
Contributor Author

lggomez commented Sep 5, 2020

Yes. The work involved is similar to #355, plus a separation of afterResponse and udAfterResponse so that OnAfterRequest middlewares behave exactly that the OnBefore ones, internal ones first and client defined after

Now that you solved the ci issue (thanks!) I was hoping to take a shot at this soon

@jeevatkm
Copy link
Member

jeevatkm commented Sep 5, 2020

@lggomez You're welcome, it was a config issue with Travis build.

Resty's middleware behavior (typically it should) will halt the execution at middleware which had an error regardless of internal or user-defined middleware.

#355 is different (thanks again for submitting PR). I'm trying to understand the design & approach of "run on all paths", could you please describe?

@zhong-wx
Copy link

zhong-wx commented Sep 6, 2020

I think "run on all paths" meas that afterResponse middlewares will be execute whether the request and beforeRequest middlewares are done successfully or not

@lggomez
Copy link
Contributor Author

lggomez commented Sep 8, 2020

It is as @zhong-wx says, I layed out the initial implementation on the linked PR

@jeevatkm
Copy link
Member

jeevatkm commented Oct 1, 2023

There is a middleware redesign coming in v3, so Resty users get to choose the middleware execution and its order. Plus this issue suggestion I will consider applying this on default flow.

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

No branches or pull requests

3 participants