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

raiseThrowables() - Issue here or in Expressive? #84

Closed moderndeveloperllc closed 7 years ago

moderndeveloperllc commented 7 years ago

I might just have really bad timing with this, but I'm having issues with 1.3.0 and Expressive master branch which I think is related to raiseThrowables(). I'm using configuration-driven pipelining, and have converted to the new style of handling errors, but thrown exceptions are not bubbling their way out to the error middleware - they are hitting the error depreciation notice in Next.

Do I just need to wait till Expressive is updated, or is this a Stratigility issue?

FWIW, here is my pipeline config (I factory ErrorHandler for a custom generator):

return [
    'dependencies'        => [
        'factories'  => [
            Helper\ServerUrlMiddleware::class          => Helper\ServerUrlMiddlewareFactory::class,
            Helper\UrlHelperMiddleware::class          => Helper\UrlHelperMiddlewareFactory::class,
            MyApp\Action\AuthenticationAction::class   => MyApp\Action\AuthenticationFactory::class,
            Zend\Stratigility\Middleware\ErrorHandler::class => MyApp\ErrorHandling\ErrorHandlerFactory::class
        ],
        'invokables' => [
            Zend\Stratigility\Middleware\OriginalMessages::class => Zend\Stratigility\Middleware\OriginalMessages::class,
            Zend\Stratigility\NoopFinalHandler::class => Zend\Stratigility\NoopFinalHandler::class,
            Helper\BodyParams\BodyParamsMiddleware::class => Helper\BodyParams\BodyParamsMiddleware::class,
            MyApp\Action\AuthorizationAction::class => MyApp\Action\AuthorizationAction::class,
            MyApp\ErrorHandling\ErrorResponseGenerator::class => MyApp\ErrorHandling\ErrorResponseGenerator::class,
        ],
    ],
    'middleware_pipeline' => [
        'always' => [
            'middleware' => [
                Zend\Stratigility\Middleware\OriginalMessages::class,
                Zend\Stratigility\Middleware\ErrorHandler::class,
                Helper\ServerUrlMiddleware::class,
                Helper\BodyParams\BodyParamsMiddleware::class,
            ],
            'priority'   => 10000,
        ],

        'routing' => [
            'middleware' => [
                ApplicationFactory::ROUTING_MIDDLEWARE,
                Helper\UrlHelperMiddleware::class,
                MyApp\Action\AuthenticationAction::class,
                MyApp\Action\AuthorizationAction::class,
                ApplicationFactory::DISPATCH_MIDDLEWARE,
            ],
            'priority'   => 1,
        ],

        'final' => [
            'middleware' => [
                Zend\Stratigility\NoopFinalHandler::class
            ],
            'priority'   => -10000,
        ],
    ],
];

Edit - I removed the error => true from the bottom pipeline group and it didn't help. Config changed to reflect that.

weierophinney commented 7 years ago

First of: the NoopFinalHandler should be used as a FinalHandler, and not as part of the middleware pipeline.

Second, I would likely put the ServerUrlMiddleware outside the error handler (i.e., pipe it earlier), as it is only updating the ServerUrl helper with the request URI, and will not raise an exception.

Third, you should add some sort of middleware to pipe following the dispatch middleware for the purpose of returning a 404 response. We include an implementation in Stratigility (Zend\Stratigility\Middleware\NotFoundHandler), but you'll likely want to create your own implementation for purposes of providing a templated response. (This is planned for Expressive 1.1, which should land likely early next week.)

Try the above suggestions, and let me know if they resolve the issue.

moderndeveloperllc commented 7 years ago

Issue unresolved. I have a gist of the output if that helps. The Exception is being thrown in MyApp\Action\AuthenticationAction::class and being caught in Dispatch::dispatchCallableMiddleware() on line 221.

  1. I am now using 'Zend\Expressive\FinalHandler' => Zend\Stratigility\NoopFinalHandler::class in one of the config files. and have removed it from the pipeline.
  2. I switched the middleware order as suggested.
  3. The middleware MyApp\Action\AuthenticationAction::class handles the 404's early. They are templated in my ErrorResponseGenerator implementation:
if (!$route instanceof RouteResult) {
    throw new BadRouteException('Route Not Found', 404);
}

Edit: The combined configs can be found in this gist. (I sorted the invokables and factories for easier reading.)

moderndeveloperllc commented 7 years ago

@weierophinney I'll close this now as it's an Expressive issue that will be fixed in 1.1 (or with $app->raiseThrowables() now).

weierophinney commented 7 years ago

404s cannot be handled early! You won't have a route result until after the routing middleware has executed, which is why I suggested pushing the 404 middleware to the last item in the pipeline.

I may have found the problem; after I get #85 merged and 1.3.1 released, I'd encourage you to try that. Also, read the RFC for Expressive 1.1 that I linked you to on another issue to get an idea of how you may want to setup your application. This will be easier with Expressive 1.1, but should be possible with a small amount of effort now.

moderndeveloperllc commented 7 years ago

@weierophinney Sorry for the confusion - by "early", I meant between routing and dispatch as opposed to the end of the pipeline. I did read the gist (and made a comment!) and was able to change over my code to a programmatic pipeline in about 20 minutes. I had already changed the middleware to the hybrid invoke/process scheme when composer updated me to 1.3. I'm fully functioning now and ready for expressive 1.1 and stratagility 2 when it comes! My middleware extends Interop\Http\Middleware\ServerMiddlewareInterface, I return $delegate->process(), et al. My index is now:

<SNIP>
/** @var Application $engine */
$engine = $container->get(Application::class);
$engine->raiseThrowables();

// Piped before we identify a route
$engine->pipe(OriginalMessages::class);
$engine->pipe(Helper\ServerUrlMiddleware::class);
$engine->pipe(ErrorHandler::class);
$engine->pipe(Helper\BodyParams\BodyParamsMiddleware::class);

$engine->pipeRoutingMiddleware();

//Piped when we know the route, but before dispatching.
$engine->pipe(Helper\UrlHelperMiddleware::class);
$engine->pipe(MyApp\Action\AuthenticationAction::class);
$engine->pipe(MyApp\Action\AuthorizationAction::class);

$engine->pipeDispatchMiddleware();

$engine->route('/ping', MyApp\Action\PingAction::class, ['GET'], 'ping');
$engine->route('/session', MyApp\Action\SessionAction::class, ['POST', 'DELETE', 'PATCH'], 'session');
<SNIP>

I should point out that your v2 migration doc includes examples with

use Psr\Http\Middleware\DelegateInterface;
use Psr\Http\Middleware\ServerMiddlewareInterface;

Which don't exist (yet).