Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion Slim/Error/Renderers/HtmlErrorRenderer.php
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ class HtmlErrorRenderer extends AbstractErrorRenderer
* @param bool $displayErrorDetails
* @return string
*/
public function render(Throwable $exception, bool $displayErrorDetails): string
public function __invoke(Throwable $exception, bool $displayErrorDetails): string
{
$title = 'Slim Application Error';

Expand Down
2 changes: 1 addition & 1 deletion Slim/Error/Renderers/JsonErrorRenderer.php
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ class JsonErrorRenderer extends AbstractErrorRenderer
* @param bool $displayErrorDetails
* @return string
*/
public function render(Throwable $exception, bool $displayErrorDetails): string
public function __invoke(Throwable $exception, bool $displayErrorDetails): string
{
$error = ['message' => $exception->getMessage()];

Expand Down
2 changes: 1 addition & 1 deletion Slim/Error/Renderers/PlainTextErrorRenderer.php
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ class PlainTextErrorRenderer extends AbstractErrorRenderer
* @param bool $displayErrorDetails
* @return string
*/
public function render(Throwable $exception, bool $displayErrorDetails): string
public function __invoke(Throwable $exception, bool $displayErrorDetails): string
{
$text = "Slim Application Error:\n";
$text .= $this->formatExceptionFragment($exception);
Expand Down
2 changes: 1 addition & 1 deletion Slim/Error/Renderers/XmlErrorRenderer.php
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ class XmlErrorRenderer extends AbstractErrorRenderer
* @param bool $displayErrorDetails
* @return string
*/
public function render(Throwable $exception, bool $displayErrorDetails): string
public function __invoke(Throwable $exception, bool $displayErrorDetails): string
{
$xml = "<error>\n <message>{$exception->getMessage()}</message>\n";

Expand Down
68 changes: 26 additions & 42 deletions Slim/Handlers/ErrorHandler.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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,
Expand Down Expand Up @@ -83,25 +84,27 @@ class ErrorHandler implements ErrorHandlerInterface
protected $exception;

/**
* @var ErrorRendererInterface|string|null
* @var int
*/
protected $renderer;
protected $statusCode;

/**
* @var int
* @var CallableResolverInterface
*/
protected $statusCode;
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;
}

Expand Down Expand Up @@ -131,7 +134,6 @@ public function __invoke(
$this->method = $request->getMethod();
$this->statusCode = $this->determineStatusCode();
$this->contentType = $this->determineContentType($request);
$this->renderer = $this->determineRenderer();

if ($logErrors) {
$this->writeToErrorLog();
Expand Down Expand Up @@ -201,56 +203,39 @@ 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;

if ($renderer !== null
&& (
(is_string($renderer) && !class_exists($renderer))
|| !in_array(ErrorRendererInterface::class, class_implements($renderer), true)
)
) {
throw new RuntimeException(
'Non compliant error renderer provided (%s). ' .
'Renderer must implement the ErrorRendererInterface'
);
}

if ($renderer === null) {
if (array_key_exists($this->contentType, $this->errorRenderers)) {
$renderer = $this->errorRenderers[$this->contentType];
} else {
$renderer = $this->defaultErrorRenderer;
}
if (array_key_exists($this->contentType, $this->errorRenderers)) {
$renderer = $this->errorRenderers[$this->contentType];
} else {
$renderer = $this->defaultErrorRenderer;
}

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;
}
Expand All @@ -263,7 +248,7 @@ public function setDefaultErrorRenderer(string $errorRenderer): void
protected function writeToErrorLog(): void
{
$renderer = new PlainTextErrorRenderer();
$error = $renderer->render($this->exception, $this->logErrorDetails);
$error = $renderer->__invoke($this->exception, $this->logErrorDetails);
$error .= "\nView in rendered output by enabling the \"displayErrorDetails\" setting.\n";
$this->logError($error);
}
Expand Down Expand Up @@ -292,9 +277,8 @@ protected function respond(): ResponseInterface
$response = $response->withHeader('Allow', $allowedMethods);
}

/** @var ErrorRendererInterface $renderer */
$renderer = $this->renderer;
$body = $renderer->render($this->exception, $this->displayErrorDetails);
$renderer = $this->determineRenderer();
$body = call_user_func($renderer, $this->exception, $this->displayErrorDetails);
$response->getBody()->write($body);

return $response;
Expand Down
2 changes: 1 addition & 1 deletion Slim/Interfaces/ErrorRendererInterface.php
Original file line number Diff line number Diff line change
Expand Up @@ -18,5 +18,5 @@ interface ErrorRendererInterface
* @param bool $displayErrorDetails
* @return string
*/
public function render(Throwable $exception, bool $displayErrorDetails): string;
public function __invoke(Throwable $exception, bool $displayErrorDetails): string;
}
2 changes: 1 addition & 1 deletion Slim/Middleware/ErrorMiddleware.php
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}

/**
Expand Down
14 changes: 7 additions & 7 deletions tests/Error/AbstractErrorRendererTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ public function testHTMLErrorRendererDisplaysErrorDetails()
{
$exception = new RuntimeException('Oops..');
$renderer = new HtmlErrorRenderer();
$output = $renderer->render($exception, true);
$output = $renderer->__invoke($exception, true);

$this->assertRegExp('/.*The application could not run because of the following error:.*/', $output);
}
Expand All @@ -34,7 +34,7 @@ public function testHTMLErrorRendererNoErrorDetails()
{
$exception = new RuntimeException('Oops..');
$renderer = new HtmlErrorRenderer();
$output = $renderer->render($exception, false);
$output = $renderer->__invoke($exception, false);

$this->assertRegExp('/.*A website error has occurred. Sorry for the temporary inconvenience.*/', $output);
}
Expand Down Expand Up @@ -66,7 +66,7 @@ public function testJSONErrorRendererDisplaysErrorDetails()
$method->setAccessible(true);

$fragment = $method->invoke($renderer, $exception);
$output = json_encode(json_decode($renderer->render($exception, true)));
$output = json_encode(json_decode($renderer->__invoke($exception, true)));
$expectedString = json_encode(['message' => 'Oops..', 'exception' => [$fragment]]);

$this->assertEquals($output, $expectedString);
Expand All @@ -77,7 +77,7 @@ public function testJSONErrorRendererDoesNotDisplayErrorDetails()
$exception = new Exception('Oops..');

$renderer = new JsonErrorRenderer();
$output = json_encode(json_decode($renderer->render($exception, false)));
$output = json_encode(json_decode($renderer->__invoke($exception, false)));

$this->assertEquals($output, json_encode(['message' => 'Oops..']));
}
Expand All @@ -92,7 +92,7 @@ public function testJSONErrorRendererDisplaysPreviousError()
$method = $reflectionRenderer->getMethod('formatExceptionFragment');
$method->setAccessible(true);

$output = json_encode(json_decode($renderer->render($exception, true)));
$output = json_encode(json_decode($renderer->__invoke($exception, true)));

$fragments = [
$method->invoke($renderer, $exception),
Expand All @@ -112,7 +112,7 @@ public function testXMLErrorRendererDisplaysErrorDetails()
$renderer = new XmlErrorRenderer();

/** @var stdClass $output */
$output = simplexml_load_string($renderer->render($exception, true));
$output = simplexml_load_string($renderer->__invoke($exception, true));

$this->assertEquals($output->message[0], 'Ooops...');
$this->assertEquals((string) $output->exception[0]->type, 'Exception');
Expand Down Expand Up @@ -142,7 +142,7 @@ public function testPlainTextErrorRendererDisplaysErrorDetails()
$exception = new Exception('Ooops...', 0, $previousException);

$renderer = new PlainTextErrorRenderer();
$output = $renderer->render($exception, true);
$output = $renderer->__invoke($exception, true);

$this->assertRegExp('/Ooops.../', $output);
}
Expand Down
67 changes: 19 additions & 48 deletions tests/Handlers/ErrorHandlerTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -19,50 +19,10 @@
use Slim\Exception\HttpNotFoundException;
use Slim\Handlers\ErrorHandler;
use Slim\Tests\Mocks\MockCustomException;
use Slim\Tests\Mocks\MockErrorRenderer;
use Slim\Tests\TestCase;

class ErrorHandlerTest extends TestCase
{
public function testDetermineContentTypeMethodDoesNotThrowExceptionWhenPassedValidRenderer()
{
$handler = $this
->getMockBuilder(ErrorHandler::class)
->disableOriginalConstructor()
->getMock();
$class = new ReflectionClass(ErrorHandler::class);

$reflectionProperty = $class->getProperty('renderer');
$reflectionProperty->setAccessible(true);
$reflectionProperty->setValue($handler, MockErrorRenderer::class);

$method = $class->getMethod('determineRenderer');
$method->setAccessible(true);
$method->invoke($handler);

$this->addToAssertionCount(1);
}

/**
* @expectedException \RuntimeException
*/
public function testDetermineContentTypeMethodThrowsExceptionWhenPassedAnInvalidRenderer()
{
$handler = $this
->getMockBuilder(ErrorHandler::class)
->disableOriginalConstructor()
->getMock();
$class = new ReflectionClass(ErrorHandler::class);

$reflectionProperty = $class->getProperty('renderer');
$reflectionProperty->setAccessible(true);
$reflectionProperty->setValue($handler, 'NonExistentRenderer::class');

$method = $class->getMethod('determineRenderer');
$method->setAccessible(true);
$method->invoke($handler);
}

public function testDetermineRenderer()
{
$handler = $this
Expand All @@ -71,6 +31,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');
Expand All @@ -79,20 +43,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()
Expand Down Expand Up @@ -260,7 +228,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);
Expand All @@ -273,7 +241,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);
Expand All @@ -287,7 +255,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']);

Expand All @@ -306,7 +274,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();

Expand Down
Loading