From 2a0f8ff7ed5ae655a7a048176593ee1e70c61503 Mon Sep 17 00:00:00 2001 From: Sami Mazouz Date: Sun, 13 Aug 2023 17:43:14 +0100 Subject: [PATCH] chore: use `ExceptionHandler` contract --- framework/core/composer.json | 1 + .../core/src/Admin/AdminServiceProvider.php | 9 -- framework/core/src/Api/ApiServiceProvider.php | 9 -- .../core/src/Forum/ForumServiceProvider.php | 9 -- framework/core/src/Foundation/Application.php | 1 + .../Bootstrap/RegisterCoreProviders.php | 1 - .../ErrorHandling/ExceptionHandler.php | 93 +++++++++++++++++++ .../ErrorHandling/JsonApiFormatter.php | 5 +- .../src/Foundation/ErrorServiceProvider.php | 16 ++++ .../core/src/Http/Middleware/HandleErrors.php | 57 ------------ framework/core/src/Http/UrlGenerator.php | 4 +- framework/core/src/Install/Installer.php | 16 +--- framework/core/src/Queue/ExceptionHandler.php | 68 -------------- .../core/src/Queue/QueueServiceProvider.php | 4 - 14 files changed, 119 insertions(+), 174 deletions(-) create mode 100644 framework/core/src/Foundation/ErrorHandling/ExceptionHandler.php delete mode 100644 framework/core/src/Http/Middleware/HandleErrors.php delete mode 100644 framework/core/src/Queue/ExceptionHandler.php diff --git a/framework/core/composer.json b/framework/core/composer.json index c5ac8fac0..912a432b4 100644 --- a/framework/core/composer.json +++ b/framework/core/composer.json @@ -88,6 +88,7 @@ "symfony/mime": "^6.3", "symfony/polyfill-intl-messageformatter": "^1.27", "symfony/postmark-mailer": "^6.3", + "symfony/psr-http-message-bridge": "^2.3", "symfony/translation": "^6.3", "symfony/translation-contracts": "^2.5", "symfony/yaml": "^6.3", diff --git a/framework/core/src/Admin/AdminServiceProvider.php b/framework/core/src/Admin/AdminServiceProvider.php index 496e819c7..4e859a34e 100644 --- a/framework/core/src/Admin/AdminServiceProvider.php +++ b/framework/core/src/Admin/AdminServiceProvider.php @@ -37,7 +37,6 @@ class AdminServiceProvider extends AbstractServiceProvider $this->container->singleton('flarum.admin.middleware', function () { return [ HttpMiddleware\InjectActorReference::class, - 'flarum.admin.error_handler', HttpMiddleware\StartSession::class, HttpMiddleware\RememberFromCookie::class, HttpMiddleware\AuthenticateWithSession::class, @@ -50,14 +49,6 @@ class AdminServiceProvider extends AbstractServiceProvider ]; }); - $this->container->bind('flarum.admin.error_handler', function (Container $container) { - return new HttpMiddleware\HandleErrors( - $container->make(Registry::class), - $container['flarum.config']->inDebugMode() ? $container->make(WhoopsFormatter::class) : $container->make(ViewFormatter::class), - $container->tagged(Reporter::class) - ); - }); - $this->container->bind('flarum.assets.admin', function (Container $container) { /** @var \Flarum\Frontend\Assets $assets */ $assets = $container->make('flarum.assets.factory')('admin'); diff --git a/framework/core/src/Api/ApiServiceProvider.php b/framework/core/src/Api/ApiServiceProvider.php index 89d2ea597..6e8a4a3d8 100644 --- a/framework/core/src/Api/ApiServiceProvider.php +++ b/framework/core/src/Api/ApiServiceProvider.php @@ -45,7 +45,6 @@ class ApiServiceProvider extends AbstractServiceProvider $this->container->singleton('flarum.api.middleware', function () { return [ HttpMiddleware\InjectActorReference::class, - 'flarum.api.error_handler', Middleware\FakeHttpMethods::class, HttpMiddleware\StartSession::class, HttpMiddleware\RememberFromCookie::class, @@ -57,14 +56,6 @@ class ApiServiceProvider extends AbstractServiceProvider ]; }); - $this->container->bind('flarum.api.error_handler', function (Container $container) { - return new HttpMiddleware\HandleErrors( - $container->make(Registry::class), - new JsonApiFormatter($container['flarum.config']->inDebugMode()), - $container->tagged(Reporter::class) - ); - }); - $this->container->singleton('flarum.api.notification_serializers', function () { return [ 'discussionRenamed' => BasicDiscussionSerializer::class diff --git a/framework/core/src/Forum/ForumServiceProvider.php b/framework/core/src/Forum/ForumServiceProvider.php index 3785dcac0..f53d7a819 100644 --- a/framework/core/src/Forum/ForumServiceProvider.php +++ b/framework/core/src/Forum/ForumServiceProvider.php @@ -44,7 +44,6 @@ class ForumServiceProvider extends AbstractServiceProvider $this->container->singleton('flarum.forum.middleware', function () { return [ HttpMiddleware\InjectActorReference::class, - 'flarum.forum.error_handler', HttpMiddleware\CollectGarbage::class, HttpMiddleware\StartSession::class, HttpMiddleware\RememberFromCookie::class, @@ -58,14 +57,6 @@ class ForumServiceProvider extends AbstractServiceProvider ]; }); - $this->container->bind('flarum.forum.error_handler', function (Container $container) { - return new HttpMiddleware\HandleErrors( - $container->make(Registry::class), - $container['flarum.config']->inDebugMode() ? $container->make(WhoopsFormatter::class) : $container->make(ViewFormatter::class), - $container->tagged(Reporter::class) - ); - }); - $this->container->bind('flarum.assets.forum', function (Container $container) { /** @var Assets $assets */ $assets = $container->make('flarum.assets.factory')('forum'); diff --git a/framework/core/src/Foundation/Application.php b/framework/core/src/Foundation/Application.php index 682aa42b5..1ff6e0d96 100644 --- a/framework/core/src/Foundation/Application.php +++ b/framework/core/src/Foundation/Application.php @@ -83,6 +83,7 @@ class Application extends IlluminateContainer implements LaravelApplication protected function registerBaseServiceProviders(): void { $this->register(new EventServiceProvider($this)); + $this->register(new ErrorServiceProvider($this)); $this->register(new RoutingServiceProvider($this)); // Because we need to check very early if the version of the app diff --git a/framework/core/src/Foundation/Bootstrap/RegisterCoreProviders.php b/framework/core/src/Foundation/Bootstrap/RegisterCoreProviders.php index 68310eed2..d536551f7 100644 --- a/framework/core/src/Foundation/Bootstrap/RegisterCoreProviders.php +++ b/framework/core/src/Foundation/Bootstrap/RegisterCoreProviders.php @@ -42,7 +42,6 @@ class RegisterCoreProviders implements IlluminateBootstrapperInterface $app->register(ConsoleServiceProvider::class); $app->register(DiscussionServiceProvider::class); $app->register(ExtensionServiceProvider::class); - $app->register(ErrorServiceProvider::class); $app->register(FilesystemServiceProvider::class); $app->register(FilterServiceProvider::class); $app->register(FormatterServiceProvider::class); diff --git a/framework/core/src/Foundation/ErrorHandling/ExceptionHandler.php b/framework/core/src/Foundation/ErrorHandling/ExceptionHandler.php new file mode 100644 index 000000000..062861ad2 --- /dev/null +++ b/framework/core/src/Foundation/ErrorHandling/ExceptionHandler.php @@ -0,0 +1,93 @@ +getHandledError($e); + + if ($error->shouldBeReported()) { + foreach ($this->reporters as $reporter) { + $reporter->report($e); + } + } + } + + public function render($request, Throwable $e): Response /** @phpstan-ignore-line */ + { + return $this->resolveFormatter($request)->format( + $this->getHandledError($e), $request + ); + } + + public function renderForConsole($output, Throwable $e): void + { + (new ConsoleApplication())->renderThrowable($e, $output); + } + + public function shouldReport(Throwable $e): bool + { + return $this->getHandledError($e)->shouldBeReported(); + } + + /** + * Get and cache the handled error for the given exception. + */ + protected function getHandledError(Throwable $e): HandledError + { + return $this->handledErrors[$this->exceptionKey($e)] ??= $this->registry->handle($e); + } + + /** + * Get a unique key for the given exception. + */ + protected function exceptionKey(Throwable $e): string + { + return get_class($e).':'.$e->getMessage().$e->getLine(); + } + + protected function resolveFormatter(Request $request): HttpFormatter + { + return match (true) { + $request->expectsJson(), + $request->routeIs('api.*') => Arr::first($this->formatters, fn (HttpFormatter $formatter) => $formatter instanceof JsonApiFormatter), + $this->config->inDebugMode() => Arr::first($this->formatters, fn (HttpFormatter $formatter) => $formatter instanceof WhoopsFormatter), + default => Arr::first($this->formatters, fn (HttpFormatter $formatter) => $formatter instanceof ViewFormatter), + }; + } +} diff --git a/framework/core/src/Foundation/ErrorHandling/JsonApiFormatter.php b/framework/core/src/Foundation/ErrorHandling/JsonApiFormatter.php index 411baacb0..2b3409647 100644 --- a/framework/core/src/Foundation/ErrorHandling/JsonApiFormatter.php +++ b/framework/core/src/Foundation/ErrorHandling/JsonApiFormatter.php @@ -10,6 +10,7 @@ namespace Flarum\Foundation\ErrorHandling; use Flarum\Api\JsonApiResponse; +use Flarum\Foundation\Config; use Illuminate\Http\Request; use Symfony\Component\HttpFoundation\Response; use Tobscure\JsonApi\Document; @@ -22,7 +23,7 @@ use Tobscure\JsonApi\Document; class JsonApiFormatter implements HttpFormatter { public function __construct( - private readonly bool $includeTrace = false + private readonly Config $config ) { } @@ -46,7 +47,7 @@ class JsonApiFormatter implements HttpFormatter 'code' => $error->getType(), ]; - if ($this->includeTrace) { + if ($this->config->inDebugMode()) { $default['detail'] = (string) $error->getException(); } diff --git a/framework/core/src/Foundation/ErrorServiceProvider.php b/framework/core/src/Foundation/ErrorServiceProvider.php index 8a27d1941..cacb29ead 100644 --- a/framework/core/src/Foundation/ErrorServiceProvider.php +++ b/framework/core/src/Foundation/ErrorServiceProvider.php @@ -11,6 +11,9 @@ namespace Flarum\Foundation; use Flarum\Extension\Exception as ExtensionException; use Flarum\Foundation\ErrorHandling as Handling; +use Flarum\Foundation\ErrorHandling\ExceptionHandler; +use Illuminate\Contracts\Container\Container; +use Illuminate\Contracts\Debug\ExceptionHandler as ExceptionHandlerContract; use Illuminate\Database\Eloquent\ModelNotFoundException; use Illuminate\Validation\ValidationException as IlluminateValidationException; use Tobscure\JsonApi\Exception\InvalidParameterException; @@ -19,6 +22,13 @@ class ErrorServiceProvider extends AbstractServiceProvider { public function register(): void { + $this->container->singleton(ExceptionHandlerContract::class, function (Container $container) { + return $container->make(ExceptionHandler::class, [ + 'formatters' => $container->tagged(Handling\HttpFormatter::class), + 'reporters' => $container->tagged(Handling\Reporter::class), + ]); + }); + $this->container->singleton('flarum.error.statuses', function () { return [ // 400 Bad Request @@ -73,5 +83,11 @@ class ErrorServiceProvider extends AbstractServiceProvider }); $this->container->tag(Handling\LogReporter::class, Handling\Reporter::class); + + $this->container->tag([ + Handling\JsonApiFormatter::class, + Handling\ViewFormatter::class, + Handling\WhoopsFormatter::class, + ], Handling\HttpFormatter::class); } } diff --git a/framework/core/src/Http/Middleware/HandleErrors.php b/framework/core/src/Http/Middleware/HandleErrors.php deleted file mode 100644 index 719589ef4..000000000 --- a/framework/core/src/Http/Middleware/HandleErrors.php +++ /dev/null @@ -1,57 +0,0 @@ -registry->handle($e); - - if ($error->shouldBeReported()) { - foreach ($this->reporters as $reporter) { - $reporter->report($error->getException()); - } - } - -// dump($e); - - return $this->formatter->format($error, $request); - } - } -} diff --git a/framework/core/src/Http/UrlGenerator.php b/framework/core/src/Http/UrlGenerator.php index 99e201f69..a8f980521 100644 --- a/framework/core/src/Http/UrlGenerator.php +++ b/framework/core/src/Http/UrlGenerator.php @@ -16,13 +16,13 @@ class UrlGenerator extends IlluminateUrlGenerator public function base(string $frontend): string { - $url = $this->config->url(); + $url = rtrim($this->config->url(), '/'); if ($frontend) { $url .= '/'.$this->config->path($frontend); } - return $url; + return rtrim($url, '/'); } public function path(string $frontend, string $path): string diff --git a/framework/core/src/Install/Installer.php b/framework/core/src/Install/Installer.php index dbe692abd..82658069e 100644 --- a/framework/core/src/Install/Installer.php +++ b/framework/core/src/Install/Installer.php @@ -10,23 +10,18 @@ namespace Flarum\Install; use Flarum\Foundation\AppInterface; -use Flarum\Foundation\ErrorHandling\Registry; -use Flarum\Foundation\ErrorHandling\Reporter; -use Flarum\Foundation\ErrorHandling\WhoopsFormatter; use Flarum\Http\Middleware as HttpMiddleware; use Flarum\Install\Console\InstallCommand; -use Illuminate\Contracts\Container\Container; -use Laminas\Stratigility\MiddlewarePipe; -use Psr\Http\Server\RequestHandlerInterface; +use Illuminate\Contracts\Foundation\Application; class Installer implements AppInterface { public function __construct( - protected Container $container + protected Application $container ) { } - public function getContainer(): Container + public function getContainer(): Application { return $this->container; } @@ -34,11 +29,6 @@ class Installer implements AppInterface public function getMiddlewareStack(): array { return [ - new HttpMiddleware\HandleErrors( - $this->container->make(Registry::class), - $this->container->make(WhoopsFormatter::class), - $this->container->tagged(Reporter::class) - ), $this->container->make(HttpMiddleware\StartSession::class), ]; } diff --git a/framework/core/src/Queue/ExceptionHandler.php b/framework/core/src/Queue/ExceptionHandler.php deleted file mode 100644 index f4b4e61cc..000000000 --- a/framework/core/src/Queue/ExceptionHandler.php +++ /dev/null @@ -1,68 +0,0 @@ -logger->error((string) $e); - } - - /** - * Render an exception into an HTTP response. - * - * @param \Illuminate\Http\Request $request - * @param Throwable $e - * @return void - */ - public function render($request, Throwable $e) /** @phpstan-ignore-line */ - { -// dd($e); - } - - /** - * Render an exception to the console. - * - * @param \Symfony\Component\Console\Output\OutputInterface $output - * @param Throwable $e - * @return void - */ - public function renderForConsole($output, Throwable $e) - { - // TODO: Implement renderForConsole() method. - } - - /** - * Determine if the exception should be reported. - * - * @param Throwable $e - * @return bool - */ - public function shouldReport(Throwable $e) - { - return true; - } -} diff --git a/framework/core/src/Queue/QueueServiceProvider.php b/framework/core/src/Queue/QueueServiceProvider.php index bf7d6fa68..3c0becd4e 100644 --- a/framework/core/src/Queue/QueueServiceProvider.php +++ b/framework/core/src/Queue/QueueServiceProvider.php @@ -61,10 +61,6 @@ class QueueServiceProvider extends AbstractServiceProvider return $queue; }); - $this->container->singleton(ExceptionHandling::class, function (Container $container) { - return new ExceptionHandler($container['log']); - }); - $this->container->singleton(Worker::class, function (Container $container) { /** @var Config $config */ $config = $container->make(Config::class);