From e24b208d460b1a408dbf26fab8f5a3d41bd9908c Mon Sep 17 00:00:00 2001 From: Franz Liedke Date: Tue, 14 Aug 2018 23:15:50 +0200 Subject: [PATCH] Split up HandleErrors middleware into distinct classes These are completely distinct functionalities, toggled through the system-wide debug flag. By moving the selection of the middleware to use to the place where the middleware pipe is built, we make the middleware itself be unaware of these flags. The two classes are more focused on what they are doing, with the constructor dependencies clearly representing their requirements. In addition, this means we can just use the HandleErrorsWithWhoops middleware in the installer, which means we do not need to worry about how to inject a SettingsRepositoryInterface implementation when flarum is not yet set up. --- .../core/src/Admin/AdminServiceProvider.php | 10 ++++-- .../core/src/Forum/ForumServiceProvider.php | 10 ++++-- ...dleErrors.php => HandleErrorsWithView.php} | 18 ++-------- .../Middleware/HandleErrorsWithWhoops.php | 34 +++++++++++++++++++ 4 files changed, 51 insertions(+), 21 deletions(-) rename framework/core/src/Http/Middleware/{HandleErrors.php => HandleErrorsWithView.php} (88%) create mode 100644 framework/core/src/Http/Middleware/HandleErrorsWithWhoops.php diff --git a/framework/core/src/Admin/AdminServiceProvider.php b/framework/core/src/Admin/AdminServiceProvider.php index 3849d183c..6ec339bd2 100644 --- a/framework/core/src/Admin/AdminServiceProvider.php +++ b/framework/core/src/Admin/AdminServiceProvider.php @@ -17,7 +17,8 @@ use Flarum\Foundation\AbstractServiceProvider; use Flarum\Frontend\RecompileFrontendAssets; use Flarum\Http\Middleware\AuthenticateWithSession; use Flarum\Http\Middleware\DispatchRoute; -use Flarum\Http\Middleware\HandleErrors; +use Flarum\Http\Middleware\HandleErrorsWithView; +use Flarum\Http\Middleware\HandleErrorsWithWhoops; use Flarum\Http\Middleware\ParseJsonBody; use Flarum\Http\Middleware\RememberFromCookie; use Flarum\Http\Middleware\SetLocale; @@ -46,8 +47,11 @@ class AdminServiceProvider extends AbstractServiceProvider $pipe = new MiddlewarePipe; // All requests should first be piped through our global error handler - $debugMode = ! $app->isUpToDate() || $app->inDebugMode(); - $pipe->pipe($app->make(HandleErrors::class, ['debug' => $debugMode])); + if ($app->inDebugMode()) { + $pipe->pipe($app->make(HandleErrorsWithWhoops::class)); + } else { + $pipe->pipe($app->make(HandleErrorsWithView::class)); + } $pipe->pipe($app->make(ParseJsonBody::class)); $pipe->pipe($app->make(StartSession::class)); diff --git a/framework/core/src/Forum/ForumServiceProvider.php b/framework/core/src/Forum/ForumServiceProvider.php index 625ff636b..62655ef2a 100644 --- a/framework/core/src/Forum/ForumServiceProvider.php +++ b/framework/core/src/Forum/ForumServiceProvider.php @@ -17,7 +17,8 @@ use Flarum\Foundation\AbstractServiceProvider; use Flarum\Http\Middleware\AuthenticateWithSession; use Flarum\Http\Middleware\CollectGarbage; use Flarum\Http\Middleware\DispatchRoute; -use Flarum\Http\Middleware\HandleErrors; +use Flarum\Http\Middleware\HandleErrorsWithView; +use Flarum\Http\Middleware\HandleErrorsWithWhoops; use Flarum\Http\Middleware\ParseJsonBody; use Flarum\Http\Middleware\RememberFromCookie; use Flarum\Http\Middleware\SetLocale; @@ -49,8 +50,11 @@ class ForumServiceProvider extends AbstractServiceProvider $pipe = new MiddlewarePipe; // All requests should first be piped through our global error handler - $debugMode = ! $app->isUpToDate() || $app->inDebugMode(); - $pipe->pipe($app->make(HandleErrors::class, ['debug' => $debugMode])); + if ($app->inDebugMode()) { + $pipe->pipe($app->make(HandleErrorsWithWhoops::class)); + } else { + $pipe->pipe($app->make(HandleErrorsWithView::class)); + } $pipe->pipe($app->make(ParseJsonBody::class)); $pipe->pipe($app->make(CollectGarbage::class)); diff --git a/framework/core/src/Http/Middleware/HandleErrors.php b/framework/core/src/Http/Middleware/HandleErrorsWithView.php similarity index 88% rename from framework/core/src/Http/Middleware/HandleErrors.php rename to framework/core/src/Http/Middleware/HandleErrorsWithView.php index 8df217bf3..0ef7f5139 100644 --- a/framework/core/src/Http/Middleware/HandleErrors.php +++ b/framework/core/src/Http/Middleware/HandleErrorsWithView.php @@ -13,7 +13,6 @@ namespace Flarum\Http\Middleware; use Exception; use Flarum\Settings\SettingsRepositoryInterface; -use Franzl\Middleware\Whoops\WhoopsRunner; use Illuminate\Contracts\View\Factory as ViewFactory; use Psr\Http\Message\ResponseInterface as Response; use Psr\Http\Message\ServerRequestInterface as Request; @@ -23,7 +22,7 @@ use Psr\Log\LoggerInterface; use Symfony\Component\Translation\TranslatorInterface; use Zend\Diactoros\Response\HtmlResponse; -class HandleErrors implements Middleware +class HandleErrorsWithView implements Middleware { /** * @var ViewFactory @@ -45,25 +44,18 @@ class HandleErrors implements Middleware */ protected $settings; - /** - * @var bool - */ - protected $debug; - /** * @param ViewFactory $view * @param LoggerInterface $logger * @param TranslatorInterface $translator * @param SettingsRepositoryInterface $settings - * @param bool $debug */ - public function __construct(ViewFactory $view, LoggerInterface $logger, TranslatorInterface $translator, SettingsRepositoryInterface $settings, $debug = false) + public function __construct(ViewFactory $view, LoggerInterface $logger, TranslatorInterface $translator, SettingsRepositoryInterface $settings) { $this->view = $view; $this->logger = $logger; $this->translator = $translator; $this->settings = $settings; - $this->debug = $debug; } /** @@ -74,11 +66,7 @@ class HandleErrors implements Middleware try { return $handler->handle($request); } catch (Exception $e) { - if ($this->debug) { - return WhoopsRunner::handle($e, $request); - } else { - return $this->formatException($e); - } + return $this->formatException($e); } } diff --git a/framework/core/src/Http/Middleware/HandleErrorsWithWhoops.php b/framework/core/src/Http/Middleware/HandleErrorsWithWhoops.php new file mode 100644 index 000000000..57d56b2ad --- /dev/null +++ b/framework/core/src/Http/Middleware/HandleErrorsWithWhoops.php @@ -0,0 +1,34 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +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; + +class HandleErrorsWithWhoops implements Middleware +{ + /** + * Catch all errors that happen during further middleware execution. + */ + public function process(Request $request, Handler $handler): Response + { + try { + return $handler->handle($request); + } catch (Exception $e) { + return WhoopsRunner::handle($e, $request); + } + } +}