Should I add a decorator to handle exceptions?

Question

PHP

Hello.

There is a consumer class (handler) that receives messages from the queue. It contains a try catch block, service methods are called inside, these service methods can throw exceptions, these methods also access repositories, such as creating records in the database and updating, respectively, repositories can also throw exceptions of the PDOException type. All these errors are caught in the consumer (handler), that is, at the very top. I was offered to move exception handling from the consumer to a separate decorator, which, accordingly, will already pull the consumer (handler) method that works with services. That is, logically, the decorator in this case will become a consumer, since this will be the entry point of the message. Is such a layer necessary? How does this make the code more convenient? For now, I think this is an over-complication.

Here is the code

public function __invoke(OfdTicketMessage $ofdTicketMessage): void
    {
        $this->logger->info(
            'OfdTicketMessage was received',
            ['data' => $ofdTicketMessage->toArray()]
        );

        try {
            $this->ofdTicketService->create($ofdTicketMessage);
            $this->orangeApiOfdTicketService->createTicket($ofdTicketMessage);

            $this->ofdTicketService->update(
                $ofdTicketMessage,
                [
                    'processed' => true,
                    'statusCode' => OrangeApiResponseStatus::RESPONSE_CREATED
                ]
            );
        } catch (AlreadyCreatedException|AlreadyProcessedException|ConflictException $e) {
            $this->logger->info(
                'Need to ack message because: ' . $e->getMessage(),
                ['ofdTicketId' => $ofdTicketMessage->getId()]
            );

            throw new  UnrecoverableMessageHandlingException($e->getMessage(), $e->getCode());
        } catch (OfdOrangeApiException $e) {
            $this->logger->error(
                'Ofd orange api error: ' . $e->getMessage(),
                ['ofdTicketId' => $ofdTicketMessage->getId()]
            );

            $this->ofdTicketService->update(
                $ofdTicketMessage,
                [
                    'error' => 'Ofd orange api error: ' . $e->getMessage(),
                    'statusCode' => $e->getCode()
                ]
            );
        } catch (TransportExceptionInterface $e) {
            $this->logger->error(
                'Can\'t send request to ofd orange api: ' . $e->getMessage(),
                ['ofdTicketId' => $ofdTicketMessage->getId()]
            );

            $this->ofdTicketService->update(
                $ofdTicketMessage,
                [
                    'error' => 'Can\'t send request to ofd orange api: ' . $e->getMessage(),
                    'statusCode' => $e->getCode()
                ]
            );
        } catch (\Exception|\Throwable $e) {
            $this->logger->error(
                'Something went wrong: ' . $e->getMessage(),
                ['ofdTicketId' => $ofdTicketMessage->getId()]
            );

            $this->ofdTicketService->update(
                $ofdTicketMessage,
                [
                    'error' => 'Something went wrong: ' . $e->getMessage(),
                    'statusCode' => InternalExceptionStatus::INTERNAL_ERROR
                ]
            );

            throw new  UnrecoverableMessageHandlingException($e->getMessage(), $e->getCode());
        }
    }

Answer 1

Depends on what logic is executed in the exception handlers. If there is just logging of exceptions, which is in no way connected with the logic of the consumer, then it is quite justified to separate this logic. But you can not separate. It all depends on the desire to reuse the consumer and the decorator. It also makes sense to separate such independent things to make unit testing easier.
But if there is more complex logic in exception handling, for example, related to recovery after an error, intertwined with consumer logic, then it will be difficult to separate such logic and it is not always possible to tell whether this should be done without seeing the code.

Answer 2

It's not a decorator, it's a proxy. I would rather do error logging in the exceptions themselves. If the request to api may fail due to api or network server problems, then it is better not to throw an exception right away, but to try to make a couple more attempts, and only then throw an exception.