From 2df5be7bcba7b287adc924d58ccba891f4f932e0 Mon Sep 17 00:00:00 2001 From: Toby Zerner Date: Sun, 11 Nov 2018 17:54:19 +1030 Subject: [PATCH 1/4] Catch Throwables so that we handle internal PHP errors too --- framework/core/src/Api/ErrorHandler.php | 7 ++++++- framework/core/src/Api/Middleware/HandleErrors.php | 4 ++-- .../core/src/Http/Middleware/HandleErrorsWithView.php | 6 +++--- .../core/src/Http/Middleware/HandleErrorsWithWhoops.php | 4 ++-- 4 files changed, 13 insertions(+), 8 deletions(-) diff --git a/framework/core/src/Api/ErrorHandler.php b/framework/core/src/Api/ErrorHandler.php index 3f6125adc..21ea56e3b 100644 --- a/framework/core/src/Api/ErrorHandler.php +++ b/framework/core/src/Api/ErrorHandler.php @@ -12,6 +12,7 @@ namespace Flarum\Api; use Exception; +use Throwable; use Tobscure\JsonApi\Document; use Tobscure\JsonApi\ErrorHandler as JsonApiErrorHandler; @@ -34,8 +35,12 @@ class ErrorHandler * @param Exception $e * @return JsonApiResponse */ - public function handle(Exception $e) + public function handle(Throwable $e) { + if (! $e instanceof Exception) { + $e = new Exception($e->getMessage(), $e->getCode(), $e); + } + $response = $this->errorHandler->handle($e); $document = new Document; diff --git a/framework/core/src/Api/Middleware/HandleErrors.php b/framework/core/src/Api/Middleware/HandleErrors.php index bba861ac2..cfcc54068 100644 --- a/framework/core/src/Api/Middleware/HandleErrors.php +++ b/framework/core/src/Api/Middleware/HandleErrors.php @@ -11,12 +11,12 @@ namespace Flarum\Api\Middleware; -use Exception; use Flarum\Api\ErrorHandler; use Psr\Http\Message\ResponseInterface as Response; use Psr\Http\Message\ServerRequestInterface as Request; use Psr\Http\Server\MiddlewareInterface as Middleware; use Psr\Http\Server\RequestHandlerInterface as Handler; +use Throwable; class HandleErrors implements Middleware { @@ -40,7 +40,7 @@ class HandleErrors implements Middleware { try { return $handler->handle($request); - } catch (Exception $e) { + } catch (Throwable $e) { return $this->errorHandler->handle($e); } } diff --git a/framework/core/src/Http/Middleware/HandleErrorsWithView.php b/framework/core/src/Http/Middleware/HandleErrorsWithView.php index 0ef7f5139..27e2118ac 100644 --- a/framework/core/src/Http/Middleware/HandleErrorsWithView.php +++ b/framework/core/src/Http/Middleware/HandleErrorsWithView.php @@ -11,7 +11,6 @@ namespace Flarum\Http\Middleware; -use Exception; use Flarum\Settings\SettingsRepositoryInterface; use Illuminate\Contracts\View\Factory as ViewFactory; use Psr\Http\Message\ResponseInterface as Response; @@ -20,6 +19,7 @@ use Psr\Http\Server\MiddlewareInterface as Middleware; use Psr\Http\Server\RequestHandlerInterface as Handler; use Psr\Log\LoggerInterface; use Symfony\Component\Translation\TranslatorInterface; +use Throwable; use Zend\Diactoros\Response\HtmlResponse; class HandleErrorsWithView implements Middleware @@ -65,12 +65,12 @@ class HandleErrorsWithView implements Middleware { try { return $handler->handle($request); - } catch (Exception $e) { + } catch (Throwable $e) { return $this->formatException($e); } } - protected function formatException(Exception $error) + protected function formatException(Throwable $error) { $status = 500; $errorCode = $error->getCode(); diff --git a/framework/core/src/Http/Middleware/HandleErrorsWithWhoops.php b/framework/core/src/Http/Middleware/HandleErrorsWithWhoops.php index 57d56b2ad..44e3f2881 100644 --- a/framework/core/src/Http/Middleware/HandleErrorsWithWhoops.php +++ b/framework/core/src/Http/Middleware/HandleErrorsWithWhoops.php @@ -11,12 +11,12 @@ namespace Flarum\Http\Middleware; -use Exception; use Franzl\Middleware\Whoops\WhoopsRunner; use Psr\Http\Message\ResponseInterface as Response; use Psr\Http\Message\ServerRequestInterface as Request; use Psr\Http\Server\MiddlewareInterface as Middleware; use Psr\Http\Server\RequestHandlerInterface as Handler; +use Throwable; class HandleErrorsWithWhoops implements Middleware { @@ -27,7 +27,7 @@ class HandleErrorsWithWhoops implements Middleware { try { return $handler->handle($request); - } catch (Exception $e) { + } catch (Throwable $e) { return WhoopsRunner::handle($e, $request); } } From 4302687876dccf6656e00e563c337cba304da1ac Mon Sep 17 00:00:00 2001 From: Toby Zerner Date: Sun, 11 Nov 2018 17:57:55 +1030 Subject: [PATCH 2/4] Stop logging errors that use a custom view Having a custom view implies that a friendly message is displayed to the user, in which case we can bet that the exception won't need to be "debugged" per se. --- framework/core/src/Http/Middleware/HandleErrorsWithView.php | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/framework/core/src/Http/Middleware/HandleErrorsWithView.php b/framework/core/src/Http/Middleware/HandleErrorsWithView.php index 27e2118ac..960074f39 100644 --- a/framework/core/src/Http/Middleware/HandleErrorsWithView.php +++ b/framework/core/src/Http/Middleware/HandleErrorsWithView.php @@ -81,11 +81,10 @@ class HandleErrorsWithView implements Middleware $status = $errorCode; } - // Log the exception (with trace) - $this->logger->debug($error); - if (! $this->view->exists($name = "flarum.forum::error.$status")) { $name = 'flarum.forum::error.default'; + + $this->logger->error($error); } $view = $this->view->make($name) From 95944e363063cb620e11e5f727fcca51954da1a9 Mon Sep 17 00:00:00 2001 From: Toby Zerner Date: Sun, 11 Nov 2018 18:00:57 +1030 Subject: [PATCH 3/4] Log errors that occur in the API stack This takes place only in the FallbackExceptionHandler. Having a custom exception handler implies that a friendly message is displayed in the API response, in which case we can bet that the exception won't need to be "debugged" per se. --- framework/core/src/Api/ApiServiceProvider.php | 3 +- .../FallbackExceptionHandler.php | 79 +++++++++++++++++++ 2 files changed, 80 insertions(+), 2 deletions(-) create mode 100644 framework/core/src/Api/ExceptionHandler/FallbackExceptionHandler.php diff --git a/framework/core/src/Api/ApiServiceProvider.php b/framework/core/src/Api/ApiServiceProvider.php index 50fbfd12d..d49398eae 100644 --- a/framework/core/src/Api/ApiServiceProvider.php +++ b/framework/core/src/Api/ApiServiceProvider.php @@ -25,7 +25,6 @@ use Flarum\Http\RouteCollection; use Flarum\Http\RouteHandlerFactory; use Flarum\Http\UrlGenerator; use Tobscure\JsonApi\ErrorHandler; -use Tobscure\JsonApi\Exception\Handler\FallbackExceptionHandler; use Tobscure\JsonApi\Exception\Handler\InvalidParameterExceptionHandler; use Zend\Stratigility\MiddlewarePipe; @@ -80,7 +79,7 @@ class ApiServiceProvider extends AbstractServiceProvider $handler->registerHandler(new ExceptionHandler\TokenMismatchExceptionHandler); $handler->registerHandler(new ExceptionHandler\ValidationExceptionHandler); $handler->registerHandler(new InvalidParameterExceptionHandler); - $handler->registerHandler(new FallbackExceptionHandler($this->app->inDebugMode())); + $handler->registerHandler(new ExceptionHandler\FallbackExceptionHandler($this->app->inDebugMode(), $this->app->make('log'))); return $handler; }); diff --git a/framework/core/src/Api/ExceptionHandler/FallbackExceptionHandler.php b/framework/core/src/Api/ExceptionHandler/FallbackExceptionHandler.php new file mode 100644 index 000000000..f14ce579d --- /dev/null +++ b/framework/core/src/Api/ExceptionHandler/FallbackExceptionHandler.php @@ -0,0 +1,79 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace Flarum\Api\ExceptionHandler; + +use Exception; +use Psr\Log\LoggerInterface; +use Tobscure\JsonApi\Exception\Handler\ExceptionHandlerInterface; +use Tobscure\JsonApi\Exception\Handler\ResponseBag; + +class FallbackExceptionHandler implements ExceptionHandlerInterface +{ + /** + * @var bool + */ + protected $debug; + + /** + * @var LoggerInterface + */ + protected $logger; + + /** + * @param bool $debug + * @param LoggerInterface $logger + */ + public function __construct($debug, LoggerInterface $logger) + { + $this->debug = $debug; + $this->logger = $logger; + } + + /** + * {@inheritdoc} + */ + public function manages(Exception $e) + { + return true; + } + + /** + * {@inheritdoc} + */ + public function handle(Exception $e) + { + $status = 500; + $error = $this->constructError($e, $status); + + if (! $this->debug) { + $this->logger->error($e); + } + + return new ResponseBag($status, [$error]); + } + + /** + * @param Exception $e + * @param $status + * @return array + */ + private function constructError(Exception $e, $status) + { + $error = ['code' => $status, 'title' => 'Internal server error']; + + if ($this->debug) { + $error['detail'] = (string) $e; + } + + return $error; + } +} From 3ab7da66c837474f6d4cbf9cb28eb9c28d962787 Mon Sep 17 00:00:00 2001 From: Toby Zerner Date: Tue, 13 Nov 2018 07:47:01 +1030 Subject: [PATCH 4/4] Log errors when debug mode is on too --- .../FallbackExceptionHandler.php | 4 +--- .../Http/Middleware/HandleErrorsWithWhoops.php | 18 ++++++++++++++++++ 2 files changed, 19 insertions(+), 3 deletions(-) diff --git a/framework/core/src/Api/ExceptionHandler/FallbackExceptionHandler.php b/framework/core/src/Api/ExceptionHandler/FallbackExceptionHandler.php index f14ce579d..1087dbe89 100644 --- a/framework/core/src/Api/ExceptionHandler/FallbackExceptionHandler.php +++ b/framework/core/src/Api/ExceptionHandler/FallbackExceptionHandler.php @@ -54,9 +54,7 @@ class FallbackExceptionHandler implements ExceptionHandlerInterface $status = 500; $error = $this->constructError($e, $status); - if (! $this->debug) { - $this->logger->error($e); - } + $this->logger->error($e); return new ResponseBag($status, [$error]); } diff --git a/framework/core/src/Http/Middleware/HandleErrorsWithWhoops.php b/framework/core/src/Http/Middleware/HandleErrorsWithWhoops.php index 44e3f2881..83df56af9 100644 --- a/framework/core/src/Http/Middleware/HandleErrorsWithWhoops.php +++ b/framework/core/src/Http/Middleware/HandleErrorsWithWhoops.php @@ -16,10 +16,24 @@ use Psr\Http\Message\ResponseInterface as Response; use Psr\Http\Message\ServerRequestInterface as Request; use Psr\Http\Server\MiddlewareInterface as Middleware; use Psr\Http\Server\RequestHandlerInterface as Handler; +use Psr\Log\LoggerInterface; use Throwable; class HandleErrorsWithWhoops implements Middleware { + /** + * @var LoggerInterface + */ + protected $logger; + + /** + * @param LoggerInterface $logger + */ + public function __construct(LoggerInterface $logger) + { + $this->logger = $logger; + } + /** * Catch all errors that happen during further middleware execution. */ @@ -28,6 +42,10 @@ class HandleErrorsWithWhoops implements Middleware try { return $handler->handle($request); } catch (Throwable $e) { + if ($e->getCode() !== 404) { + $this->logger->error($e); + } + return WhoopsRunner::handle($e, $request); } }