Is catching all PHP exceptions best practice?

Viewed 846

For any any that PHP might throw an exception, you can enclose it in a try-catch block. Example:

// Collect file list
try {
    $optParams = [
        'orderBy' => 'modifiedTime',
        'pageSize' => $count, # Google default is 100, maximum is 1000
        'q' => "mimeType = 'application/vnd.google-apps.spreadsheet' and modifiedTime >= '$modifiedAfter'",
        'fields' => 'nextPageToken, files(id,modifiedTime)'
    ];

    $results = $googleService->files->listFiles($optParams);
    foreach ($results->getFiles() as $file) {
        if ($file->getModifiedTime() <= $modifiedAfter) {
            if ($file->getId() <= $idGreaterThan) {
                continue;
            }
        }
        $retval[$file->getId()] = $file->getModifiedTime();
    }
    return $retval;
} catch (\Google_Service_Exception $e) {
    echo 'ERROR GOOGLE SERVICE: ';
    echo json_encode($e->getErrors());
    return null;
}

How should somebody evaluate if catching the error is best practice?

3 Answers

No, it is not good practice to handle every possible exception.

Handle exceptions where you expect exceptions may occur and can handle them, or where you need to fail gracefully in a particular way (such as unwinding some action). For anything else, let it crash.

If you catch all your exceptions, you make debugging difficult for yourself. Additionally, if you don't re-throw or otherwise exit, you have the potential to cause unexpected results elsewhere in your code.

In your specific example above, if you want to output a particular error message, wrap the inner exception with your own and have a general error handler that will output the message.

Exceptions and try/catch are flow control tools to be used. Part of writing robust code is ensuring it safely comes to a halt when it's off the rails.

As said on phpdelusions (https://phpdelusions.net/pdo#errors):

Despite a widespread delusion, you should never catch errors to report them. A module (like a database layer) should not report its errors. This function has to be delegated to an application-wide handler. All we need is to raise an error (in the form of exception) - which we already did. That's all. Nor should you "always wrap your PDO operations in a try/catch" like the most popular tutorial from tutsplus recommends. Quite contrary, catching an exception should be rather an exceptional case (pun intended).

In fact, there is nothing special in PDO exceptions - they are errors all the same. Thus, you have to treat them exactly the same way as other errors. If you had an error handler before, you shouldn't create a dedicated one for PDO. If you didn't care - it's all right too, as PHP is good with basic error handling and will conduct PDO exceptions all right.

To highlight Ebski's comment above, you should attempt some form of global handling. So to answer your question, yes everything should be around a try catch that can handle storing stack traces in files and prints beautiful messages to the user. Here is an example

    /** Attempt to safely catch errors and public alerts in a closure
     * @param callable $lambda
     * @return callable
     */
    public static function catchErrors(callable $lambda): callable
    {
        return function (...$argv) use ($lambda) {
            try {
                ob_start(null,null,  PHP_OUTPUT_HANDLER_CLEANABLE | PHP_OUTPUT_HANDLER_FLUSHABLE | PHP_OUTPUT_HANDLER_REMOVABLE);
                $argv = \call_user_func_array($lambda, $argv);
            } catch (\Throwable $e) {
                if (!$e instanceof PublicAlert) {
                    PublicAlert::danger('Developers make mistakes, and you found a big one! We\'ve logged this event and will be investigating soon.'); // TODO - Change what is logged
                    if (APP_LOCAL) {
                        PublicAlert::warning(\get_class($e) . $e->getMessage());
                    }
                    try {
                        ErrorCatcher::generateLog($e);
                    } catch (\Throwable $e) {
                        PublicAlert::danger('Error handling failed.');
                        print $e->getMessage();
                        PublicAlert::info(json_encode($e));

                    }
                } //elseif (APP_LOCAL) {
                    // Why did we do this
                   // ErrorCatcher::generateLog($e);
                //}
                /** @noinspection CallableParameterUseCaseInTypeContextInspection */
                $argv = null;
            } finally {
                if (ob_get_status() && ob_get_length()) {
                    $out = ob_get_contents();
                    ob_end_clean();
                    print <<<END
                                <div class="callout callout-info">
                                <h4>You have printed to the screen while within the catchErrors() function!</h4>
                                Don't slip up in your production code!
                                <a href="http://carbonphp.com/">Note: All MVC routes are wrapped in this function. Output to the browser should be done within the view! Use this as a reporting tool only.</a>
                                </div><pre>$out</pre>
END;

                }
                Database::verify('Check that all database commit chains have finished successfully. You may need to self::commit().');     // Check that all database commit chains have finished successfully, otherwise attempt to remove
                return $argv;
            }
        };
    }
Related