Skip to content

[BC-Break] Add request and response object to the preHandle and postHandle hooks #146

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

Closed
wants to merge 1 commit into from

Conversation

acasademont
Copy link
Contributor

I think it's reasonable to be able to access the request and response in the preHandle and postHandle hooks.

For example, I'm trying to trace some stuff using Blackfire.io and those hooks are the earliest possible place where I can start the probes. But I don't want to probe every request, only those that have a specific header, that's what I can't do right now. My only choice right now is starting the probe inside the app, in a Symfony request listener, but that's a bit too late if I want to profile the whole thing.

@andig
Copy link
Contributor

andig commented Nov 30, 2018

Would it make sense to (re)use one of the PSR-15 interfaces for this?

Tentatively tagged for 2.0 due to the bc break.

@andig andig added this to the 2.0 milestone Nov 30, 2018
@acasademont
Copy link
Contributor Author

We could definitely use the Middleware's interface from PSR15 but that would require quite a big rework (maybe in addition to php-pm/php-pm#439). This is not urgent at all, I already implemented what I wanted at the symfony event listener level instead of PHP-PM.

@andig
Copy link
Contributor

andig commented Mar 6, 2019

ping @acasademont I've started merging for 2.0. Is this something you'd like to have included? If its useful I feel it would be nice to build it on top of php-pm/php-pm#439, but keep all work in the slaves, not the master (even if that might not address all use cases).

@acasademont
Copy link
Contributor Author

Yes definitely, all work should be kept at the slave level. TBH i'm not really sure about this PR anymore. It think is nice to have it for the sake of completeness but I can live without it.

@andig
Copy link
Contributor

andig commented Mar 7, 2019

Then lets put this on hold for time being.

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

Successfully merging this pull request may close these issues.

2 participants