zendframework / zend-stratigility

Middleware for PHP built on top of PSR-7 and PSR-15
BSD 3-Clause "New" or "Revised" License
235 stars 57 forks source link

Calling Final Handler #34

Closed harikt closed 7 years ago

harikt commented 8 years ago

Hi all,

Working with zend-expressive with no error handlers installed, I noticed something that seems the problem at stratigility or the way I am thinking.

In Zend\Stratigility\MiddlewarePipe , we are passing Zend\Stratigility\FinalHandler to the Zend\Stratigility\Next.

So on a middlewares ( expect we have many middlewares handling on the same route M1, M2, M3 and most of them are similar as below )

<?php
use Psr\Http\Message\ResponseInterface;
use Psr\Http\Message\ServerRequestInterface;
class Middle1
{
    public function __invoke(ServerRequestInterface $request, ResponseInterface $response, callable $next = null)
    {
         // do something
         if($next) {
             $response = $next($request, $response);
         }
         // modify response if needed
        return $response;
    }
}

And you can see at Zend\Stratigility\Next once the queue got empty it calls the final handler and return a response which is 404. https://github.com/zendframework/zend-stratigility/blob/1.1.2/src/FinalHandler.php#L137 .

So always the 404 is returned.

I feel it should not be like that and it should still wait to get the response back and should only call the Final Handler if there is no response got.

So my belief is that it should not pass the FinalHandler to the Next class.

Thoughts ?

Thank you.

weierophinney commented 8 years ago

The final handler should only return a 404 if:

Otherwise, it returns the response it receives verbatim.

MiddlewarePipe, if not provided an $out instance (which may be a final handler, but could be a Next instance), creates the final handler with the response it receives and passed it to the Next instance it creates in order to execute its pipeline; this is done to ensure that it can always return a response on completion.

My point is: the final handler should only be invoked if the queue is exhausted, and it will only return a 404 under the very specific circumstances listed above. Check your middleware and see if perhaps you're creating a situation where that could be occurring.

harikt commented 8 years ago

So @weierophinney ,

I understood the problem

The two points may be true in the case for the Middlewares don't create or change a response in the middlewares and only after the last . See the middleware example mentioned earlier.

So I think it may need some other way to change the response and send it on the first middleware as a work around fix.

weierophinney commented 7 years ago

I'm closing this as I'm not sure if there was a resolution, but also because error handling has changed drastically, first as an opt-in feature of the 1.3 series, and second as the default mechanism in the 2.0 series. In these versions, you can now create middleware that performs error handling, vs. having a "final handler"; the "final handler" should now only be responsible for returning a default response.

harikt commented 7 years ago

Sure. No problem. If I happen to came across this in future, I hope I can re-open.

Thank you for your time.