1

When catching and wrapping an exception in PHP, assuming the new exception doesn't have a meaningful code of its own, should you also use the caught exception's code value? or the default value? In other words...

Should I do this?

try {
    do_something();
} catch (GenericOrThirdParty_CaughtException $e) {

    // Reuse the caught exception's code:
    throw new MyCustom_WrappingException('Message', $e->getCode(), $e);
}

Or this?

try {
    do_something();
} catch (GenericOrThirdParty_CaughtException $e) {

    // Use the default exception code:
    throw new MyCustom_WrappingException('Message', 0, $e);
}

I could imagine arguments in favor of either approach, but I'm unaware of an accepted best practice. Is there any? What are the pros and cons of each approach? Is there another approach I haven't considered?

3
  • Are CaughtException and WrappingException a standard PHP exception classes, or classes specific to your application? Jul 19, 2022 at 16:00
  • If you're defining custom exception classes, define custom codes that make sense to your application. Reusing codes defined for existing exception classes could cause confusion. Jul 20, 2022 at 13:05
  • @GregBurghardt CaughtException is a standard PHP or third-party exception, and WrappingException is my own application-specific exception class. Jul 22, 2022 at 20:09

3 Answers 3

4

As a general rule, I'd say that no, you do not need to keep the code.

The exception code is hardly ever relevant; when was the last time you had to handle the exception based on the given code? Unless you're connecting to SOAP servers, the answer might just be "never". It's usually better to split off your exceptions into multiple classes instead.

Also, the wrapped exception can be accessed using $exception->getPrevious(). So if the code does turn out to be relevant, it's not irrecoverably lost, assuming you indeed wrap the old exception when raising the new one.

Consider these examples:

class defaultException extends \Exception {}
class connectionRefusedException extends defaultException {}
class authorizationFailedException extends defaultException {}

try {
  // code
}
catch (connectionRefusedException $e) {
 // ...
}
catch (authorizationFailedException $e) {
  // ...
}
catch (defaultException $e) {
  // ...
}

Versus:

class defaultException extends \Exception {}
try {
  // code
}
catch (defaultException $e) {
  switch ($e->getCode()) {
    case 1:
      // ...
    break;
    case 2:
      // ...
    break;
    default:
      // ...
  }
}

The first example is, in my opinion, cleaner, easier to read and understand, and more language-agnostic. It does not require the exception code to be set. Setting it anyway implies that the code is relevant somehow, which might lead others to misinterpret your intent and write their own code-based exception handler.

To summarize: If you're able, define specialized exceptions, and do not rely on exception codes. When wrapping exceptions, there's hardly ever a reason to copy the code in the original exception, since you probably won't (read: shouldn't) use it.

1
  • This is my thinking, too. We'll see how many other people agree. :) Jul 22, 2022 at 20:16
0

It depends...

If you want to use your newly thrown exception in order to log/debug/... so it can be used by a developper to get informations about errors, it is always better to keep as much details as possible.

If you want to use your newly thrown exception to display an error message to an end user, the end used does not care (and should not know) about the details. In this case you can remove it.

-2

Follow the general principle of writing the simplest code that works:

class WrappingException extends \Exception
{
}

try {
    do_something();
} catch (CaughtException $e) {
    throw new WrappingException('Message');
}

Or you might prefer the almost as simple:

class WrappingException extends \Exception
{
    public function __construct(string $message = 'message')
    {
        parent::__construct($message);
    }
}

try {
    do_something();
} catch (CaughtException $e) {
    throw new WrappingException();
}

Only add more things to the code if you find you want to use them.

1
  • 2
    I think this is bad advice, because your catch block is throwing away potentially useful information by not using the $previous parameter to the WrappingException constructor.
    – IMSoP
    Jul 21, 2022 at 9:01

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

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