Merge branch 'fix-stack-queue'

CHANGELOG.md

@@ -1,3 +1,7 @@
+# 2.6.0
+
+* Fix 2.4.0 pushHandler changing the order of handlers.
+
 # 2.5.1
 
 * Fix error messaging in a rare case.

README.md

@@ -62,7 +62,7 @@ If you are not using any of these frameworks, here's a very simple way to instal
 
     ```php
     $whoops = new \Whoops\Run;
-    $whoops->prependHandler(new \Whoops\Handler\PrettyPageHandler);
+    $whoops->pushHandler(new \Whoops\Handler\PrettyPageHandler);
     $whoops->register();
     ```
 

examples/example-ajax-only.php

@@ -26,7 +26,7 @@ $run = new Run();
 
 // We want the error page to be shown by default, if this is a
 // regular request, so that's the first thing to go into the stack:
-$run->prependHandler(new PrettyPageHandler());
+$run->pushHandler(new PrettyPageHandler());
 
 // Now, we want a second handler that will run before the error page,
 // and immediately return an error message in JSON format, if something
@@ -42,8 +42,8 @@ if (\Whoops\Util\Misc::isAjaxRequest()) {
     // tl;dr: error[] becomes errors[[]]
    $jsonHandler->setJsonApi(true);
 
-    // And prepend it into the stack:
-    $run->prependHandler($jsonHandler);
+    // And push it into the stack:
+    $run->pushHandler($jsonHandler);
 }
 
 // That's it! Register Whoops and throw a dummy exception:

examples/example.php

@@ -50,10 +50,10 @@ $handler->addDataTableCallback('Details', function(\Whoops\Exception\Inspector $
     return $data;
 });
 
-$run->prependHandler($handler);
+$run->pushHandler($handler);
 
 // Example: tag all frames inside a function with their function name
-$run->prependHandler(function ($exception, $inspector, $run) {
+$run->pushHandler(function ($exception, $inspector, $run) {
 
     $inspector->getFrames()->map(function ($frame) {
 

src/Whoops/Run.php

@@ -29,7 +29,7 @@ final class Run implements RunInterface
     /**
      * @var HandlerInterface[]
      */
-    private $handlerQueue = [];
+    private $handlerStack = [];
 
     private $silencedPatterns = [];
 
@@ -41,105 +41,64 @@ final class Run implements RunInterface
     }
 
     /**
-     * Prepends a handler to the start of the queue
-     *
-     * @throws InvalidArgumentException  If argument is not callable or instance of HandlerInterface
-     * @param  Callable|HandlerInterface $handler
-     * @return Run
-     * @deprecated use appendHandler and prependHandler instead
-     */
-    public function pushHandler($handler)
-    {
-        return $this->prependHandler($handler);
-    }
-
-    /**
-     * Appends a handler to the end of the queue
-     *
-     * @throws InvalidArgumentException  If argument is not callable or instance of HandlerInterface
-     * @param  Callable|HandlerInterface $handler
-     * @return Run
+     * Explicitly request your handler runs as the last of all currently registered handlers
      */
     public function appendHandler($handler)
     {
-        array_push($this->handlerQueue, $this->resolveHandler($handler));
+        array_unshift($this->handlerStack, $this->resolveHandler($handler));
         return $this;
     }
 
     /**
-     * Prepends a handler to the start of the queue
+     * Explicitly request your handler runs as the first of all currently registered handlers
+     */
+    public function prependHandler($handler)
+    {
+        return $this->pushHandler($handler);
+    }
+
+    /**
+     * Register your handler as the last of all currently registered handlers.
+     * Prefer using appendHandler and prependHandler for clarity.
      *
      * @throws InvalidArgumentException  If argument is not callable or instance of HandlerInterface
      * @param  Callable|HandlerInterface $handler
      * @return Run
      */
-    public function prependHandler($handler)
+    public function pushHandler($handler)
     {
-        array_unshift($this->handlerQueue, $this->resolveHandler($handler));
+        $this->handlerStack[] = $this->resolveHandler($handler);
         return $this;
     }
 
     /**
-     * Create a CallbackHandler from callable and throw if handler is invalid
-     *
-     * @throws InvalidArgumentException  If argument is not callable or instance of HandlerInterface
-     * @param Callable|HandlerInterface $handler
-     * @return HandlerInterface
-     */
-    private function resolveHandler($handler)
-    {
-        if (is_callable($handler)) {
-            $handler = new CallbackHandler($handler);
-        }
-
-        if (!$handler instanceof HandlerInterface) {
-            throw new InvalidArgumentException(
-                "Argument to " . __METHOD__ . " must be a callable, or instance of "
-                . "Whoops\\Handler\\HandlerInterface"
-            );
-        }
-
-        return $handler;
-    }
-
-    /**
-     * Removes the last handler in the queue and returns it.
+     * Removes the last handler in the stack and returns it.
      * Returns null if there"s nothing else to pop.
      * @return null|HandlerInterface
      */
     public function popHandler()
     {
-        return array_pop($this->handlerQueue);
-    }
-
-    /**
-     * Removes the first handler in the queue and returns it.
-     * Returns null if there"s nothing else to shift.
-     * @return null|HandlerInterface
-     */
-    public function shiftHandler()
-    {
-        return array_shift($this->handlerQueue);
+        return array_pop($this->handlerStack);
     }
 
     /**
      * Returns an array with all handlers, in the
-     * order they were added to the queue.
+     * order they were added to the stack.
      * @return array
      */
     public function getHandlers()
     {
-        return $this->handlerQueue;
+        return $this->handlerStack;
     }
 
     /**
-     * Clears all handlers in the handlerQueue, including
+     * Clears all handlers in the handlerStack, including
      * the default PrettyPage handler.
      * @return Run
      */
     public function clearHandlers()
     {
-        $this->handlerQueue = [];
+        $this->handlerStack = [];
         return $this;
     }
 
@@ -303,13 +262,13 @@ final class Run implements RunInterface
         // we might want to send it straight away to the client,
         // or return it silently.
         $this->system->startOutputBuffering();
-        
+
         // Just in case there are no handlers:
         $handlerResponse = null;
         $handlerContentType = null;
 
         try {
-            foreach ($this->handlerQueue as $handler) {
+            foreach (array_reverse($this->handlerStack) as $handler) {
                 $handler->setRun($this);
                 $handler->setInspector($inspector);
                 $handler->setException($exception);
@@ -441,6 +400,22 @@ final class Run implements RunInterface
      */
     private $canThrowExceptions = true;
 
+    private function resolveHandler($handler)
+    {
+        if (is_callable($handler)) {
+            $handler = new CallbackHandler($handler);
+        }
+
+        if (!$handler instanceof HandlerInterface) {
+            throw new InvalidArgumentException(
+                  "Handler must be a callable, or instance of "
+                . "Whoops\\Handler\\HandlerInterface"
+            );
+        }
+
+        return $handler;
+    }
+
     /**
      * Echo something to the browser
      * @param  string $output

tests/Whoops/RunTest.php

@@ -82,8 +82,8 @@ class RunTest extends TestCase
         $handlerOne = $this->getHandler();
         $handlerTwo = $this->getHandler();
 
-        $run->prependHandler($handlerOne);
-        $run->prependHandler($handlerTwo);
+        $run->pushHandler($handlerOne);
+        $run->pushHandler($handlerTwo);
 
         $handlers = $run->getHandlers();
 
@@ -99,7 +99,7 @@ class RunTest extends TestCase
     public function testPushInvalidHandler()
     {
         $run = $this->getRunInstance();
-        $run->prependHandler($banana = 'actually turnip');
+        $run->pushHandler($banana = 'actually turnip');
     }
 
     /**
@@ -108,7 +108,7 @@ class RunTest extends TestCase
     public function testPushClosureBecomesHandler()
     {
         $run = $this->getRunInstance();
-        $run->prependHandler(function () {});
+        $run->pushHandler(function () {});
         $this->assertInstanceOf('Whoops\\Handler\\CallbackHandler', $run->popHandler());
     }
 
@@ -124,9 +124,9 @@ class RunTest extends TestCase
         $handlerTwo   = $this->getHandler();
         $handlerThree = $this->getHandler();
 
-        $run->appendHandler($handlerOne);
-        $run->appendHandler($handlerTwo);
-        $run->appendHandler($handlerThree);
+        $run->pushHandler($handlerOne);
+        $run->pushHandler($handlerTwo);
+        $run->pushHandler($handlerThree);
 
         $this->assertSame($handlerThree, $run->popHandler());
         $this->assertSame($handlerTwo, $run->popHandler());
@@ -164,7 +164,7 @@ class RunTest extends TestCase
         $run->register();
 
         $handler = $this->getHandler();
-        $run->prependHandler($handler);
+        $run->pushHandler($handler);
 
         $run->unregister();
         throw $this->getException("I'm not supposed to be caught!");
@@ -183,17 +183,17 @@ class RunTest extends TestCase
         $handlerThree = $this->getHandler();
         $handlerFour  = $this->getHandler();
 
-        $run->appendHandler($handlerOne);
-        $run->appendHandler($handlerTwo);
+        $run->pushHandler($handlerOne);
+        $run->prependHandler($handlerTwo);
         $run->appendHandler($handlerThree);
         $run->appendHandler($handlerFour);
 
         $handlers = $run->getHandlers();
 
-        $this->assertSame($handlers[0], $handlerOne);
-        $this->assertSame($handlers[1], $handlerTwo);
-        $this->assertSame($handlers[2], $handlerThree);
-        $this->assertSame($handlers[3], $handlerFour);
+        $this->assertSame($handlers[0], $handlerFour);
+        $this->assertSame($handlers[1], $handlerThree);
+        $this->assertSame($handlers[2], $handlerOne);
+        $this->assertSame($handlers[3], $handlerTwo);
     }
 
     /**
@@ -218,9 +218,9 @@ class RunTest extends TestCase
         $handlerThree->shouldReceive('handle')
             ->andReturnUsing(function () use ($order) { $order[] = 3; });
 
-        $run->prependHandler($handlerOne);
-        $run->prependHandler($handlerTwo);
-        $run->prependHandler($handlerThree);
+        $run->pushHandler($handlerOne);
+        $run->pushHandler($handlerTwo);
+        $run->pushHandler($handlerThree);
 
         // Get an exception to be handled, and verify that the handlers
         // are given the handler, and in the inverse order they were
@@ -238,20 +238,38 @@ class RunTest extends TestCase
 
         $handlerOne = $this->getHandler();
         $handlerTwo = $this->getHandler();
+        $handlerThree = $this->getHandler();
+        $handlerFour = $this->getHandler();
 
-        $run->prependHandler($handlerOne);
+        $run->pushHandler($handlerOne);
         $run->prependHandler($handlerTwo);
+        $run->appendHandler($handlerThree);
+        $run->appendHandler($handlerFour);
 
         $test = $this;
-        $handlerOne
+        $handlerFour
             ->shouldReceive('handle')
             ->andReturnUsing(function () use ($test) {
-                $test->fail('$handlerOne should not be called');
+                $test->fail('$handlerFour should not be called');
+            });
+
+        $handlerThree
+            ->shouldReceive('handle')
+            ->andReturn(Handler::LAST_HANDLER);
+
+        $twoRan = false;
+
+        $handlerOne
+            ->shouldReceive('handle')
+            ->andReturnUsing(function () use ($test, &$twoRan) {
+                $test->assertTrue($twoRan);
             });
 
         $handlerTwo
             ->shouldReceive('handle')
-            ->andReturn(Handler::LAST_HANDLER);
+            ->andReturnUsing(function () use (&$twoRan) {
+                $twoRan = true;
+            });
 
         $run->handleException($this->getException());
 
@@ -268,7 +286,7 @@ class RunTest extends TestCase
         $run->register();
 
         $handler = $this->getHandler();
-        $run->prependHandler($handler);
+        $run->pushHandler($handler);
 
         $test = $this;
         $handler
@@ -289,7 +307,7 @@ class RunTest extends TestCase
         $run->register();
 
         $handler = $this->getHandler();
-        $run->prependHandler($handler);
+        $run->pushHandler($handler);
 
         $test = $this;
         $handler
@@ -318,7 +336,7 @@ class RunTest extends TestCase
         $run->register();
 
         $handler = $this->getHandler();
-        $run->prependHandler($handler);
+        $run->pushHandler($handler);
 
         $test = $this;
         $handler
@@ -344,7 +362,7 @@ class RunTest extends TestCase
         $run->register();
 
         $handler = $this->getHandler();
-        $run->prependHandler($handler);
+        $run->pushHandler($handler);
 
         $test = $this;
         $handler
@@ -367,7 +385,7 @@ class RunTest extends TestCase
         $run->register();
 
         $handler = $this->getHandler();
-        $run->prependHandler($handler);
+        $run->pushHandler($handler);
 
         @strpos();
 
@@ -394,7 +412,7 @@ class RunTest extends TestCase
             $this->assertSame(99, $e->getLine());
         }
     }
-    
+
     /**
      * @covers Whoops\Run::handleException
      * @covers Whoops\Run::writeToOutput
@@ -402,7 +420,7 @@ class RunTest extends TestCase
     public function testOutputIsSent()
     {
         $run = $this->getRunInstance();
-        $run->prependHandler(function () {
+        $run->pushHandler(function () {
             echo "hello there";
         });
 
@@ -419,7 +437,7 @@ class RunTest extends TestCase
     {
         $run = $this->getRunInstance();
         $run->writeToOutput(false);
-        $run->prependHandler(function () {
+        $run->pushHandler(function () {
             echo "hello there";
         });