When a middleware instantiation leads to a `Interop\Container\Exception\ContainerException`, the exception is silenced, and execution continues

[Ocramius]

Test case will come, but defining a middleware pipeline config with invalid string services as middleware, or strings pointing to factories that throw exceptions, expressive simply ignores those exceptions and keeps going.

Test case will come, but basically, I'd expect:

Expressive did:

Also: sorry for the half-submitted initial issue: switching from windows to mac to windows to mac is messing with my ability to use a keyboard.

[Ocramius]

Turns out that this is a bug caused by the stratigility Dispatch.

Specifically, stratigility will simply catch any Throwable and then call the next middleware, skipping on the current one.

This makes sense if $next is a middleware designed for error handling, but not otherwise.

I'm unsure if this is expressive configuring the middleware pipeline incorrectly, or if stratiglity is wrong upfront.

Here's an expressive application showing the issue in practice:

<?php

use Zend\Expressive\Application;
use Zend\Expressive\Container\ApplicationFactory;
use Zend\ServiceManager\ServiceManager;

require_once __DIR__ . '/../vendor/autoload.php';

$config = [
    'debug' => false,
    'dependencies' => [
        'factories' => [
            Application::class => ApplicationFactory::class,
            'failing-middleware' => function () {
                throw new \Exception('This will fail at instantiation');
            },
            'index' => function () {
                die('this middleware should NEVER be reached');
            },
        ],
    ],
    'middleware_pipeline' => [
        'foo' => [
            'middleware' => [
                'failing-middleware',
            ],
            'priority' => 100,
        ],
        'routing' => [
            'middleware' => [
                ApplicationFactory::ROUTING_MIDDLEWARE,
                ApplicationFactory::DISPATCH_MIDDLEWARE,
            ],
            'priority' => 1,
        ],
        'error' => [
            'middleware' => [
            ],
            'error'    => true,
            'priority' => -10000,
        ],
    ],
    'routes' => [
        [
            'name' => 'home',
            'path' => '/',
            'middleware' => 'index',
            'allowed_methods' => ['GET'],
        ],
    ],
];

$config['dependencies']['services']['config'] = $config;

(new ServiceManager($config['dependencies']))->get(Application::class)->run();

Note: adding an error middleware doesn't seem to make any difference

[Ocramius]

As discussed with @xtreamwayz, these are the versions involved in this bug:

• zendframework/zend-expressive:1.0.5
• zendframework/zend-stratigility:1.3.1

Some wild guesses:

12:04:28 xtreamwayz | ocramius: I'm guessing you are using stratigility 1.3 and expressive 1.1 is not out. I don't think there is an option for raiseThrowables yet in 1.0.5. 12:04:48 xtreamwayz | Probably if you update composer and restrict to straigility 1.2 might fix it.

[moderndeveloperllc]

Going to Stratigility 1.3 can cause issues if you are using a config-driven setup vs. a procedural approach. Config-driven raise_throwables was implemented in develop with this commit, but that's 1.1 of course. Not sure if @weierophinney is going to want to backport this, or just set the 1.0.x branch to Stratigility 1.2. FWIW, it should also work fine if you add the line to your index.php:

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

[Ocramius]

Indeed, hijacking raiseThrowables to set it as the default fixes the issue for me, but will not trigger the error middleware.

In my opinion, the entire idea of the error middleware being piped with all the others is kinda useless: probably to be deprecated in favor of just a $errorMiddleware($request, $response, $next) with the exception to be injected as a request attribute.

[weierophinney]

In my opinion, the entire idea of the error middleware being piped with all the others is kinda useless: probably to be deprecated in favor of just a $errorMiddleware($request, $response, $next) with the exception to be injected as a request attribute.

This is the direction we're moving for 1.1, actually, though without using request attributes.

What will happen instead is that the raise_throwables flag will be enabled by default with new applications, and users will use middleware for handling errors.

Error conditions will typically just be exceptions:

function ($request, $response, $next)
{
    throw new RuntimeException('Oh, no, I cannot handle this!');
}

You'll then have middleware in an outer layer that handles exceptions. We will ship a default implementation, but you can also use your own, or stack multiple such middleware each capable of handling specific exceptions:

function ($request, $response, $next)
{
    try {
        $response = $next($request, $response);
        return $response;
    } catch (UnauthorizedException $e) {
    } catch (Throwable $e) {
        throw $e;
    }

    // Handle UnauthorizedException; maybe display a login form?
    // ...
    return $someNewResponse;
}

Stratigility's "error middleware" concept from 1.0 is deprecated starting in 1.3, and will be removed entirely in 2.0 in favor of this concept.

Regarding your specific error, I was able to reproduce it. For those that wonder, the results of executing the above application are:

this middleware should NEVER be reached

What SHOULD happen is that the failing-middleware callback will raise an exception, and the Stratigility Next/Dispatch implementation will call the first error middleware in the stack (which is empty currently). My suspicion is that this is a subtle bug in Stratigility, and I'll see if I can find a solution for it today.

Thanks for the detailed report, @Ocramius!

[weierophinney]

Found the issue, and it's in Stratigility.

The crux of the issue is that internally, Dispatch checks to see if the middleware provided implements the http-middleware interfaces; if so, it dispatches it differently. That "differently" means that the $err argument is ignored from that point forward.

Expressive's ErrorMiddlewarePipe does not implement http-middleware interfaces, but it composes a MiddlewarePipe... and in Stratigility 1.3, that class does implement these interfaces. That means that as the middleware the ErrorMiddlewarePipe composes is dispatched, the error is silently dropped.

The solution will likely be to test in Dispatch for the following:

if ($route->handler instanceof ServerMiddlewareInterface
    && ! $route->handler instanceof MiddlewarePipe
) {
    return $this->dispatchInteropMiddleware(/* ... */);
}

I'll open an issue on Stratigility, and, once fixed, we'll do another 1.0.X release of Expressive that requires that version or above (or versions prior to the 1.3 series).

[weierophinney]

Confirmed: the change I suggested does work. I'll get a test case written for Stratigility, and submit a patch there.

[Ocramius]

@weierophinney will this be fixed by zendframework/zend-stratigility#95? Specifically: does the example application above crash correctly when run against that patch? (sorry, not on a dev environment atm)

[weierophinney]

@Ocramius yes, application crashes correctly when run against that patch; I've tested against your example both with and without the patch, and the patch makes it behave correctly.

[Ocramius]

Fixed in https://github.com/zendframework/zend-stratigility/releases/tag/1.3.2 via zendframework/zend-stratigility#95.

Closing as duplicate here.