Is rethrowing an exception leaking an abstraction?

Question

I have an interface method that states in documentation it will throw a specific type of exception. An implementation of that method uses something that throws an exception. The internal exception is caught and the exception declared by the interface contract is thrown. Here's a little code example to better explain. It is written in PHP but is pretty simple to follow.

// in the interface

/**
 * @return This method returns a doohickey to use when you need to foo
 * @throws DoohickeyDisasterException
 */
public function getThatDoohickey();

// in the implementation

public function getThatDoohickey() {

    try {
        $SomethingInTheClass->doSomethingThatThrowsAnException();
    } catch (Exception $Exc) {
        throw new DoohickeyDisasterException('Message about doohickey failure');
    }

    // other code may return the doohickey

}

I'm using this method in an attempt to prevent the abstraction from leaking.

My questions are: Would passing the thrown internal exception as the previous exception be leaking the abstraction? If not, would it be suitable to simply reuse the previous exception's message? If it would be leaking the abstraction could you provide some guidance on why you think it does?

Just to clarify, my question would involve changing to the following line of code

throw new DoohickeyDisasterException($Exc->getMessage(), null, $Exc);

Answer 1

My questions are: Would passing the thrown internal exception as the previous exception be leaking the abstraction? If not, would it be suitable to simply reuse the previous exception's message?

The answer is: "it depends".

Specifically, it depends on the exception, and what it means. In essence, rethrowing it means you are adding that to your interface. Would you do that if you wrote the code you are calling yourself, or is this just to avoid rebranding the exception?

If your exception includes a trace down to the root cause, in the called code, that might leak data outside the abstraction - but, generally, I think that is acceptable because the exception is either handled by the caller (and no one cares where it came from) or shown to the user (and you want to know the root cause, for debugging.)

Frequently, though, just allowing the exception through is a very reasonable choice of implementation, and not a problem of abstraction. Just ask "would I throw this if the code I am calling didn't?"

Answer 2

Strictly speaking, if the 'inner' exception reveals anything about the inner workings of an implementation, you are leaking. So technically, you should always catch everything and throw your own exception type.

BUT.

Quite a few important arguments can be made against this. Most notably:

• Exceptions are exceptional things; they violate the rules of 'normal operation' in many ways already, so they might as well violate encapsulation and interface-level abstraction as well.
• The benefit of having a meaningful stack trace and to-the-point error information often outweighs OOP correctness, as long as you don't leak internal program information past the UI barrier (which would be information disclosure).
• Wrapping every inner exception in a suitable outer exception type can lead to lots of useless boilerplate code, which defeats the entire purpose of exceptions (namely, implement error handling without polluting the regular code flow with error handling boilerplate).

That said, catching and wrapping exceptions often does make sense, if only to add additional information to your logs, or to prevent leaking internal information to the end user. Error handling is still an art form, and it's hard to boil it down to a few general rules. Some exceptions should be bubbled, others shouldn't, and the decision also depends on context. If you're coding a UI, you don't want to let anything through to the user unfiltered; but if you're coding a library that implements some network protocol, bubbling certain exceptions is probably the right thing to do (after all, if the network is down, there is little you can do to enhance the information or recover-and-continue; translating NetworkDownException to FoobarNetworkDownException is just meaningless cruft).

Answer 3

First of all, what you're doing here is not rethrowing an exception. Rethrowing would be to literally throw the same exception, like this:

try {
    $SomethingInTheClass->doSomethingThatThrowsAnException();
} catch (Exception $Exc) {
    throw $Exc;
}

Obviously, this is a bit pointless. Rethrowing is for circumstances where you want to release some resources or log the exception or whatever else you might want to do, without stopping the exception from bubbling up the stack.

What you are doing already is replacing all exceptions, no matter what the cause, with a DoohickeyDisasterException. Which may or may not be what you intend to do. But I would suggest that any time you do this, you should also do as you suggest and pass the original exception as an inner exception, just in case it is useful in the stack trace.

But this is not about leaky abstractions, that's another issue.

To answer the question of whether a rethrown exception (or indeed an uncaught exception) can be a leaky abstraction, I would say that it depends on your calling code. If that code requires knowledge of the abstracted framework then it is a leaky abstraction, because if you replace that framework with another then you have to change code outside of your abstraction.

For example, if DoohickeyDisasterException is a class from the framework and your calling code looks like this, then you have a leaky abstraction:

try {
    $wrapper->getThatDoohickey();
} catch (DoohickeyDisasterException $Exc) {
    echo "The Doohickey is all wrong!";
    echo $Exc->getMessage();
    gotoAHappyPlaceAndSingHappySongs();
}

If you replace your third-party framework with another, which uses a different exception name, and you have to find all these references and change them. Better to create your own Exception class and wrap the framework exception in it.

If, on the other hand, DoohickeyDisasterException is one of your own making then you're not leaking abstractions, you should concern yourself more with my first point.

Answer 4

The rule I try to go by is: wrap it if you caused it. Similarly, if a bug were to be created because of a runtime exception, it should be some sort of DoHickeyException or MyApplicationException.

What that implies is that if the cause of the error is something external to your code, and you're not expecting it to fail then fail gracefully and rethrow it. Likewise, if it's a problem with your code, then wrap it in a (potentially) custom exception.

For instance, if a file is expected to be on disk, or a service is expected to be available, then handle it gracefully and re-throw the error so that the consumer of your code can figure out why that resource is unavailable. If you created too much data for some type, then it's your error to wrap and throw.