Merge branch '4.x' into patch-prophecies

slimphp · Dec 22, 2019 · f4617ae · f4617ae
2 parents bbe28d2 + 4b83e02
commit f4617ae
Show file tree

Hide file tree

Showing 6 changed files with 143 additions and 14 deletions.
diff --git a/Slim/Error/AbstractErrorRenderer.php b/Slim/Error/AbstractErrorRenderer.php
@@ -9,7 +9,9 @@
 
 namespace Slim\Error;
 
+use Slim\Exception\HttpException;
 use Slim\Interfaces\ErrorRendererInterface;
+use Throwable;
 
 /**
  * Abstract Slim application error renderer
@@ -19,4 +21,31 @@
  */
 abstract class AbstractErrorRenderer implements ErrorRendererInterface
 {
+    /**
+     * @var string
+     */
+    protected $defaultErrorTitle = 'Slim Application Error';
+
+    /**
+     * @var string
+     */
+    protected $defaultErrorDescription = 'A website error has occurred. Sorry for the temporary inconvenience.';
+
+    protected function getErrorTitle(Throwable $exception): string
+    {
+        if ($exception instanceof HttpException) {
+            return $exception->getTitle();
+        }
+
+        return $this->defaultErrorTitle;
+    }
+
+    protected function getErrorDescription(Throwable $exception): string
+    {
+        if ($exception instanceof HttpException) {
+            return $exception->getDescription();
+        }
+
+        return $this->defaultErrorDescription;
+    }
 }
diff --git a/Slim/Error/Renderers/HtmlErrorRenderer.php b/Slim/Error/Renderers/HtmlErrorRenderer.php
@@ -24,17 +24,15 @@ class HtmlErrorRenderer extends AbstractErrorRenderer
      */
     public function __invoke(Throwable $exception, bool $displayErrorDetails): string
     {
-        $title = 'Slim Application Error';
-
         if ($displayErrorDetails) {
             $html = '<p>The application could not run because of the following error:</p>';
             $html .= '<h2>Details</h2>';
             $html .= $this->renderExceptionFragment($exception);
         } else {
-            $html = '<p>A website error has occurred. Sorry for the temporary inconvenience.</p>';
+            $html = "<p>{$this->getErrorDescription($exception)}</p>";
         }
 
-        return $this->renderHtmlBody($title, $html);
+        return $this->renderHtmlBody($this->getErrorTitle($exception), $html);
     }
 
     /**

diff --git a/Slim/Error/Renderers/JsonErrorRenderer.php b/Slim/Error/Renderers/JsonErrorRenderer.php
@@ -24,7 +24,7 @@ class JsonErrorRenderer extends AbstractErrorRenderer
      */
     public function __invoke(Throwable $exception, bool $displayErrorDetails): string
     {
-        $error = ['message' => $exception->getMessage()];
+        $error = ['message' => $this->getErrorTitle($exception)];
 
         if ($displayErrorDetails) {
             $error['exception'] = [];

diff --git a/Slim/Error/Renderers/PlainTextErrorRenderer.php b/Slim/Error/Renderers/PlainTextErrorRenderer.php
@@ -24,12 +24,15 @@ class PlainTextErrorRenderer extends AbstractErrorRenderer
      */
     public function __invoke(Throwable $exception, bool $displayErrorDetails): string
     {
-        $text = "Slim Application Error:\n";
-        $text .= $this->formatExceptionFragment($exception);
+        $text = "{$this->getErrorTitle($exception)}\n";
 
-        while ($displayErrorDetails && $exception = $exception->getPrevious()) {
-            $text .= "\nPrevious Error:\n";
+        if ($displayErrorDetails) {
             $text .= $this->formatExceptionFragment($exception);
+
+            while ($exception = $exception->getPrevious()) {
+                $text .= "\nPrevious Error:\n";
+                $text .= $this->formatExceptionFragment($exception);
+            }
         }
 
         return $text;

diff --git a/Slim/Error/Renderers/XmlErrorRenderer.php b/Slim/Error/Renderers/XmlErrorRenderer.php
@@ -25,7 +25,7 @@ class XmlErrorRenderer extends AbstractErrorRenderer
     public function __invoke(Throwable $exception, bool $displayErrorDetails): string
     {
         $xml = '<' . '?xml version="1.0" encoding="UTF-8" standalone="yes"?' . ">\n";
-        $xml .= "<error>\n  <message>" . $this->createCdataSection($exception->getMessage()) . "</message>\n";
+        $xml .= "<error>\n  <message>" . $this->createCdataSection($this->getErrorTitle($exception)) . "</message>\n";
 
         if ($displayErrorDetails) {
             do {

diff --git a/tests/Error/AbstractErrorRendererTest.php b/tests/Error/AbstractErrorRendererTest.php
@@ -10,12 +10,14 @@
 namespace Slim\Tests\Error;
 
 use Exception;
+use PHPUnit\Framework\MockObject\MockObject;
 use ReflectionClass;
 use RuntimeException;
 use Slim\Error\Renderers\HtmlErrorRenderer;
 use Slim\Error\Renderers\JsonErrorRenderer;
 use Slim\Error\Renderers\PlainTextErrorRenderer;
 use Slim\Error\Renderers\XmlErrorRenderer;
+use Slim\Exception\HttpException;
 use Slim\Tests\TestCase;
 use stdClass;
 
@@ -28,6 +30,7 @@ public function testHTMLErrorRendererDisplaysErrorDetails()
         $output = $renderer->__invoke($exception, true);
 
         $this->assertRegExp('/.*The application could not run because of the following error:.*/', $output);
+        $this->assertContains('Oops..', $output);
     }
 
     public function testHTMLErrorRendererNoErrorDetails()
@@ -37,6 +40,7 @@ public function testHTMLErrorRendererNoErrorDetails()
         $output = $renderer->__invoke($exception, false);
 
         $this->assertRegExp('/.*A website error has occurred. Sorry for the temporary inconvenience.*/', $output);
+        $this->assertNotContains('Oops..', $output);
     }
 
     public function testHTMLErrorRendererRenderFragmentMethod()
@@ -56,6 +60,30 @@ public function testHTMLErrorRendererRenderFragmentMethod()
         $this->assertRegExp('/.*Line*/', $output);
     }
 
+    public function testHTMLErrorRendererRenderHttpException()
+    {
+        $exceptionTitle = 'title';
+        $exceptionDescription = 'description';
+
+        $httpExceptionProphecy = $this->prophesize(HttpException::class);
+
+        $httpExceptionProphecy
+            ->getTitle()
+            ->willReturn($exceptionTitle)
+            ->shouldBeCalledOnce();
+
+        $httpExceptionProphecy
+            ->getDescription()
+            ->willReturn($exceptionDescription)
+            ->shouldBeCalledOnce();
+
+        $renderer = new HtmlErrorRenderer();
+        $output = $renderer->__invoke($httpExceptionProphecy->reveal(), false);
+
+        $this->assertContains($exceptionTitle, $output, 'Should contain http exception title');
+        $this->assertContains($exceptionDescription, $output, 'Should contain http exception description');
+    }
+
     public function testJSONErrorRendererDisplaysErrorDetails()
     {
         $exception = new Exception('Oops..');
@@ -67,7 +95,7 @@ public function testJSONErrorRendererDisplaysErrorDetails()
 
         $fragment = $method->invoke($renderer, $exception);
         $output = json_encode(json_decode($renderer->__invoke($exception, true)));
-        $expectedString = json_encode(['message' => 'Oops..', 'exception' => [$fragment]]);
+        $expectedString = json_encode(['message' => 'Slim Application Error', 'exception' => [$fragment]]);
 
         $this->assertEquals($output, $expectedString);
     }
@@ -79,7 +107,7 @@ public function testJSONErrorRendererDoesNotDisplayErrorDetails()
         $renderer = new JsonErrorRenderer();
         $output = json_encode(json_decode($renderer->__invoke($exception, false)));
 
-        $this->assertEquals($output, json_encode(['message' => 'Oops..']));
+        $this->assertEquals($output, json_encode(['message' => 'Slim Application Error']));
     }
 
     public function testJSONErrorRendererDisplaysPreviousError()
@@ -99,11 +127,33 @@ public function testJSONErrorRendererDisplaysPreviousError()
             $method->invoke($renderer, $previousException),
         ];
 
-        $expectedString = json_encode(['message' => 'Oops..', 'exception' => $fragments]);
+        $expectedString = json_encode(['message' => 'Slim Application Error', 'exception' => $fragments]);
 
         $this->assertEquals($output, $expectedString);
     }
 
+    public function testJSONErrorRendererRenderHttpException()
+    {
+        $exceptionTitle = 'title';
+
+        $httpExceptionProphecy = $this->prophesize(HttpException::class);
+
+        $httpExceptionProphecy
+            ->getTitle()
+            ->willReturn($exceptionTitle)
+            ->shouldBeCalledOnce();
+
+        $renderer = new JsonErrorRenderer();
+        $output = json_encode(json_decode($renderer->__invoke($httpExceptionProphecy->reveal(), false)));
+
+        $this->assertEquals(
+            $output,
+            json_encode(['message' => $exceptionTitle]),
+            'Should contain http exception title'
+        );
+    }
+
+
     public function testXMLErrorRendererDisplaysErrorDetails()
     {
         $previousException = new RuntimeException('Oops..');
@@ -114,9 +164,30 @@ public function testXMLErrorRendererDisplaysErrorDetails()
         /** @var stdClass $output */
         $output = simplexml_load_string($renderer->__invoke($exception, true));
 
-        $this->assertEquals($output->message[0], 'Ooops...');
+        $this->assertEquals($output->message[0], 'Slim Application Error');
         $this->assertEquals((string) $output->exception[0]->type, 'Exception');
+        $this->assertEquals((string) $output->exception[0]->message, 'Ooops...');
         $this->assertEquals((string) $output->exception[1]->type, 'RuntimeException');
+        $this->assertEquals((string) $output->exception[1]->message, 'Oops..');
+    }
+
+    public function testXMLErrorRendererRenderHttpException()
+    {
+        $exceptionTitle = 'title';
+
+        $httpExceptionProphecy = $this->prophesize(HttpException::class);
+
+        $httpExceptionProphecy
+            ->getTitle()
+            ->willReturn($exceptionTitle)
+            ->shouldBeCalledOnce();
+
+        $renderer = new XmlErrorRenderer();
+
+        /** @var stdClass $output */
+        $output = simplexml_load_string($renderer->__invoke($httpExceptionProphecy->reveal(), true));
+
+        $this->assertEquals($output->message[0], $exceptionTitle, 'Should contain http exception title');
     }
 
     public function testPlainTextErrorRendererFormatFragmentMethod()
@@ -146,4 +217,32 @@ public function testPlainTextErrorRendererDisplaysErrorDetails()
 
         $this->assertRegExp('/Ooops.../', $output);
     }
+
+    public function testPlainTextErrorRendererNotDisplaysErrorDetails()
+    {
+        $previousException = new RuntimeException('Oops..');
+        $exception = new Exception('Ooops...', 0, $previousException);
+
+        $renderer = new PlainTextErrorRenderer();
+        $output = $renderer->__invoke($exception, false);
+
+        $this->assertEquals("Slim Application Error\n", $output, 'Should show only one string');
+    }
+
+    public function testPlainTextErrorRendererRenderHttpException()
+    {
+        $exceptionTitle = 'title';
+
+        $httpExceptionProphecy = $this->prophesize(HttpException::class);
+
+        $httpExceptionProphecy
+            ->getTitle()
+            ->willReturn($exceptionTitle)
+            ->shouldBeCalledOnce();
+
+        $renderer = new PlainTextErrorRenderer();
+        $output = $renderer->__invoke($httpExceptionProphecy->reveal(), true);
+
+        $this->assertContains($exceptionTitle, $output, 'Should contain http exception title');
+    }
 }