42

Let's say I have a class with a method like this:

/*
 *
 * Loads the user from username.
 *
 * @param string $username The username
 *
 * @return UserInterface
 *
 * @throws userNotFoundException if the user is not found
 */
public function getUser($username)
{
    // someFunction return an UserInterface class if found, or null if not.
    $user = someFunction('SELECT ....', $username);
    if ($user === null) {
        throw new userNotFoundException();
    }

    return $user
}

Now let's say that someFunction could throw a InvalidArgumentException / RuntimeException / PDOException for XYZ reasons. What should I do? And what not?

Number 1

Add all the possible exceptions that could throw someFunction in php-docs.

/*
 *
 * Loads the user from username.
 *
 * @param string $username The username
 *
 * @return UserInterface
 *
 * @throws userNotFoundException if the user is not found
 * @throws InvalidArgumentException
 * @throws ...
 */

Number 2

Add a try-catch block to ensure that the method should throw exceptions ONLY documented

/*
 *
 * Loads the user from username.
 *
 * @param string $username The username
 *
 * @return UserInterface
 *
 * @throws userNotFoundException if the user is not found
 * @throws RuntimeException 
 */
public function getUser($username)
{
    try {
        $user = someFunction('SELECT ....', $username);
    } catch (Exception $e) {
        throw new RuntimeException();
    }

    if ($user === null) {
        throw new userNotFoundException();
    }

    return $user
}

Number 3

Don't do anything.

Federkun
  • 36,084
  • 8
  • 78
  • 90
  • What is the difference of Number1 and Number2 with regards to the documentation? Are you asking about best practice for documentation or handling the exception? – Czar Pino Mar 23 '13 at 14:12

4 Answers4

26

Personally I would consider treating @throws similar to Java's checked exceptions

The way this works in Java is that basically exceptions that inherit from RuntimeException can be thrown and don't have to be handled. Any other types of exceptions must have a try-catch block to handle them. This handling code must be in the caller.

Basically in PHP kinda like this:

When a method has a @throws annotation, you have to add code to handle its exceptions.

Any exceptions that don't get mentioned are optional for handling in the calling code.


Now, I don't 100% follow this principle myself. The whole handling exceptions thing is sort of up to the programmer's preference, but this is just some thoughts on how I think it could be handled in a reasonable fashion.

Jani Hartikainen
  • 42,745
  • 10
  • 68
  • 86
  • yea, I was looking for 'how it could be handled', thanks for sharing! So, the original code that I have presented should be okay, right? If `someFunction` throw any exception, that should be handled by the application handle – Federkun Mar 23 '13 at 14:29
  • 1
    Yep, looks ok. However, keep in mind that exceptions should not be used for control flow. I don't know if your code is just an example, but it looks to me as if a user not being found would be a common scenario which you would usually need to handle - thus it would make more sense to make that explicit by returning a null. – Jani Hartikainen Mar 23 '13 at 14:53
13

With regards to documentation, if a function explicitly throws an exception then it should be included in the function's documentation. Thus, for each throw statement, there should be a corresponding @throws in the PHP documentation.

With regards to handling, if there are some operations that should be executed when the exception is thrown then catch it. Otherwise, let it bubble up -- provided there is a catch statement going to handle it later.

Update:

A few years later, I have changed to the opinion that one should only let an exception "bubble up" unmodified when the exception is still relevant to the module's level of abstraction. Catch and re-throw strategies should be employed to make the exception more meaningful. It should also make error handling more secure by avoiding unnecessary disclosure of information about modules underlying the abstraction.

/**
 * @throws UserNotFoundException
 */
public function getUser($username)
{
    try {
        $user = someFunction('SELECT ....', $username);
    } catch (DatabaseException $dbe) {

        /* Re-throw since a database exception may no longer be
         * meaningful to the caller.
         */
        throw new UserNotFoundException();
    }

    return $user
}
Czar Pino
  • 6,258
  • 6
  • 35
  • 60
  • 2
    Your update is good, though I think it is beneficial to also distinguish [faults and contingencies](https://www.smartics.eu/confluence/display/BLOG/2013/04/28/Faults+and+Contingency+Exceptions), where contingencies would be documented, but not faults. Faults naturally bubble up to some sort of fault boundary (e.g., application top-level) where they are handled as needed (transformed, swallowed, logged, leaked, etc). OTOH, contingencies form part of a module's interface and so should be meaningful to the caller. Simply bubbling contingencies is a form of leaking implementation details. – Ken Wayne VanderLinde Mar 07 '19 at 00:22
  • @KenWayneVanderLinde That is an excellent point! What you say is a much fuller view of exceptions and I completely agree. Thanks a lot for sharing this! – Czar Pino Jul 28 '20 at 17:50
8

From a documentation maintenance point of view, I would only be adding the @throw lines for exceptions that are specifically thrown otherwise you will quickly get your documentation out of date.

acutesoftware
  • 1,091
  • 3
  • 14
  • 33
-1

Number two, but perhaps throw before return since throw actions will occur before a return...

tfont
  • 10,891
  • 7
  • 56
  • 52