From bdcf93d47465ed5d1ec48d466873cf7b8b85297a Mon Sep 17 00:00:00 2001 From: Rob Allen Date: Sun, 6 Sep 2015 11:26:30 +0100 Subject: [PATCH] Update error handlers to be content-type aware Ensure that Error, NotAllowed and NotFound return JSON, XML or HTML as determined by the request's Accept header. This makes Slim much more API friendly. --- Slim/Handlers/Error.php | 144 +++++++++++++++++++++++++++--- Slim/Handlers/NotAllowed.php | 74 +++++++++++++-- Slim/Handlers/NotFound.php | 48 ++++++++-- tests/AppTest.php | 2 +- tests/Handlers/ErrorTest.php | 54 +++++++++++ tests/Handlers/NotAllowedTest.php | 23 ++++- tests/Handlers/NotFoundTest.php | 57 ++++++++++++ 7 files changed, 371 insertions(+), 31 deletions(-) create mode 100644 tests/Handlers/ErrorTest.php create mode 100644 tests/Handlers/NotFoundTest.php diff --git a/Slim/Handlers/Error.php b/Slim/Handlers/Error.php index 10d5c7956..9796857d5 100644 --- a/Slim/Handlers/Error.php +++ b/Slim/Handlers/Error.php @@ -14,10 +14,10 @@ use Slim\Http\Body; /** - * Default error handler + * Default Slim application error handler * - * This is the default Slim application error handler. All it does is output - * a clean and simple HTML page with diagnostic information. + * It outputs the error message and diagnostic information in either JSON, XML, + * or HTML based on the Accept header. */ class Error { @@ -31,15 +31,49 @@ class Error * @return ResponseInterface */ public function __invoke(ServerRequestInterface $request, ResponseInterface $response, Exception $exception) + { + $contentType = $this->determineContentType($request->getHeaderLine('Accept')); + switch ($contentType) { + case 'application/json': + $output = $this->renderJsonErrorMessage($exception); + break; + + case 'application/xml': + $output = $this->renderXmlErrorMessage($exception); + break; + + case 'text/html': + default: + $contentType = 'text/html'; + $output = $this->renderHtmlErrorMessage($exception); + break; + } + + $body = new Body(fopen('php://temp', 'r+')); + $body->write($output); + + return $response + ->withStatus(500) + ->withHeader('Content-type', $contentType) + ->withBody($body); + } + + /** + * Render HTML error page + * + * @param Exception $exception + * @return string + */ + private function renderHtmlErrorMessage(Exception $exception) { $title = 'Slim Application Error'; $html = '

The application could not run because of the following error:

'; $html .= '

Details

'; - $html .= $this->renderException($exception); + $html .= $this->renderHtmlException($exception); while ($exception = $exception->getPrevious()) { $html .= '

Previous exception

'; - $html .= $this->renderException($exception); + $html .= $this->renderHtmlException($exception); } $output = sprintf( @@ -52,23 +86,17 @@ public function __invoke(ServerRequestInterface $request, ResponseInterface $res $html ); - $body = new Body(fopen('php://temp', 'r+')); - $body->write($output); - - return $response - ->withStatus(500) - ->withHeader('Content-type', 'text/html') - ->withBody($body); + return $output; } /** - * Render exception as html. + * Render exception as HTML. * * @param Exception $exception * * @return string */ - private function renderException(Exception $exception) + private function renderHtmlException(Exception $exception) { $code = $exception->getCode(); $message = $exception->getMessage(); @@ -95,4 +123,92 @@ private function renderException(Exception $exception) } return $html; } + + /** + * Render JSON error + * + * @param Exception $exception + * @return string + */ + private function renderJsonErrorMessage(Exception $exception) + { + $error = ['message' => 'Slim Application Error']; + $error['exception'][] = [ + 'code' => $exception->getCode(), + 'message' => $exception->getMessage(), + 'file' => $exception->getFile(), + 'line' => $exception->getLine(), + 'trace' => explode("\n", $exception->getTraceAsString()), + ]; + + while ($exception = $exception->getPrevious()) { + $error['exception'][] = [ + 'code' => $exception->getCode(), + 'message' => $exception->getMessage(), + 'file' => $exception->getFile(), + 'line' => $exception->getLine(), + 'trace' => explode("\n", $exception->getTraceAsString()), + ]; + } + + return json_encode($error); + } + + /** + * Render XML error + * + * @param Exception $exception + * @return string + */ + private function renderXmlErrorMessage(Exception $exception) + { + $xml = "\n Slim Application Error\n"; + + $xml .= << + {$exception->getCode()} + {$exception->getMessage()} + {$exception->getFile()} + {$exception->getLine()} + {$exception->getTraceAsString()} + + +EOT; + + while ($exception = $exception->getPrevious()) { + $xml .= << + {$exception->getCode()} + {$exception->getMessage()} + {$exception->getFile()} + {$exception->getLine()} + {$exception->getTraceAsString()} + + +EOT; + } + $xml .=""; + return $xml; + } + + /** + * Read the accept header and determine which content type we know about + * is wanted. + * + * @param string $acceptHeader Accept header from request + * @return string + */ + private function determineContentType($acceptHeader) + { + $list = explode(',', $acceptHeader); + $known = ['application/json', 'application/xml', 'text/html']; + + foreach ($list as $type) { + if (in_array($type, $known)) { + return $type; + } + } + + return 'text/html'; + } } diff --git a/Slim/Handlers/NotAllowed.php b/Slim/Handlers/NotAllowed.php index 319e6710e..c92eb1046 100644 --- a/Slim/Handlers/NotAllowed.php +++ b/Slim/Handlers/NotAllowed.php @@ -13,10 +13,10 @@ use Slim\Http\Body; /** - * Default not allowed handler + * Default Slim application not allowed handler * - * This is the default Slim application error handler. All it does is output - * a clean and simple HTML page with diagnostic information. + * It outputs a simple message in either JSON, XML or HTML based on the + * Accept header. */ class NotAllowed { @@ -32,22 +32,82 @@ class NotAllowed public function __invoke(ServerRequestInterface $request, ResponseInterface $response, array $methods) { $allow = implode(', ', $methods); - $body = new Body(fopen('php://temp', 'r+')); if ($request->getMethod() === 'OPTIONS') { $status = 200; $contentType = 'text/plain'; - $body->write('Allowed methods: ' . $allow); + $output = 'Allowed methods: ' . $allow; } else { $status = 405; - $contentType = 'text/html'; - $body->write('

Method not allowed. Must be one of: ' . $allow . '

'); + $contentType = $this->determineContentType($request->getHeaderLine('Accept')); + switch ($contentType) { + case 'application/json': + $output = '{"message":"Method not allowed. Must be one of: ' . $allow . '"}'; + break; + + case 'application/xml': + $output = "Method not allowed. Must be one of: $allow"; + break; + + case 'text/html': + default: + $contentType = 'text/html'; + $output = << + + Method not allowed + + + +

Method not allowed

+

Method not allowed. Must be one of: $allow

+ + +END; + break; + } } + $body = new Body(fopen('php://temp', 'r+')); + $body->write($output); + return $response ->withStatus($status) ->withHeader('Content-type', $contentType) ->withHeader('Allow', $allow) ->withBody($body); } + + /** + * Read the accept header and determine which content type we know about + * is wanted. + * + * @param string $acceptHeader Accept header from request + * @return string + */ + private function determineContentType($acceptHeader) + { + $list = explode(',', $acceptHeader); + $known = ['application/json', 'application/xml', 'text/html']; + + foreach ($list as $type) { + if (in_array($type, $known)) { + return $type; + } + } + + return 'text/html'; + } } diff --git a/Slim/Handlers/NotFound.php b/Slim/Handlers/NotFound.php index 9b3c8905d..fb612eb97 100644 --- a/Slim/Handlers/NotFound.php +++ b/Slim/Handlers/NotFound.php @@ -13,10 +13,10 @@ use Slim\Http\Body; /** - * Default not found handler + * Default Slim application not found handler. * - * This is the default Slim application not found handler. All it does is output - * a clean and simple HTML page with diagnostic information. + * It outputs a simple message in either JSON, XML or HTML based on the + * Accept header. */ class NotFound { @@ -30,9 +30,22 @@ class NotFound */ public function __invoke(ServerRequestInterface $request, ResponseInterface $response) { - $homeUrl = (string)($request->getUri()->withPath('')->withQuery('')->withFragment('')); - $output = <<determineContentType($request->getHeaderLine('Accept')); + switch ($contentType) { + case 'application/json': + $output = '{"message":"Not found"}'; + break; + + case 'application/xml': + $output = 'Not found'; + break; + + case 'text/html': + default: + $homeUrl = (string)($request->getUri()->withPath('')->withQuery('')->withFragment('')); + $contentType = 'text/html'; + $output = << Page Not Found @@ -65,12 +78,35 @@ public function __invoke(ServerRequestInterface $request, ResponseInterface $res END; + break; + } $body = new Body(fopen('php://temp', 'r+')); $body->write($output); return $response->withStatus(404) - ->withHeader('Content-Type', 'text/html') + ->withHeader('Content-Type', $contentType) ->withBody($body); } + + /** + * Read the accept header and determine which content type we know about + * is wanted. + * + * @param string $acceptHeader Accept header from request + * @return string + */ + private function determineContentType($acceptHeader) + { + $list = explode(',', $acceptHeader); + $known = ['application/json', 'application/xml', 'text/html']; + + foreach ($list as $type) { + if (in_array($type, $known)) { + return $type; + } + } + + return 'text/html'; + } } diff --git a/tests/AppTest.php b/tests/AppTest.php index 24b9f467a..b3f68103a 100644 --- a/tests/AppTest.php +++ b/tests/AppTest.php @@ -279,7 +279,7 @@ public function testInvokeReturnMethodNotAllowed() $this->assertInstanceOf('\Psr\Http\Message\ResponseInterface', $resOut); $this->assertEquals(405, (string)$resOut->getStatusCode()); $this->assertEquals(['GET'], $resOut->getHeader('Allow')); - $this->assertEquals('

Method not allowed. Must be one of: GET

', (string)$resOut->getBody()); + $this->assertContains('

Method not allowed. Must be one of: GET

', (string)$resOut->getBody()); } public function testInvokeWithMatchingRoute() diff --git a/tests/Handlers/ErrorTest.php b/tests/Handlers/ErrorTest.php new file mode 100644 index 000000000..4a66c9fdd --- /dev/null +++ b/tests/Handlers/ErrorTest.php @@ -0,0 +1,54 @@ +'], + ['text/html', ''], + ]; + } + + /** + * Test invalid method returns the correct code and content type + * + * @dataProvider errorProvider + */ + public function testError($contentType, $startOfBody) + { + $notAllowed = new Error(); + $e = new \Exception("Oops"); + + /** @var Response $res */ + $res = $notAllowed->__invoke($this->getRequest('GET', $contentType), new Response(), $e); + + $this->assertSame(500, $res->getStatusCode()); + $this->assertSame($contentType, $res->getHeaderLine('Content-Type')); + $this->assertEquals(0, strpos((string)$res->getBody(), $startOfBody)); + } + + /** + * @param string $method + * @return \PHPUnit_Framework_MockObject_MockObject|\Slim\Http\Request + */ + protected function getRequest($method, $contentType = 'text/html') + { + $req = $this->getMockBuilder('Slim\Http\Request')->disableOriginalConstructor()->getMock(); + $req->expects($this->once())->method('getHeaderLine')->will($this->returnValue($contentType)); + + return $req; + } +} diff --git a/tests/Handlers/NotAllowedTest.php b/tests/Handlers/NotAllowedTest.php index ba1c553ad..c59fe8829 100644 --- a/tests/Handlers/NotAllowedTest.php +++ b/tests/Handlers/NotAllowedTest.php @@ -13,16 +13,32 @@ class NotAllowedTest extends \PHPUnit_Framework_TestCase { - public function testInvalidMethod() + public function invalidMethodProvider() + { + return [ + ['application/json', '{'], + ['application/xml', ''], + ['text/html', ''], + ]; + } + + /** + * Test invalid method returns the correct code and content type + * + * @dataProvider invalidMethodProvider + */ + public function testInvalidMethod($contentType, $startOfBody) { $notAllowed = new NotAllowed(); /** @var Response $res */ - $res = $notAllowed->__invoke($this->getRequest('GET'), new Response(), ['POST', 'PUT']); + $res = $notAllowed->__invoke($this->getRequest('GET', $contentType), new Response(), ['POST', 'PUT']); $this->assertSame(405, $res->getStatusCode()); $this->assertTrue($res->hasHeader('Allow')); + $this->assertSame($contentType, $res->getHeaderLine('Content-Type')); $this->assertEquals('POST, PUT', $res->getHeaderLine('Allow')); + $this->assertEquals(0, strpos((string)$res->getBody(), $startOfBody)); } public function testOptions() @@ -41,10 +57,11 @@ public function testOptions() * @param string $method * @return \PHPUnit_Framework_MockObject_MockObject|\Slim\Http\Request */ - protected function getRequest($method) + protected function getRequest($method, $contentType = 'text/html') { $req = $this->getMockBuilder('Slim\Http\Request')->disableOriginalConstructor()->getMock(); $req->expects($this->once())->method('getMethod')->will($this->returnValue($method)); + $req->expects($this->any())->method('getHeaderLine')->will($this->returnValue($contentType)); return $req; } diff --git a/tests/Handlers/NotFoundTest.php b/tests/Handlers/NotFoundTest.php new file mode 100644 index 000000000..851717a30 --- /dev/null +++ b/tests/Handlers/NotFoundTest.php @@ -0,0 +1,57 @@ +'], + ['text/html', ''], + ]; + } + + /** + * Test invalid method returns the correct code and content type + * + * @dataProvider notFoundProvider + */ + public function testNotFound($contentType, $startOfBody) + { + $notAllowed = new NotFound(); + + /** @var Response $res */ + $res = $notAllowed->__invoke($this->getRequest('GET', $contentType), new Response(), ['POST', 'PUT']); + + $this->assertSame(404, $res->getStatusCode()); + $this->assertSame($contentType, $res->getHeaderLine('Content-Type')); + $this->assertEquals(0, strpos((string)$res->getBody(), $startOfBody)); + } + + /** + * @param string $method + * @return \PHPUnit_Framework_MockObject_MockObject|\Slim\Http\Request + */ + protected function getRequest($method, $contentType = 'text/html') + { + $uri = new Uri('http', 'example.com', 80, '/notfound'); + + $req = $this->getMockBuilder('Slim\Http\Request')->disableOriginalConstructor()->getMock(); + $req->expects($this->once())->method('getHeaderLine')->will($this->returnValue($contentType)); + $req->expects($this->any())->method('getUri')->will($this->returnValue($uri)); + + return $req; + } +}