[!!!][TASK] Refactor exception handling

Classes/Dispatcher/Bootstrap.php

@@ -7,11 +7,16 @@
 use Psr\EventDispatcher\EventDispatcherInterface;
 use Psr\Http\Message\ResponseInterface;
 use Psr\Http\Message\ServerRequestInterface;
+use Psr\Log\LoggerAwareInterface;
+use Psr\Log\LoggerAwareTrait;
 use SourceBroker\T3api\Domain\Repository\ApiResourceRepository;
+use SourceBroker\T3api\Exception\BadRequestException;
 use SourceBroker\T3api\Exception\ExceptionInterface;
+use SourceBroker\T3api\Exception\InternalServerErrorException;
 use SourceBroker\T3api\Serializer\ContextBuilder\DeserializationContextBuilder;
 use SourceBroker\T3api\Serializer\ContextBuilder\SerializationContextBuilder;
 use SourceBroker\T3api\Service\OperationResponseCache;
+use SourceBroker\T3api\Service\RequestLanguageService;
 use SourceBroker\T3api\Service\RouteService;
 use SourceBroker\T3api\Service\SerializerService;
 use Symfony\Bridge\PsrHttpMessage\Factory\HttpFoundationFactory;
@@ -24,8 +29,10 @@
 use TYPO3\CMS\Core\Http\Response;
 use TYPO3\CMS\Core\Utility\GeneralUtility;
 
-class Bootstrap extends AbstractDispatcher
+class Bootstrap extends AbstractDispatcher implements LoggerAwareInterface
 {
+    use LoggerAwareTrait;
+
     protected ResponseInterface $response;
 
     protected HttpFoundationFactory $httpFoundationFactory;
@@ -59,6 +66,10 @@ public function process(ServerRequestInterface $inputRequest): ResponseInterface
             $request = $this->httpFoundationFactory->createRequest($inputRequest);
             $context = (new RequestContext())->fromRequest($request);
             $this->callProcessors($request, $this->response);
+            $languageHeaderException = $inputRequest->getAttribute(RequestLanguageService::ATTRIBUTE_LANGUAGE_HEADER_EXCEPTION);
+            if ($languageHeaderException instanceof BadRequestException) {
+                throw $languageHeaderException;
+            }
             if ($this->isMainEndpointResponseClassDefined() && $this->isContextMatchingMainEndpointRoute($context)) {
                 $output = $this->processMainEndpoint();
             } else {
@@ -68,8 +79,16 @@ public function process(ServerRequestInterface $inputRequest): ResponseInterface
             $output = $this->serializerService->serialize($exception);
             $this->response = $this->response->withStatus($exception->getStatusCode(), $exception->getTitle());
         } catch (\Throwable $throwable) {
+            $eventId = bin2hex(random_bytes(8));
+            $this->logger?->error(
+                't3api: unexpected exception, event ID {eventId}',
+                ['eventId' => $eventId, 'exception' => $throwable]
+            );
             try {
-                $output = $this->serializerService->serialize($throwable);
+                // The message of an unexpected exception may contain paths, database details etc.
+                $output = $this->serializerService->serialize(
+                    SerializerService::isDebugMode() ? $throwable : new InternalServerErrorException($eventId)
+                );
                 $this->response = $this->response->withStatus(
                     SymfonyResponse::HTTP_INTERNAL_SERVER_ERROR,
                     SymfonyResponse::$statusTexts[SymfonyResponse::HTTP_INTERNAL_SERVER_ERROR]

Classes/Exception/BadRequestException.php

@@ -0,0 +1,37 @@
+<?php
+
+declare(strict_types=1);
+
+namespace SourceBroker\T3api\Exception;
+
+use SourceBroker\T3api\OpenApi\Objects\Response as OpenApiResponse;
+use Symfony\Component\HttpFoundation\Response;
+use Symfony\Component\HttpFoundation\Response as SymfonyResponse;
+
+/**
+ * Invalid data sent by the client (request body, query parameters, headers). Unlike unexpected exceptions,
+ * the description is returned to the client in every application context.
+ */
+class BadRequestException extends AbstractException implements OpenApiSupportingExceptionInterface
+{
+    public static function getOpenApiResponse(): OpenApiResponse
+    {
+        return parent::getOpenApiResponse()
+            ->statusCode(SymfonyResponse::HTTP_BAD_REQUEST)
+            ->description(self::translate('exception.bad_request.title'));
+    }
+
+    /**
+     * @param string $descriptionKey Key of the description in `locallang.xlf`, without the `exception.bad_request.` prefix
+     */
+    public function __construct(string $descriptionKey, array $arguments, int $code)
+    {
+        $this->title = self::translate('exception.bad_request.title');
+        parent::__construct(self::translate('exception.bad_request.' . $descriptionKey, $arguments), $code);
+    }
+
+    public function getStatusCode(): int
+    {
+        return Response::HTTP_BAD_REQUEST;
+    }
+}

Classes/Exception/InternalServerErrorException.php

@@ -0,0 +1,22 @@
+<?php
+
+declare(strict_types=1);
+
+namespace SourceBroker\T3api\Exception;
+
+/**
+ * Returned to the client instead of an unexpected exception outside of `Development` context, so the
+ * response does not expose its message (paths, database details, ...). The event ID identifies the
+ * log entry with the original exception.
+ */
+class InternalServerErrorException extends AbstractException
+{
+    public function __construct(string $eventId)
+    {
+        $this->title = self::translate('exception.internal_server_error.title');
+        parent::__construct(
+            self::translate('exception.internal_server_error.description', [$eventId]),
+            1793285051
+        );
+    }
+}

Classes/Filter/DistanceFilter.php

@@ -5,6 +5,7 @@
 namespace SourceBroker\T3api\Filter;
 
 use SourceBroker\T3api\Domain\Model\ApiFilter;
+use SourceBroker\T3api\Exception\BadRequestException;
 use SourceBroker\T3api\OpenApi\Objects\Parameter;
 use SourceBroker\T3api\OpenApi\Objects\Schema;
 use TYPO3\CMS\Core\Database\ConnectionPool;
@@ -142,19 +143,15 @@ public function filterProperty(
     }
 
     /**
-     * @throws \InvalidArgumentException
+     * @throws BadRequestException
      */
     protected function getLatLangParameterValues(array $values, ApiFilter $apiFilter): array
     {
         if (!isset($values[self::PARAMETER_LATITUDE], $values[self::PARAMETER_LONGITUDE])) {
-            throw new \InvalidArgumentException(
-                sprintf(
-                    'Parameters `%s[%s]` and %s[%s] are required to use distance filter',
-                    $apiFilter->getParameterName(),
-                    self::PARAMETER_LATITUDE,
-                    $apiFilter->getParameterName(),
-                    self::PARAMETER_LONGITUDE
-                )
+            throw new BadRequestException(
+                'distance_parameters',
+                [$apiFilter->getParameterName(), self::PARAMETER_LATITUDE, self::PARAMETER_LONGITUDE],
+                1793641752
             );
         }
 

Classes/Filter/OrderFilter.php

@@ -5,6 +5,7 @@
 namespace SourceBroker\T3api\Filter;
 
 use SourceBroker\T3api\Domain\Model\ApiFilter;
+use SourceBroker\T3api\Exception\BadRequestException;
 use SourceBroker\T3api\OpenApi\Objects\Parameter;
 use SourceBroker\T3api\OpenApi\Objects\Schema;
 use TYPO3\CMS\Extbase\Persistence\Generic\Qom\ConstraintInterface;
@@ -45,6 +46,10 @@ public function filterProperty(
             return null;
         }
 
+        if (!is_string($values[$property])) {
+            throw new BadRequestException('order_direction', [gettype($values[$property])], 1560890654236);
+        }
+
         $defaultDirection = $apiFilter->getStrategy()->getName();
         $direction = strtoupper($values[$property] !== '' ? $values[$property] : $defaultDirection);
 
@@ -53,7 +58,7 @@ public function filterProperty(
         }
 
         if (!in_array($direction, [QueryInterface::ORDER_ASCENDING, QueryInterface::ORDER_DESCENDING], true)) {
-            throw new \InvalidArgumentException(sprintf('Unknown order direction `%s`', $direction), 1560890654236);
+            throw new BadRequestException('order_direction', [$values[$property]], 1560890654236);
         }
 
         $query->setOrderings(array_merge($query->getOrderings(), [$property => $direction]));

Classes/OperationHandler/AbstractOperationHandler.php

@@ -8,6 +8,7 @@
 use SourceBroker\T3api\Domain\Model\OperationInterface;
 use SourceBroker\T3api\Domain\Repository\CommonRepository;
 use SourceBroker\T3api\Event\AfterDeserializeOperationEvent;
+use SourceBroker\T3api\Exception\BadRequestException;
 use SourceBroker\T3api\Exception\OperationNotAllowedException;
 use SourceBroker\T3api\Security\OperationAccessChecker;
 use SourceBroker\T3api\Serializer\ContextBuilder\DeserializationContextBuilder;
@@ -56,15 +57,19 @@ protected function getRepositoryForOperation(OperationInterface $operation): Com
     }
 
     /**
+     * @throws BadRequestException
      * @throws OperationNotAllowedException
      */
     protected function deserializeOperation(
         OperationInterface $operation,
         Request $request,
         ?AbstractDomainObject $targetObject = null
     ): AbstractDomainObject {
+        $content = $request->getContent();
+        $this->assertJsonObject($content);
+
         $object = $this->serializerService->deserialize(
-            $request->getContent(),
+            $content,
             $operation->getApiResource()->getEntity(),
             $this->deserializationContextBuilder->createFromOperation($operation, $request, $targetObject)
         );
@@ -81,4 +86,22 @@ protected function deserializeOperation(
 
         return $afterDeserializeOperationEvent->getObject();
     }
+
+    /**
+     * Checked before deserialization, as the serializer reports malformed JSON with a generic runtime exception.
+     *
+     * @throws BadRequestException
+     */
+    protected function assertJsonObject(string $content): void
+    {
+        try {
+            $data = json_decode($content, false, 512, JSON_THROW_ON_ERROR);
+        } catch (\JsonException $exception) {
+            throw new BadRequestException('invalid_json', [$exception->getMessage()], 1793641753);
+        }
+
+        if (!$data instanceof \stdClass) {
+            throw new BadRequestException('json_object', [], 1793641755);
+        }
+    }
 }

Classes/Serializer/Handler/AbstractDomainObjectHandler.php

@@ -8,6 +8,7 @@
 use JMS\Serializer\Metadata\PropertyMetadata;
 use JMS\Serializer\Visitor\DeserializationVisitorInterface;
 use SourceBroker\T3api\Annotation\ORM\Cascade;
+use SourceBroker\T3api\Exception\BadRequestException;
 use SourceBroker\T3api\Service\PropertyInfoService;
 use SourceBroker\T3api\Service\SerializerService;
 use TYPO3\CMS\Extbase\DomainObject\AbstractDomainObject;
@@ -61,8 +62,9 @@ public function deserialize(
                 return $this->processCascadePersistence($data, $targetObjectType, $context);
             }
 
-            throw new \InvalidArgumentException(
-                sprintf('It was not possible to deserialize %s into %s', gettype($data), AbstractDomainObject::class),
+            throw new BadRequestException(
+                'invalid_relation',
+                [gettype($data), implode('.', $context->getCurrentPath())],
                 1584866997736
             );
         }

Classes/Serializer/Handler/ObjectStorageHandler.php

@@ -8,6 +8,7 @@
 use JMS\Serializer\SerializationContext;
 use JMS\Serializer\Visitor\DeserializationVisitorInterface;
 use JMS\Serializer\Visitor\SerializationVisitorInterface;
+use SourceBroker\T3api\Exception\BadRequestException;
 use TYPO3\CMS\Extbase\Persistence\Generic\LazyObjectStorage;
 use TYPO3\CMS\Extbase\Persistence\ObjectStorage;
 
@@ -55,13 +56,9 @@ public function deserialize(
         }
 
         if (!is_array($data)) {
-            throw new \InvalidArgumentException(
-                sprintf(
-                    'Data of type `%s` can not be converted to %s in path `%s`',
-                    gettype($data),
-                    ObjectStorage::class,
-                    implode('.', $context->getCurrentPath())
-                ),
+            throw new BadRequestException(
+                'invalid_collection',
+                [gettype($data), implode('.', $context->getCurrentPath())],
                 1570805126535
             );
         }

Classes/Service/RequestLanguageService.php

@@ -5,6 +5,8 @@
 namespace SourceBroker\T3api\Service;
 
 use Psr\Http\Message\ServerRequestInterface;
+use SourceBroker\T3api\Exception\BadRequestException;
+use TYPO3\CMS\Core\Authentication\BackendUserAuthentication;
 use TYPO3\CMS\Core\Context\Context;
 use TYPO3\CMS\Core\Context\LanguageAspectFactory;
 use TYPO3\CMS\Core\Site\Entity\Site;
@@ -13,9 +15,16 @@
 
 class RequestLanguageService
 {
+    /**
+     * Request attribute with the exception for an invalid language header. It is thrown in
+     * `Bootstrap::process()`, so the client gets a regular API error response.
+     */
+    public const ATTRIBUTE_LANGUAGE_HEADER_EXCEPTION = 't3apiLanguageHeaderException';
+
     public function withLanguageFromHeader(ServerRequestInterface $request): ServerRequestInterface
     {
-        $languageHeader = $request->getHeader($GLOBALS['TYPO3_CONF_VARS']['EXTCONF']['t3api']['languageHeader']);
+        $headerName = $GLOBALS['TYPO3_CONF_VARS']['EXTCONF']['t3api']['languageHeader'];
+        $languageHeader = $request->getHeader($headerName);
         if ($languageHeader === []) {
             return $request;
         }
@@ -26,12 +35,23 @@ public function withLanguageFromHeader(ServerRequestInterface $request): ServerR
             return $request;
         }
 
-        $languageUid = (int)array_shift($languageHeader);
-        if ($language->getLanguageId() === $languageUid) {
+        $headerValue = trim((string)array_shift($languageHeader));
+        if ($headerValue === '') {
+            return $request;
+        }
+
+        $targetLanguage = preg_match('/^\d+$/', $headerValue) === 1 ? $this->getAvailableLanguage($site, (int)$headerValue) : null;
+        if ($targetLanguage === null) {
+            return $request->withAttribute(
+                self::ATTRIBUTE_LANGUAGE_HEADER_EXCEPTION,
+                new BadRequestException('language_header', [$headerName, $headerValue], 1793641754)
+            );
+        }
+
+        if ($language->getLanguageId() === $targetLanguage->getLanguageId()) {
             return $request;
         }
 
-        $targetLanguage = $site->getLanguageById($languageUid);
         GeneralUtility::makeInstance(Context::class)->setAspect(
             'language',
             LanguageAspectFactory::createFromSiteLanguage($targetLanguage)
@@ -41,4 +61,22 @@ public function withLanguageFromHeader(ServerRequestInterface $request): ServerR
             ->withAttribute('t3apiHeaderLanguageRequest', true)
             ->withAttribute('language', $targetLanguage);
     }
+
+    /**
+     * Same rule as for the language in URL (see TYPO3 `SiteBaseRedirectResolver`): a disabled language
+     * is available only for a backend user with access to it.
+     */
+    private function getAvailableLanguage(Site $site, int $languageId): ?SiteLanguage
+    {
+        $language = $site->getAllLanguages()[$languageId] ?? null;
+        if ($language === null || $language->isEnabled()) {
+            return $language;
+        }
+
+        $backendUser = $GLOBALS['BE_USER'] ?? null;
+
+        return $backendUser instanceof BackendUserAuthentication && $backendUser->checkLanguageAccess($languageId)
+            ? $language
+            : null;
+    }
 }

Configuration/RequestMiddlewares.php

@@ -46,6 +46,8 @@
         'target' => \SourceBroker\T3api\Middleware\T3apiRequestLanguageResolver::class,
         'after' => [
             'typo3/cms-frontend/site',
+            // `$GLOBALS['BE_USER']` is needed to allow disabled languages for backend users
+            'typo3/cms-frontend/backend-user-authentication',
         ],
         'before' => [
             'typo3/cms-frontend/tsfe',

Documentation/Miscellaneous/Changelog/Index.rst

@@ -7,6 +7,10 @@ Changelog
 5.0.0
 =====
 - Fix response cache invalidation when the page cache is cleared from the backend (cache tags passed as a list were ignored, numeric tags caused errors).
+- Log unexpected exceptions with an event ID. Outside of ``Development`` context HTTP 500 response contains a generic message with this event ID instead of the exception message. See :ref:`ThrowableSubscriber <serialization_subscribers_throwable_subscriber>`.
+- Return HTTP 500 for PHP errors (e.g. ``TypeError``) as API response with CORS headers instead of rethrowing them to TYPO3. [`issue <https://github.com/sourcebroker/t3api/issues/38>`__]
+- [!!!] Return ``400 Bad Request`` (``SourceBroker\T3api\Exception\BadRequestException``) instead of HTTP 500 for invalid client input: malformed JSON or non-object request body, invalid value of a relation, unknown order direction, missing coordinates of distance filter.
+- [!!!] Validate language header (``X-Locale``): unknown language identifier returns ``400 Bad Request`` instead of HTTP 500, non-numeric value returns ``400 Bad Request`` instead of default language. Disabled language is available only for backend users with access to it. See :ref:`multilanguage`.
 - Refactor ``CurrentFeUser`` handling. See :ref:`current user assignment <use-cases_current-user-assignment>`.
 - [!!!] CORS refactor: unify response handling, normalize configuration lists and improve preflight and cache behavior. Origin patterns now match complete origins. Review ``allowOrigin`` configuration and custom ``OPTIONS`` response handling when upgrading. See :ref:`cors`.
 - [!!!] Embed ``goldspecdigital/oooas`` library (PHP 8.5 compatibility). The package is no longer a dependency, namespace ``GoldSpecDigital\ObjectOrientedOAS`` is renamed to ``SourceBroker\T3api\OpenApi``. Update imports in custom filters implementing ``OpenApiSupportingFilterInterface`` and custom :ref:`collection response classes <customization_collection-response-schema>`. [`commit <https://github.com/sourcebroker/t3api/commit/9d63ab0>`__]

Documentation/Multilanguage/Index.rst

@@ -36,6 +36,10 @@ from your site's ``config.yaml``.
 
    curl -H "X-Locale: 1" https://14.t3api.ddev.site/_api/news/news
 
+An empty header is ignored. A value which is not the identifier of an available language (unknown identifier,
+not a number) results in ``400 Bad Request``. A disabled language (``enabled: false``) is available only for a
+logged in backend user with access to this language - same as for the language prefix in URL.
+
 Here is a part of ``config.yaml`` used for the examples (site configuration of testing instance):
 
 .. code-block:: yaml

Documentation/Serialization/Subscribers/Index.rst

@@ -98,3 +98,8 @@ ThrowableSubscriber
 This subscriber listens to the ``POST_SERIALIZE`` event. If the object is an instance of ``Throwable``, it adds
 ``hydra:description`` (exception message) and - in ``Development`` application context only - ``hydra:debug`` (stack trace)
 to the serialized object.
+
+Unexpected exceptions (not implementing ``SourceBroker\T3api\Exception\ExceptionInterface``) are logged with an event ID
+and result in HTTP 500. Outside of ``Development`` application context the response contains a generic
+``SourceBroker\T3api\Exception\InternalServerErrorException`` with this event ID instead of the original exception, so its
+message is not returned to the client. Use the event ID to find the original exception in the TYPO3 log.