0

I'm trying to found the best way to manage Exception for all over my application.

Actually, I've come with two solutions :

Solution 1

One Exception to govern them all.

namespace App\Utils\Exception;

class LogicException extends Exception
{
    /**
     * @param MessageI18N $messageI18N A message which may contain parameters for translation.
     * @param string|null $redirectRoute [optional] Route to redirect after exception have been throw.
     * @param string $status [optional] A status for alert msg (default is {@see eNotificationStatus::ERROR}).
     * @inheritDoc
     */
    public function __construct(MessageI18N $messageI18N, ?string $redirectRoute = null, string $status = eNotificationStatus::ERROR, ?int $code = 0, Throwable $previous = null){
        parent::__construct($messageI18N->getMessage(), $code, $previous);
        $this->_redirectRoute = $redirectRoute;
        $this->_status = $status;
        $this->_messageDatas = $messageI18N->getParameters();
    }
}
namespace App\Utils\Exception;

class MessageI18N
{
    private string $_message;
    private array $_parameters;

    public function __construct(string $message, array $parameters = array())
    {
        $this->_message = $message;
        $this->_parameters = $parameters;
    }
...
}

Now I can use this LogicException class everywhere I need it as :

namespace App\Services\Domain\Email;
class EmailService implements EmailServiceInterface
{
    ...

    public function sendEmailResetPassword(string $email): void
    {
        try {
            $this->_mailer->send($email);
        } catch (TransportExceptionInterface $e) {
            throw new LogicException(
               new MessageI18N(
                  "an_error_occurs.when_sending_email_to_{email}",
                  ['{email}' => $email]
               ), 'send_mail_password' // <-- route to redirect
            );
        }
    }
}
namespace App\Controller;
class EmailController extends _BaseController
{
    ...
    /**
     * @Route("email/send/password/forgot/", name="email_send_password_forgot")
     * ...
     */
    public function sendEmailResetPassword(Request $request): void
    {
        ... // Get email from request query
        try{
          $this->_emailService->sendEmailResetPassword($email);
          $this->addFlash( // Success message
             "an_email_have_been_sent_to_{email}_(check_your_spams)",
             ['{email}' => $email])
          );
        }catch(LogicException $e){
          $this->addFlash( // Error message
              $e->getStatus, // Status may be warning or error
              $this->_translator->trans(
                  $e->getMessage,
                  $e->getgetMessageDatas()
              )
          );
        }
        return $this->render('mail_password_forgot_form', ['last_mail' => $email]);
    }
}

Advantages

  • Easy to handle exception
  • Easy to provide extra parameters for I18N purpose
  • Messages can be personalise at Service layer, where it have all the specific knowledge of why error occurs.
  • The route to redirect can be personalise too.

Disadvantages

  • The exception logic take more space than service logic make it hard to read.
  • The way Exception class is extended look like a terrible mess, as an object is provided, but actualy isn't a property of LogicException. IDK why but it's look like so horrible to me.
  • If LogicException need to be change, there might be impact in all place where it is use. A lot of dependecy to this class may require a lot of change !

Solution 2

Solution 2 is about creating a new Exception class for each logic exception wich may occurs.

class EmailInvalidFormat extends \LogicException // <-- SPL\LogicException{
}
class UserNotFoundException extends \LogicException // <-- SPL\LogicException{
}
class UserNameMustContainOnlyAlphanumeric extends \UnexpectedValueException// <-- SPL\UnexpectedValueException{
}
class UserNameLength extends \LengthException// <-- SPL\LengthException{
}
class UserDupplicateName extends \LogicException// <-- SPL\LogicException{
}

... I let you imagine that i'll probably break some records if I create an exception by problem. I thin you guess that for every Exception class I'll display a specific translation message wich would've been handle by the controller. Service layer only throw Exception class without knowing anything about the message.

Advantages

  • More readable code.
  • Everything related to translation is inside of the controller.
  • If an exception class need to be rename, this don't have a lot of impacts.

Disadvantages

  • Less flexibility about message, as controller may not have a lot of informations about each exception.
  • The message will be more generic.
  • Controller got too much responsibility as he's got to manage every exception that'll be thown.
  • Code can't get reuse through other application as Exception may be related to Application logic
4
  • I'm sorry but no. I don't return exception only. Exception is thrown by Services. Controller return a view with a notification containing a user friendly exception translated. – vincent PHILIPPE Oct 14 '20 at 7:14
  • Why using Exception would be a bad thing ? How could I even proceed to return a specific message without it ? Imagine a service where 10 differents kind of exception may occurs. How could you manage to throw a specific message without the use of exceptions. – vincent PHILIPPE Oct 14 '20 at 7:19
  • Throwing exception to stop program execution when error occur is a standard way to proceed phptherightway.com/#exceptions – vincent PHILIPPE Oct 14 '20 at 7:24
  • I'm even more lost when seeing this kind of debate disq.us/p/1bahshv One args for Expression in service layer so he can have extra context. The other one args to keep exception handling only on the public API level. – vincent PHILIPPE Oct 14 '20 at 7:42
1

Error messages are not the same as exception messages

Error messages are what the user sees and exceptions are what the programmer sees. So generally unless you are writing a low level library and being very professional about it, its a waste of time to put translations in for your exceptions.

Even with an json APIs I would just return English (or pick a language) exceptions, The front end can worry about how to present the error message to the user

Secondly, exception inheritance. I know all the books say do it, but again here you are putting in a lot of work for something that really should be covered by validation in the front end rather than exceptions thrown after the fact.

9
  • Actually my exceptions are used to return error message for user to show. That's why I think about inheritance so I can filter user friendly message with translation + context, and on the other hand, technical exceptions for programmer sees. Maybe I'm not taking the right way, but I don't know how I could return multiple error message on a validation service without using exception. They are handy because they prevent the code for being executed. It make the code IMO more readable – vincent PHILIPPE Oct 15 '20 at 8:05
  • yes that what i think you are doing wrong – Ewan Oct 15 '20 at 8:05
  • So how does my services should communicate the encoutered errors to the controller, so the controller will display it in a user friendly way (like alert message) – vincent PHILIPPE Oct 15 '20 at 8:08
  • If a mail fail to be sent, i cannot just let the website crash with a 500 error. I should say to the user to retying again later as an example. – vincent PHILIPPE Oct 15 '20 at 8:10
  • dont communicate via throwing exceptions. return a result from your function call and translate that result in the UI – Ewan Oct 15 '20 at 8:12
0

So as I can see using exception as a flow controll looks bad. Some references to these topics :

So Even though there's a lot of debate around this question, I've make my opinion to not use Exception to stop propagation and take control over the work flow.

First I was thinking that it's a standard way to proceed. Throwing an exception to stop the process and let the controller decide the message to display. It was even possible to filter Exception by using inheritance.

I was even comforted in this idea by the "Fail early" principle. So basically I create all of my service arround this concept of throwing exception if a problem occurs. And then my controller catch my personnal exception (like LogicException) and display it nicely to the view like :

namespace App\Controller;

class ProductController{
   private ProductServiceInterface $productService;
   public function __construct(ProductServiceInterface $productService){
      $this->_productService = $productService;
   }
   
   public function show($id){
      try{
         $product = $this->_productService->getProductById($id);
      }catch(_LogicException $e){
         $this->addFlash($e->getStatus, $e->getMessage());
         if ($e->getRedirectRoute()) return $this->redirectToRoute($e->getRedirectRoute());
      }
   }
}

But the more I use this pattern, the more I see it looks wrong in many way :

  • Controller have responsibility to handle exceptions and returning a Response to a request (Breaking single responsibility principle).
  • Service should have the responsibility to handle exception, as they got some usefull context to provide to exceptions.
  • Keeps the controller as simple as possible ! (One of the best topics about this subject)
  • I have to dupplicate the handing exception logic on every function, for every controller. And dupplicate code is never a good idea. Even though I could just extends all controller on a base class, I still have to surround with try/catch on every functions for every controller. And if I change the exception handling logic I have dependecy on every Controller so I have to change it everywhere arround the application. (Encapsulate What May Change).

I think that many of tutorial propose as an exercice to try/catch incorrect user input so it may lead many misguided peoples (like me) to think that Exception can be use to control wrok flow when a user input is incorrect. Always following the "Fail early" principles, it seams easy to stop the process with an exception, and so I adopt it as a standard way to proceed.

Of course, if you encouter critical error (database connection unavailable, mailer dsn unavailable...), you still have to throw an exception. This is especially true for technical errors. You should've two different behavior, one in debug mode so Exception is thrown and you can debug it easily. And a no debug mode (for production environment...) where technical Exception are stored in a log file, and then a user friendly message display "Something went wrong" and for instance will proceed to redirect user to security.

So finally, I'm going to use a solution based on this solution : https://gist.github.com/javiereguiluz/a66f084a4f2cf11cf0ee#gistcomment-1308966

This is a verry elegant solution to me. Using a event dispatcher and a event listener have a lot of advantages :

  • Giving the responsibility to handle error message to a Event listener.
  • Can access to context through the Event object passed to dispatcher.
  • Can use inheritance to have translation logic inside one EventHelper class.
  • All error message in one place. If a message is needed to be change, there is no need to search for it everywhere.

Tutorial about event listener

What do you think about this solution, please let me know.

Not the answer you're looking for? Browse other questions tagged or ask your own question.