Pokemon Catch - Generic Exception Handling In Apex

Pros

One of the primary advantages of the Pokemon Catch is that you reduce the complexity of code in many cases. For example, if you're concerned about a NullPointerException, a CalloutException, a QueryException, and a DmlException, you have two choices:

try {
  ...
} catch(NullPointerException npe) {
  ...
} catch(CalloutException ce) {
  ...
} catch(QueryException qe) {
  ...
} catch(DmlException de) {
  ...
}

... or ...

try {
  ...
} catch(Exception e) {
  ...
}

Which is easier to write? Which is easier to get 100% code coverage on? Code coverage alone may be one benefit of catching Exception directly, if you have many scenarios where you have to handle a bunch of Exception types.

However, both of those patterns are actually types of code smell. The former suggests that the developer couldn't perform many of the usual checks (e.g. a NullPointerException should never occur, because you should be able to simply say if(something != null) ...), while the latter suggests that they have no idea what the API is doing, and so are going to just catch everything and hope for the best.

Cons

The Pokemon Catch usually results in bugs, particularly regression bugs, from going unnoticed until some time after they're in production, usually right around the time users start asking "wait, why is the system behaving this way?" The more times you use Exception this way, the more risk you're inherently taking. It's preferable to have a hard crash with a stack trace than a silently debugged error that may go unnoticed for months.

The Exception

Of course, there are times when you do explicitly want to catch Exception directly. For example, a well-designed Exception-handling framework might wrap each of your methods in a consistent manner that allows solid error reporting, roll-back of partial operations, and so on. For code that advanced, I'd say it's perfectly acceptable. For the other 99.999% of code out there, it should not be used.

The Unfortunate

Unfortunately, every single line of code in the documentation either uses no error handling, or shows only the use of Exception. This is just plain wrong. While it does make it easy for novice programmers to understand what's going on, it'd be far more preferable if they used the correct data types and not abstract so much.

The Documentation

One thing that's notably absent from Apex that was a feature of early Java is the throws keyword. This keyword would force developers calling this method to catch the exception. While I'm glad we don't have that in Apex Code, because some odd 90% of the API probably throws NullPointerException, it was a nicety in terms of self-documenting code.

The documentation itself doesn't tell you which methods can throw exceptions, or which types, which means that we end up with most developers going through NPE paranoia phases; we start using try-catch around every method, because we never know what's going to happen, or (like me), we experiment with null parameters to figure out which methods will break when called with null values.

One thing I'd love to do is to introduce a list of all known methods that throw exceptions, but it would take a lot of work and a lot of maintenance, which I don't have time for. I think it'd be a pretty cool thing to have on hand, though. Even better, I'd love it if salesforce.com would do this for us. Just put a little icon in the manual next to each method that throws NPE (or some other type).

The Takeaway

Avoid catching Exception at all. Ever. It's "never" the correct solution. However, sometimes we just don't have the time to write up all the proper code coverage, or sometimes the code coverage is "impossible" to achieve if we follow best practices. In general, if you can avoid using it, do so, and if it's unavoidable, I'd recommend documenting why it's unavoidable so you have something to reference later.

In Java, I'd agree with the sfdcfox's takeaway. But (particularly in the absence of documentation about which Exceptions Apex will throw), catching Exception can make a lot of sense. I tend to go through various stages as the code goes through its lifecycle and end up with a hybrid where I catch some things specifically, then catch 'em all in the end.

For example, I often use the pattern of having a custom object which represents a callout which needs to be made. It will have a field like Status=(Pending, Complete, Error). A batch process looks for pending requests, makes the callout, then updates their status to show that the callout succeeded or failed.

I might start with "inadequate" exception handling in the first version. I know my method might throw CalloutException, but I don't know what else, so I leave it uncaught in the expectation that testing will crash hard and show me the errors:

try {
    doSomething();
    //Set Status__c=Complete
} catch(CalloutException e) {
    // maybe retry the callout somehow
}

Testing turns out that the external system can return results that I hadn't considered and these cause NullPointerExceptions. If possible, I update doSomething() to handle those cases and add them to my Mock class for unit testing - no change to Exception handling.

Then, testing reveals we get the occasional UNABLE_TO_LOCK_ROW error so the code becomes

try {
    doSomething();
    //Set Status__c=Complete
} catch(CalloutException e) {
    // maybe retry the callout somehow
 } catch(DmlException e) {
     if(e.getDmlType(0) == StatusCode.UNABLE_TO_LOCK_ROW) {
         retries--;
         //Do a retry
     } else {
          throw e;
     }
 }

When we finally push this to production, we can't have untrapped errors or our requests will get stuck in the queue with Status=Pending and clog up the system. So, we end up with a hybrid: first the known errors, then the Rumsfeldian unknown unknowns:

try {
    doSomething();
    //Set Status__c=Complete
} catch(CalloutException e) {
    // maybe retry the callout somehow
 } catch(DmlException e) {
     if(e.getDmlType(0) == StatusCode.UNABLE_TO_LOCK_ROW) {
         retries--;
         //Do a retry
     } else {
          throw e;
     }
 } catch(Exception e) {
     // Log e and set Status=Error
 }

I realize I'm a "late comer" to answering this question, but it was closed by the time I saw it initially. Now that it's been reopened, I feel I have something important to add that's not been given consideration so want to add my thoughts.

Exceptions are about validating Data
Exactly what are exceptions? Exceptions are, for the most part, about validating incoming data. Ask yourself, if data fails validation, does it belong in the database? I don't believe it does. If it's allowed to go through, there must be a system in place that ensures corrective action is going to take place to fix the record that caused the exception. What will happen in some other section of code that you didn't write? It would seem to me the record would be likely to cause another exception to occur.

Database Degrades when Exceptions are caught with no action
So, in allowing a record to move forward into the database that's caused an exception, we're introducing data that is I believe is likely to cause additional problems with other code that may run operations on it. If we can't identify the cause of the exception and don't know what we're looking for, then how can we expect someone else to know how to validate the data we're passing to them? I don't believe we can.

Corrupt Data Propagates errors in other code
It says to me that either we don't understand what our code is doing or else we don't understand the nature of our data. In either case, our work is incomplete. With a true separation of concerns, our data should not be passed to other modules with dependencies. In this situation, how can a developer know if there's a dependency that exists or not? I'd assert that they can't.

Admins can't fix what they don't know to look for
Can we expect an admin to "fix" a record that throws an exception we're unable to predict will be thrown because of it's nature? How will they know what to look for? How will we be able to document repairing the exception and what to look for when it occurs? Once again, I'd assert that's something we can't do!

Could a newbie admin fix the data error?
Further, what happens when an Admin is totally new to Salesforce and has no clue about the data they're responsible for maintaining? What do you think will happen to the quality of the the Org's database in those situations?

What is the cost of a customer's data?
The most valuable thing a Salesforce customer has is their database. It's very expensive for them to create, maintain and/or reproduce. The last thing we as developers want to allow to happen is for their database to become "corrupted" with incomplete, inaccurate or erroneous data. When we allow data to be inserted to the database that's caused an "anonymous exception" we've caught, to me, that's exactly what we're doing. We're degrading the quality of their very costly and expensive database they've invested lots of money in.

What constitutes adequate exception handling?
Writing to a log is an inadequate solution. Who reads them and takes action? Logs don't directly alert an admin to tell them to take action and investigate what might be in out of bounds in a record that would cause the exception to occur. Sending an email to the Admin is at a minimum, what I believe is appropriate for a developer to do when they catch an exception of any kind which they're unable to correct with code; let alone a generic "pokeman" exception.

Would you bet your pension on data filled with exceptions?
The essence of what I'm conveying here is that in my view, to simply catch the exception and allow the data to pass is irresponsible on our part and can be very costly to the customer who owns the org. What happens when they rely on that data for analysis to make decisions in their business? Would you want to rely on a database that contained questionable data to make multimillion dollar decisions? I certainly wouldn't and I can't imagine customers wanting to do that either.

I think I've "beaten the dead horse" enough to make my point as to why I believe this is not a good practice and why it's not one I would use.