Merge branch 'hotfix/next-as-delegate-with-error'

Close #85

CHANGELOG.md

@@ -6,6 +6,29 @@ Versions prior to 1.0 were originally released as `phly/conduit`; please visit
 its [CHANGELOG](https://github.com/phly/conduit/blob/master/CHANGELOG.md) for
 details.
 
+## 1.3.1 - 2016-11-10
+
+### Added
+
+- Nothing.
+
+### Deprecated
+
+- Nothing.
+
+### Removed
+
+- Nothing.
+
+### Fixed
+
+- [#85](https://github.com/zendframework/zend-stratigility/pull/85) fixes an
+  issue with how the `$done` or `$nextDelegate` is invoked by `Next` when an
+  error is present. Previously, the class was detecting a `Next` instance as an
+  http-interop `DelegateInterface` instance and dropping the error; this would
+  then mean if the instance contained error middleware, it would never be
+  dispatched.
+
 ## 1.3.0 - 2016-11-10
 
 ### Added

src/Next.php

@@ -396,7 +396,9 @@ private function dispatchNextDelegate(
         ResponseInterface $response = null,
         $err = null
     ) {
-        if ($nextDelegate instanceof DelegateInterface) {
+        if ($nextDelegate instanceof DelegateInterface
+            && (! $nextDelegate instanceof Next || $err === null)
+        ) {
             return $nextDelegate->process($request);
         }
 

test/MiddlewarePipeTest.php

@@ -638,6 +638,27 @@ public function testWillDecorateCallableMiddlewareAsInteropMiddlewareIfResponseP
         $this->assertAttributeSame($this->response, 'responsePrototype', $test);
     }
 
+    /**
+     * @group http-interop
+     */
+    public function testWillNotDecorateCallableMiddlewareAsInteropMiddlewareIfResponsePrototypeIsNotPresent()
+    {
+        $pipeline = new MiddlewarePipe();
+
+        $middleware = function () {
+        };
+        $pipeline->pipe($middleware);
+
+        $r = new ReflectionProperty($pipeline, 'pipeline');
+        $r->setAccessible(true);
+        $queue = $r->getValue($pipeline);
+
+        $route = $queue->dequeue();
+        $test = $route->handler;
+        $this->assertNotInstanceOf(CallableMiddlewareWrapper::class, $test);
+        $this->assertInternalType('callable', $test);
+    }
+
     /**
      * @todo Remove with 2.0.0
      */

test/NextTest.php

@@ -910,4 +910,39 @@ public function testEnablingRaiseThrowablesFlagWillCauseInvocationToRaiseMiddlew
 
         $this->assertTrue($triggered, 'Deprecation notice not triggered');
     }
+
+    /**
+     * @todo Remove for 2.0.0
+     * @group error-handling
+     */
+    public function testNestedNextInvocationWithAnErrorShouldDispatchErrorMiddleware()
+    {
+        $internalQueue = clone $this->queue;
+        $internalQueue->enqueue(new Route('/', function ($request, $response, $next) {
+            return $next($request, $response, 'ERROR');
+        }));
+
+        $nextDelegateQueue = clone $this->queue;
+        $nextDelegateQueue->enqueue(new Route('/', function ($err, $request, $response, $next) {
+            $response->getBody()->write('ERROR DETECTED');
+            return $response->withStatus(599);
+        }));
+
+        $finalDelegate = $this->prophesize(DelegateInterface::class);
+        $finalDelegate->process(Argument::any())->shouldNotBeCalled();
+
+        $nextDelegate = new Next($nextDelegateQueue, $finalDelegate->reveal());
+        $internalNext = new Next($internalQueue, $nextDelegate);
+
+        set_error_handler(function ($errno, $errstr) {
+            return false !== strstr($errstr, 'error middleware is deprecated');
+        }, E_USER_DEPRECATED);
+
+        $response = $internalNext($this->request, $this->response);
+
+        restore_error_handler();
+
+        $this->assertEquals(599, $response->getStatusCode());
+        $this->assertEquals('ERROR DETECTED', (string) $response->getBody());
+    }
 }