5
\$\begingroup\$

I read some answers here on stack exchange, but none really "hits the nail on the head" about where to throw exceptions, where I shouldn't and where to catch them.

the idea that an exception should be thrown in exceptional cases is very vague, for example, if I can't find a user in my data source (repository) I can't continue the flow of my code, can this be considered exceptional?

I also understand that there are caveats about flow control with exceptions, for me it does not seem right to use try/catch within the service, to catch all the exceptions that a repository/adapter/whatever may throw to control what to do next.

based on this, a code example with a "service" and a controller calling the service and catching all of the service exceptions:

Service:

class CheckUserUltraSecretInformation
{

    public function __construct(
        private CachedUserDataRepository $cachedDataRepository,
    )
    {
    }

    public function handle(string $document)
    {
        $cachedUserData = $this->cachedDataRepository->findByDocument($document);

        if (empty($cachedUserData)) {
            throw new CachedUserDataNotFoundException('User Cached information not found, cannot proceed');
        }

        if ($cachedUserData->hasExpired()) {
            throw new CachedUserDataHasExpiredException('Cached user data is expired');
        }
        // return ultra secret information
    }
}

Controller:

class UserSecretInformationController
{

    public function __construct(
        private CheckUserUltraSecretInformation $checkUserUltraSecretInformation)
    {
    }

    public function checkUserInformation(Request $request)
    {
        $document = $request->input('document');

        try {
            $userUltraSecretInformation = $this->checkUserUltraSecretInformation->handle($document);
        } catch (CachedUserDataNotFoundException $exception) {

            // return json response with http code 400

        } catch (CachedUserDataExpiredException $exception) {
            //returns json response with http code 500
        }
    }
}
  • Is it "correct" that my repository does not throw exceptions, and only returns null/false when it cannot return an entity? if not, what is the problem?

  • if my repository throws exceptions, should my service capture them and re-launch them as service exceptions? is this considered flow control?

  • Are the outer layers the "most correct" to capture and handle exceptions? in my example the controller is used, can It know the name of the services/use-cases exceptions?

\$\endgroup\$
17
  • 2
    \$\begingroup\$ We don't do general "best-practice" questions or example code on Code Review. Please see Why is hypothetical example code off-topic for CR? and How to get the best value out of Code Review: Asking Questions. \$\endgroup\$ Jun 8, 2021 at 15:04
  • \$\begingroup\$ This is my view on the matter, maybe it helps: Point 1) All errors and exceptions thrown by the system in case of a failure (for ex. in case of "db connection could not be established" - see PDO's Errors/Exceptions) should be let to propagate to the entry point of your application. Only at this level should they be handled, like this: \$\endgroup\$
    – user182221
    Oct 26, 2021 at 9:19
  • \$\begingroup\$ 1.a) In a "production" application environment, only a general, user-friendly message should be displayed to the user: "An error occurred during your request. Please try again later". At the same time the detailed error or exception should be logged, for later debug purposes. Recommendation for PHP 7+: Any raised Error should be transformed to ErrorException prior to handling it as previously said. \$\endgroup\$
    – user182221
    Oct 26, 2021 at 9:19
  • \$\begingroup\$ 1.b) In a "development" application environment, the detailed error or exception should be both displayed to the user and logged, for later debug purposes. \$\endgroup\$
    – user182221
    Oct 26, 2021 at 9:20
  • \$\begingroup\$ Recommendation: Use the error handling functions, as following: a) Use register_shutdown_function for handling all fatal errors (here are some examples). Fatal errors are those error types which can not be handled with a user-defined function (like "set_error_handler"). The list of fatal errors is presented here \$\endgroup\$
    – user182221
    Oct 26, 2021 at 9:21

2 Answers 2

6
\$\begingroup\$

the idea that an exception should be thrown in exceptional cases is very vague, for example, if I can't find a user in my data source (repository) I can't continue the flow of my code, can this be considered exceptional?

My personal approach is to raise an exception if I encounter a scenario where I make a truthy assumption during code execution, but that assumption turns out to be falsey. So in your example, if you can't continue the expected code execution path without a valid user object returned from the repository, throw an exception at the point where having a user assumption turns out to be false.

Where to initially throw an exception and where to catch depends on the use case. The rule of thumb I tend to follow is to throw an exception as soon as I can't continue or recover the desired code execution path, then allow that exception to propagate back up the stack to a place that understands how to handle that exception. Don't catch an exception if all you're going to do is re-throw.

Is it "correct" that my repository does not throw exceptions, and only returns null/false when it cannot return an entity? if not, what is the problem?

I would say this is acceptable. Occasionally, a falsey response from your repository is not exceptional and is perfectly acceptable. I would also argue it's not the responsibility of the repository to decide if a falsey response is acceptable or not (for example when checking the existence of a record).

if my repository throws exceptions, should my service capture them and re-launch them as service exceptions? is this considered flow control?

Catch an exception where you intend to handle that exception. Sometimes, it makes sense to catch and re-throw an exception, such as in cases where you want to add information to the exception or cast it to a more meaningful exception for the caller above.

Are the outer layers the "most correct" to capture and handle exceptions? in my example the controller is used, can It know the name of the services/use-cases exceptions?

Catch and handle where it makes sense. In your case, it makes sense as the controller is the last caller before returning a response to the client and it's not the responsibility of your service to format an error response for the caller.

\$\endgroup\$
1
\$\begingroup\$

Throw an exception when your code can't proceed the happy path without some information, or prerequisites. For example, in a hypothetical method makeSale(int $userId) you would throw an exception if there is no matching record for the given ID. You can't return a meaningful value or perform the required action, so you need to throw. If the ID comes from user input, then the user input should be validated before to make sure that it is a real user.

In your example, it looks like you are trying to perform validations for user input. Exceptions are not a good way to do it, although possible. Validations should return boolean true or false to the controller, and then the controller returns a response to the user with an explanatory message. Don't return code 500 for validations; that code is meant for server errors, not user errors. When a user provides wrong information then they need to see an explanatory message.

Expired document or invalid document number are validation errors due to invalid input from the user. This is not a server error. Your controller needs to call the right validation methods that will tell you whether the input is correct or not.

\$\endgroup\$

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