From 6983f9b2bf25a26435a03e47ecbecaa1c022fa76 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sun, 20 Feb 2022 10:36:54 +0100 Subject: [PATCH] Added middleware that mitigates big error traces being logged for those not using mercure --- module/Rest/config/dependencies.config.php | 7 ++++ module/Rest/config/routes.config.php | 4 ++- module/Rest/src/Action/MercureInfoAction.php | 8 +---- .../Rest/src/Exception/MercureException.php | 5 ++- .../NotConfiguredMercureErrorHandler.php | 34 +++++++++++++++++++ 5 files changed, 47 insertions(+), 11 deletions(-) create mode 100644 module/Rest/src/Middleware/Mercure/NotConfiguredMercureErrorHandler.php diff --git a/module/Rest/config/dependencies.config.php b/module/Rest/config/dependencies.config.php index 5f0d5c05..a7a4b2ca 100644 --- a/module/Rest/config/dependencies.config.php +++ b/module/Rest/config/dependencies.config.php @@ -6,7 +6,9 @@ namespace Shlinkio\Shlink\Rest; use Laminas\ServiceManager\AbstractFactory\ConfigAbstractFactory; use Laminas\ServiceManager\Factory\InvokableFactory; +use Mezzio\ProblemDetails\ProblemDetailsResponseFactory; use Mezzio\Router\Middleware\ImplicitOptionsMiddleware; +use Psr\Log\LoggerInterface; use Shlinkio\Shlink\Common\Mercure\LcobucciJwtProvider; use Shlinkio\Shlink\Core\Domain\DomainService; use Shlinkio\Shlink\Core\Options; @@ -49,6 +51,7 @@ return [ Middleware\ShortUrl\DropDefaultDomainFromRequestMiddleware::class => ConfigAbstractFactory::class, Middleware\ShortUrl\DefaultShortCodesLengthMiddleware::class => ConfigAbstractFactory::class, Middleware\ShortUrl\OverrideDomainMiddleware::class => ConfigAbstractFactory::class, + Middleware\Mercure\NotConfiguredMercureErrorHandler::class => ConfigAbstractFactory::class, ], ], @@ -90,6 +93,10 @@ return [ 'config.url_shortener.default_short_codes_length', ], Middleware\ShortUrl\OverrideDomainMiddleware::class => [DomainService::class], + Middleware\Mercure\NotConfiguredMercureErrorHandler::class => [ + ProblemDetailsResponseFactory::class, + LoggerInterface::class, + ], ], ]; diff --git a/module/Rest/config/routes.config.php b/module/Rest/config/routes.config.php index 16f83149..c9b42579 100644 --- a/module/Rest/config/routes.config.php +++ b/module/Rest/config/routes.config.php @@ -4,6 +4,8 @@ declare(strict_types=1); namespace Shlinkio\Shlink\Rest; +use Shlinkio\Shlink\Rest\Middleware\Mercure\NotConfiguredMercureErrorHandler; + $contentNegotiationMiddleware = Middleware\ShortUrl\CreateShortUrlContentNegotiationMiddleware::class; $dropDomainMiddleware = Middleware\ShortUrl\DropDefaultDomainFromRequestMiddleware::class; $overrideDomainMiddleware = Middleware\ShortUrl\OverrideDomainMiddleware::class; @@ -46,7 +48,7 @@ return [ Action\Domain\ListDomainsAction::getRouteDef(), Action\Domain\DomainRedirectsAction::getRouteDef(), - Action\MercureInfoAction::getRouteDef(), + Action\MercureInfoAction::getRouteDef([NotConfiguredMercureErrorHandler::class]), ], ]; diff --git a/module/Rest/src/Action/MercureInfoAction.php b/module/Rest/src/Action/MercureInfoAction.php index d6710357..1454cbbc 100644 --- a/module/Rest/src/Action/MercureInfoAction.php +++ b/module/Rest/src/Action/MercureInfoAction.php @@ -10,7 +10,6 @@ use Psr\Http\Message\ResponseInterface; use Psr\Http\Message\ServerRequestInterface; use Shlinkio\Shlink\Common\Mercure\JwtProviderInterface; use Shlinkio\Shlink\Rest\Exception\MercureException; -use Throwable; use function sprintf; @@ -32,12 +31,7 @@ class MercureInfoAction extends AbstractRestAction $days = $this->mercureConfig['jwt_days_duration'] ?? 1; $expiresAt = Chronos::now()->addDays($days); - - try { - $jwt = $this->jwtProvider->buildSubscriptionToken($expiresAt); - } catch (Throwable $e) { - throw MercureException::mercureNotConfigured($e); - } + $jwt = $this->jwtProvider->buildSubscriptionToken($expiresAt); return new JsonResponse([ 'mercureHubUrl' => sprintf('%s/.well-known/mercure', $hubUrl), diff --git a/module/Rest/src/Exception/MercureException.php b/module/Rest/src/Exception/MercureException.php index 6c318e93..9435cb54 100644 --- a/module/Rest/src/Exception/MercureException.php +++ b/module/Rest/src/Exception/MercureException.php @@ -7,7 +7,6 @@ namespace Shlinkio\Shlink\Rest\Exception; use Fig\Http\Message\StatusCodeInterface; use Mezzio\ProblemDetails\Exception\CommonProblemDetailsExceptionTrait; use Mezzio\ProblemDetails\Exception\ProblemDetailsExceptionInterface; -use Throwable; class MercureException extends RuntimeException implements ProblemDetailsExceptionInterface { @@ -16,9 +15,9 @@ class MercureException extends RuntimeException implements ProblemDetailsExcepti private const TITLE = 'Mercure integration not configured'; private const TYPE = 'MERCURE_NOT_CONFIGURED'; - public static function mercureNotConfigured(?Throwable $prev = null): self + public static function mercureNotConfigured(): self { - $e = new self('This Shlink instance is not integrated with a mercure hub.', 1, $prev); + $e = new self('This Shlink instance is not integrated with a mercure hub.'); $e->detail = $e->getMessage(); $e->title = self::TITLE; diff --git a/module/Rest/src/Middleware/Mercure/NotConfiguredMercureErrorHandler.php b/module/Rest/src/Middleware/Mercure/NotConfiguredMercureErrorHandler.php new file mode 100644 index 00000000..32a714b6 --- /dev/null +++ b/module/Rest/src/Middleware/Mercure/NotConfiguredMercureErrorHandler.php @@ -0,0 +1,34 @@ +handle($request); + } catch (MercureException $e) { + // Throwing this kind of exception makes a big error trace to be logged, for anyone who has decided to not + // use mercure. + // It happens every time the shlink-web-client is opened, so this mitigates the problem by just logging a + // simple warning, and casting the exception to a response on the fly. + $this->logger->warning($e->getMessage()); + return $this->respFactory->createResponseFromThrowable($request, $e); + } + } +}