[4.x] Use callable resolver in the error handler to resolve error renderers

Slim/CallableResolver.php

@@ -14,6 +14,7 @@
 use Psr\Http\Server\RequestHandlerInterface;
 use RuntimeException;
 use Slim\Interfaces\CallableResolverInterface;
+use Slim\Interfaces\ErrorRendererInterface;
 
 /**
  * This class resolves a string of the format 'class:method' into a closure
@@ -75,13 +76,17 @@ public function resolve($toResolve): callable
             // if no method has been specified explicitly
             if ($instance instanceof RequestHandlerInterface && $method === null) {
                 $method = 'handle';
+            } elseif ($instance instanceof ErrorRendererInterface && $method === null) {
+                $method = 'render';
             }
 
             $resolved = [$instance, $method ?? '__invoke'];
         }
 
         if ($resolved instanceof RequestHandlerInterface) {
             $resolved = [$resolved, 'handle'];
+        } elseif ($resolved instanceof ErrorRendererInterface) {
+            $resolved = [$resolved, 'render'];
         }
 
         if (!is_callable($resolved)) {

Slim/Handlers/ErrorHandler.php

@@ -19,6 +19,7 @@
 use Slim\Error\Renderers\XmlErrorRenderer;
 use Slim\Exception\HttpException;
 use Slim\Exception\HttpMethodNotAllowedException;
+use Slim\Interfaces\CallableResolverInterface;
 use Slim\Interfaces\ErrorHandlerInterface;
 use Slim\Interfaces\ErrorRendererInterface;
 use Throwable;
@@ -32,12 +33,12 @@
 class ErrorHandler implements ErrorHandlerInterface
 {
     /**
-     * @var string
+     * @var ErrorRendererInterface|string|callable
      */
     protected $defaultErrorRenderer = HtmlErrorRenderer::class;
 
     /**
-     * @var string[]
+     * @var array
      */
     protected $errorRenderers = [
         'application/json' => JsonErrorRenderer::class,
@@ -83,7 +84,7 @@ class ErrorHandler implements ErrorHandlerInterface
     protected $exception;
 
     /**
-     * @var ErrorRendererInterface|string|null
+     * @var ErrorRendererInterface|callable|null
      */
     protected $renderer;
 
@@ -92,16 +93,23 @@ class ErrorHandler implements ErrorHandlerInterface
      */
     protected $statusCode;
 
+    /**
+     * @var CallableResolverInterface
+     */
+    protected $callableResolver;
+
     /**
      * @var ResponseFactoryInterface
      */
     protected $responseFactory;
 
     /**
-     * @param ResponseFactoryInterface $responseFactory
+     * @param CallableResolverInterface $callableResolver
+     * @param ResponseFactoryInterface  $responseFactory
      */
-    public function __construct(ResponseFactoryInterface $responseFactory)
+    public function __construct(CallableResolverInterface $callableResolver, ResponseFactoryInterface $responseFactory)
     {
+        $this->callableResolver = $callableResolver;
         $this->responseFactory = $responseFactory;
     }
 
@@ -203,11 +211,11 @@ protected function determineContentType(ServerRequestInterface $request): string
      * Determine which renderer to use based on content type
      * Overloaded $renderer from calling class takes precedence over all
      *
-     * @return ErrorRendererInterface
+     * @return callable
      *
      * @throws RuntimeException
      */
-    protected function determineRenderer(): ErrorRendererInterface
+    protected function determineRenderer(): callable
     {
         $renderer = $this->renderer;
 
@@ -231,26 +239,26 @@ protected function determineRenderer(): ErrorRendererInterface
             }
         }
 
-        return new $renderer();
+        return $this->callableResolver->resolve($renderer);
     }
 
     /**
      * Register an error renderer for a specific content-type
      *
-     * @param string $contentType   The content-type this renderer should be registered to
-     * @param string $errorRenderer The error renderer class name
+     * @param string  $contentType  The content-type this renderer should be registered to
+     * @param ErrorRendererInterface|string|callable $errorRenderer The error renderer
      */
-    public function registerErrorRenderer(string $contentType, string $errorRenderer): void
+    public function registerErrorRenderer(string $contentType, $errorRenderer): void
     {
         $this->errorRenderers[$contentType] = $errorRenderer;
     }
 
     /**
      * Set the default error renderer
      *
-     * @param string $errorRenderer The default error renderer class name
+     * @param ErrorRendererInterface|string|callable $errorRenderer The default error renderer
      */
-    public function setDefaultErrorRenderer(string $errorRenderer): void
+    public function setDefaultErrorRenderer($errorRenderer): void
     {
         $this->defaultErrorRenderer = $errorRenderer;
     }
@@ -292,10 +300,10 @@ protected function respond(): ResponseInterface
             $response = $response->withHeader('Allow', $allowedMethods);
         }
 
-        /** @var ErrorRendererInterface $renderer */
-        $renderer = $this->renderer;
-        $body = $renderer->render($this->exception, $this->displayErrorDetails);
-        $response->getBody()->write($body);
+        if (is_callable($this->renderer)) {
+            $body = call_user_func($this->renderer, $this->exception, $this->displayErrorDetails);
+            $response->getBody()->write($body);
+        }
 
         return $response;
     }

Slim/Middleware/ErrorMiddleware.php

@@ -136,7 +136,7 @@ public function getDefaultErrorHandler()
             return $this->callableResolver->resolve($this->defaultErrorHandler);
         }
 
-        return new ErrorHandler($this->responseFactory);
+        return new ErrorHandler($this->callableResolver, $this->responseFactory);
     }
 
     /**

tests/CallableResolverTest.php

@@ -13,7 +13,9 @@
 use Prophecy\Prophecy\ObjectProphecy;
 use Psr\Container\ContainerInterface;
 use Slim\CallableResolver;
+use Slim\Interfaces\ErrorRendererInterface;
 use Slim\Tests\Mocks\CallableTest;
+use Slim\Tests\Mocks\ErrorRendererTest;
 use Slim\Tests\Mocks\InvokableTest;
 use Slim\Tests\Mocks\RequestHandlerTest;
 
@@ -158,6 +160,16 @@ public function testResolutionToAPsrRequestHandlerClassWithCustomMethod()
         $this->assertEquals('custom', $callable[1]);
     }
 
+    public function testObjErrorRendererClass()
+    {
+        $obj = new ErrorRendererTest();
+        $resolver = $this->getCallableResolver();
+        $callable = $resolver->resolve($obj);
+
+        $this->assertIsCallable($callable);
+        $this->assertInstanceOf(ErrorRendererInterface::class, $callable[0]);
+    }
+
     /**
      * @expectedException \RuntimeException
      */

tests/Handlers/ErrorHandlerTest.php

@@ -32,6 +32,10 @@ public function testDetermineContentTypeMethodDoesNotThrowExceptionWhenPassedVal
             ->getMock();
         $class = new ReflectionClass(ErrorHandler::class);
 
+        $callableResolverProperty = $class->getProperty('callableResolver');
+        $callableResolverProperty->setAccessible(true);
+        $callableResolverProperty->setValue($handler, $this->getCallableResolver());
+
         $reflectionProperty = $class->getProperty('renderer');
         $reflectionProperty->setAccessible(true);
         $reflectionProperty->setValue($handler, MockErrorRenderer::class);
@@ -71,6 +75,10 @@ public function testDetermineRenderer()
             ->getMock();
         $class = new ReflectionClass(ErrorHandler::class);
 
+        $callableResolverProperty = $class->getProperty('callableResolver');
+        $callableResolverProperty->setAccessible(true);
+        $callableResolverProperty->setValue($handler, $this->getCallableResolver());
+
         $reflectionProperty = $class->getProperty('contentType');
         $reflectionProperty->setAccessible(true);
         $reflectionProperty->setValue($handler, 'application/json');
@@ -79,20 +87,24 @@ public function testDetermineRenderer()
         $method->setAccessible(true);
 
         $renderer = $method->invoke($handler);
-        $this->assertInstanceOf(JsonErrorRenderer::class, $renderer);
+        $this->assertIsCallable($renderer);
+        $this->assertInstanceOf(JsonErrorRenderer::class, $renderer[0]);
 
         $reflectionProperty->setValue($handler, 'application/xml');
         $renderer = $method->invoke($handler);
-        $this->assertInstanceOf(XmlErrorRenderer::class, $renderer);
+        $this->assertIsCallable($renderer);
+        $this->assertInstanceOf(XmlErrorRenderer::class, $renderer[0]);
 
         $reflectionProperty->setValue($handler, 'text/plain');
         $renderer = $method->invoke($handler);
-        $this->assertInstanceOf(PlainTextErrorRenderer::class, $renderer);
+        $this->assertIsCallable($renderer);
+        $this->assertInstanceOf(PlainTextErrorRenderer::class, $renderer[0]);
 
         // Test the default error renderer
         $reflectionProperty->setValue($handler, 'text/unknown');
         $renderer = $method->invoke($handler);
-        $this->assertInstanceOf(HtmlErrorRenderer::class, $renderer);
+        $this->assertIsCallable($renderer);
+        $this->assertInstanceOf(HtmlErrorRenderer::class, $renderer[0]);
     }
 
     public function testDetermineStatusCode()
@@ -260,7 +272,7 @@ public function testAcceptableMediaTypeIsNotFirstInList()
 
     public function testRegisterErrorRenderer()
     {
-        $handler = new ErrorHandler($this->getResponseFactory());
+        $handler = new ErrorHandler($this->getCallableResolver(), $this->getResponseFactory());
         $handler->registerErrorRenderer('application/slim', PlainTextErrorRenderer::class);
 
         $reflectionClass = new ReflectionClass(ErrorHandler::class);
@@ -273,7 +285,7 @@ public function testRegisterErrorRenderer()
 
     public function testSetDefaultErrorRenderer()
     {
-        $handler = new ErrorHandler($this->getResponseFactory());
+        $handler = new ErrorHandler($this->getCallableResolver(), $this->getResponseFactory());
         $handler->setDefaultErrorRenderer(PlainTextErrorRenderer::class);
 
         $reflectionClass = new ReflectionClass(ErrorHandler::class);
@@ -287,7 +299,7 @@ public function testSetDefaultErrorRenderer()
     public function testOptions()
     {
         $request = $this->createServerRequest('/', 'OPTIONS');
-        $handler = new ErrorHandler($this->getResponseFactory());
+        $handler = new ErrorHandler($this->getCallableResolver(), $this->getResponseFactory());
         $exception = new HttpMethodNotAllowedException($request);
         $exception->setAllowedMethods(['POST', 'PUT']);
 
@@ -306,7 +318,10 @@ public function testWriteToErrorLog()
             ->withHeader('Accept', 'application/json');
 
         $handler = $this->getMockBuilder(ErrorHandler::class)
-            ->setConstructorArgs(['responseFactory' => $this->getResponseFactory()])
+            ->setConstructorArgs([
+                'callableResolver' => $this->getCallableResolver(),
+                'responseFactory' => $this->getResponseFactory(),
+            ])
             ->setMethods(['writeToErrorLog', 'logError'])
             ->getMock();
 

tests/Mocks/ErrorRendererTest.php

@@ -0,0 +1,24 @@
+<?php
+/**
+ * Slim Framework (https://slimframework.com)
+ *
+ * @license https://github.com/slimphp/Slim/blob/4.x/LICENSE.md (MIT License)
+ */
+
+declare(strict_types=1);
+
+namespace Slim\Tests\Mocks;
+
+use Slim\Interfaces\ErrorRendererInterface;
+use Throwable;
+
+class ErrorRendererTest implements ErrorRendererInterface
+{
+    /**
+     * {@inheritdoc}
+     */
+    public function render(Throwable $exception, bool $displayErrorDetails): string
+    {
+        return $exception->getMessage().($displayErrorDetails ? ' +Details' : '');
+    }
+}

tests/TestCase.php

@@ -10,12 +10,15 @@
 namespace Slim\Tests;
 
 use PHPUnit\Framework\TestCase as PHPUnitTestCase;
+use Psr\Container\ContainerInterface;
 use Psr\Http\Message\ResponseFactoryInterface;
 use Psr\Http\Message\ResponseInterface;
 use Psr\Http\Message\ServerRequestFactoryInterface;
 use Psr\Http\Message\ServerRequestInterface;
 use Psr\Http\Message\StreamFactoryInterface;
 use Psr\Http\Message\StreamInterface;
+use Slim\CallableResolver;
+use Slim\Interfaces\CallableResolverInterface;
 use Slim\Tests\Providers\PSR7ObjectProvider;
 
 abstract class TestCase extends PhpUnitTestCase
@@ -47,6 +50,16 @@ protected function getStreamFactory(): StreamFactoryInterface
         return $psr7ObjectProvider->getStreamFactory();
     }
 
+    /**
+     * @param ContainerInterface|null $container
+     *
+     * @return CallableResolverInterface
+     */
+    protected function getCallableResolver(?ContainerInterface $container = null): CallableResolverInterface
+    {
+        return new CallableResolver($container);
+    }
+
     /**
      * @param string $uri
      * @param string $method