43

MethodA calls an MethodB which in turn calls MethodC.

There is NO exception handling in MethodB or MethodC. But there is exception handling in MethodA.

In MethodC an exception occurs.

Now, that exception is bubbling up to MethodA, which handles it appropriately.

What is wrong with this?

In my mind, at some point a caller will execute MethodB or MethodC, and when exceptions do occur in those methods, what will be gained from handling exceptions inside those methods, which essentially is just a try/catch/finally block instead of just let them bubble up to the callee?

The statement or consensus around exception handling is to throw when execution cannot continue due to just that - an exception. I get that. But why not catch the exception further up the chain instead of having try/catch blocks all the way down.

I understand it when you need to free up resources. That's a different matter entirely.

  • 46
    Why do you think the consensus is to have a chain of pass through catches? – Caleth May 9 at 11:49
  • With a good IDE and a proper coding style, you can know that some exception can be thrown when a method is called. Handle it or allow it to be propagated is the decision of the caller. I don't see any problem with this. – Hieu Le May 9 at 12:23
  • 14
    If a method can't handle the exception, and is merely rethrowing it, I would say that is a code smell. If a method can't handle the exception, and doesn't need to do anything else when an exception is thrown then there is no need for a try-catch block at all. – Greg Burghardt May 9 at 12:27
  • 7
    "What is wrong with this ?" : nothing – Ewan May 9 at 12:42
  • 5
    Pass-through catching (that doesn't wrap exceptions in different types or anything like that) defeats the whole purpose of exceptions. Exception throwing is a complex mechanism, and it was built intentionally. If pass-through catches were the intended use case, then all you would need is to implement a Result<T> type (a type that either stores a result of a computation, or an error), and return it from your otherwise throwing functions. Propagating an error up the stack would entail reading every return value, checking if its an error, and returning an error if so. – Alexander May 10 at 3:12
137

As a general principle, don't catch exceptions unless you know what to do with them. If MethodC throws an exception, but MethodB has no useful way to handle it, then it should allow the exception to propagate up to MethodA.

The only reasons why a method should have a catch and rethrow mechanism are:

  • You want to convert one exception to a different one that is more meaningful to the caller above.
  • You want to add extra information to the exception.
  • You need a catch clause to clean up resources that would be leaked without one.

Otherwise, catching exceptions at the wrong level tends to result in code that silently fails without providing any useful feedback to the calling code (and ultimately the user of the software). The alternative of catching an exception and then immediately rethrowing it is pointless.

  • 27
    @GregBurghardt if your language has something akin to try ... finally ..., then use that, not catch & rethrow – Caleth May 9 at 13:05
  • 19
    "catching an exception and then immediately rethrowing it is pointless" Depending on the language and how you go about this it can be actively harmful to the codebase. Often people attempting this remove a lot of information about the exception such as the original stacktrace. I've dealt with code where the caller gets an exception that is completely misleading as to what happened and where. – JimmyJames May 9 at 15:12
  • 7
    "don't catch exceptions unless you know what to do with them". That sounds reasonable on first glance, but causes problems later on. What you're doing here is leaking implementation details to your callers. Imagine that you're using a particular ORM in your implementation to load data. If you don't catch that ORM's specific exceptions but just let them bubble up, you cannot replace your data layer without breaking compatibility with existing users. That's one of the more obvious cases, but it can get quite insidious and is hard to catch. – Voo May 9 at 15:50
  • 11
    @Voo In your example you do know what to do with it. Wrap it in a documented exception specific to your code, e.g. LoadDataException and include the original exception details according to your language features, so that future maintainers are able to see the root cause without having to attach a debugger and figure out how to reproduce the problem. – Colin Young May 9 at 16:22
  • 14
    @Voo You seem to have missed the "You want to convert one exception to a different one that is more meaningful to the caller above" reason for catch/rethrow scenarios. – jpmc26 May 9 at 18:00
21

What is wrong with this ?

Absolutely nothing.

Now, that exception is bubbling up to MethodA, which handles it appropriately.

"handles it appropriately" is the important part. That's the crux of Structured Exception Handling.

If your code can do something "useful" with an Exception, go for it. If not, then let well alone.

. . . why not catch the exception further up the chain instead of having try/catch blocks all the way down.

That's exactly what you should be doing. If you're reading code that has handler/rethrowers "all the way down", then you're [probably] reading some pretty poor code.

Sadly, some Developers just see catch blocks as "boiler-plate" code that they throw in (no pun intended) to every method they write, often because they don't really "get" Exception Handling and think they have to add something so that Exceptions don't "escape" and kill their program.

Part of the difficulty here is that, most of the time, this problem won't even get noticed, because Exceptions aren't being thrown all the time but, when they are, the program is going to waste an awful lot of time and effort gradually unpicking the call stack to get up to somewhere that actually does something useful with the Exception.

  • 7
    What's even worse is when the application catches the exception, and then logs it (where hopefully it won't just sit there forever) and attempts to continue on as usual, even when it really can't. – Solomon Ucko May 9 at 17:03
  • 1
    @SolomonUcko: Well, it depends. If for example you are writing a simple RPC server, and an unhandled exception bubbles all the way up to the main event loop, your only reasonable option is to log it, send an RPC error to the remote peer, and resume processing events. The other alternative is to kill the whole program, which will drive your SREs nuts when the server dies in production. – Kevin May 11 at 16:01
  • @Kevin In that case, there should be a single catch at the highest level possible that logs the error and returns an error response. Not catch blocks sprinkled everywhere. If you don't feel like listing every possible checked exception (in languages such as Java), just wrap it in a RuntimeException instead of logging it there, attempting to continue, and running into more errors or even vulnerabilities. – Solomon Ucko May 11 at 18:01
8

You have to make a difference between Libraries and Applications.

Libraries can throw uncaught exceptions freely

When you design a library, at some point you have to think about what can go wrong. Parameters could be in the wrong range or null, external resources could be unavailable, etc.

Your library most often will not have a way to deal with them in a sensible manner. The only sensible solution is to throw an appropriate Exception and let the developer of the Application deal with it.

Applications should always at some point catch exceptions

When an Exception is caught, I like to categorize them as either Errors or Fatal Errors. A regular Error means that a single operation within my Application failed. For instance, an open document could not be saved, because the destination was not writable. The only sensible think for the Application to do is inform the user that the operation could not be completed successfully, give human-readable information in regards to the problem and then let the user decide what to do next.

A Fatal Error is an error the main Application logic cannot recover from. For instance, if the graphics device driver crashes in a video game, there is no way for the Application to "gracefully" inform the user. In this case, a log file should be written and, if possible, the user should be informed in some way or another.

Even in such a severe case, the Application should handle this Exception in a meaningful way. This might include writing a Log file, sending a Crash Report, etc. There is no reason for the Application not to respond to the Exception in some way.

  • Indeed, if you have a library for something like disk write operations or, say, other hardware manipulation, you can end up in all sorts of unexpected events. What if a hard drive is yanked out during writing? What a CD drive shorts out while reading? That's outside your control and while you could do something (e.g., pretend it was successful), it's often a good practice to throw an exception to the library user and let them decide. Maybe a HDDPluggedOutDuringWritingException can be dealt with and is non-fatal for the application. The program can decide what to do with that. – VLAZ May 10 at 16:47
  • 1
    @VLAZ What is fatal vs. non-fatal is something the application has to decide. The library should tell what happened. The Application has to decide how to react to it. – MechMK1 May 13 at 8:45
0

What's wrong with the pattern you describe is that method A will have no way of distinguishing between three scenarios:

  1. Method B failed in an anticipated fashion.

  2. Method C failed in a fashion not anticipated by method B, but while method B was performing an operation that could be safely abandoned.

  3. Method C failed in a fashion not anticipated by method B, but while method B was performing an operation that placed things in a supposed-to-be-temporary incoherent state which B failed to clean up because of C's failure.

The only way method A will be able to distinguish those scenarios will be if the exception thrown from B includes information sufficient for that purpose, or if the stack unwinding for method B causes the object to be left in an explicitly invalidated state. Unfortunately, most exception frameworks make both patterns awkward, forcing programmers to make "lesser evil" design decisions.

  • 2
    Scenario 2 and 3 are bugs in method B. Method A shouldn't try to fix those. – Sjoerd May 10 at 21:12
  • @Sjoerd: How is method B supposed to anticipate all of the ways in which method C might fail? – supercat May 10 at 21:37
  • By well known patterns like performing all operations that might throw into temp variables, then with operations that cannot throw - e.g. swap - swap the old with the new state. Another pattern is defining operations that can be repeated safely, so you can retry the operation without fear of messing up. There are full books about writing 'exception safe code', so I can't tell you everything here. – Sjoerd May 10 at 22:09
  • This is a good point for not using exceptions at all (which would be an excellent decision imho). But I guess, it does not really answer the question, as the OP seems intent on using exceptions in the first place, and only asks where the catch should be. – cmaster May 13 at 5:28
  • @Sjoerd Method B becomes a lot easier to reason about if exceptions are forbidden by the language. Because in that case, you actually see all control flow paths through B, and you don't have to second guess which operators might be overloaded in a throwing fashion (C++) to avoid scenario 3. We are paying a lot in terms of code clarity and safety in order to be lazy about returning errors by "just" throwing exceptions. Because, in the end, error handling is a vital part of code. – cmaster May 13 at 5:33

protected by gnat May 10 at 17:05

Thank you for your interest in this question. Because it has attracted low-quality or spam answers that had to be removed, posting an answer now requires 10 reputation on this site (the association bonus does not count).

Would you like to answer one of these unanswered questions instead?

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