Checked vs. Unchecked Exceptions in Service Layer

2019-01-21 20:48发布

问题:

I work on a project with a legacy service layer that returns null in many places if a requested record does not exist, or cannot be accessed due to the caller not being authorized. I am talking about specific records requested by ID. For instance, something like:

UserService.get(userId);

I have recently pushed to have this API changed, or supplemented with a new API that throws exceptions instead. The debate over checked vs unchecked exceptions has ensued.

Taking a note from the designers of JPA/Hibernate et all., I have suggested that unchecked exceptions may be most appropriate. My argument being that users of the API cannot be reasonably expected to recover from these exceptions and in 99% of the cases we can at best notify the application user that some error has occurred.

Having runtime exceptions propagate up to generic handling mechanisms will obviously reduce a lot of the complexity and required branch handling involved in dealing with edge-case exceptions. But, there is a lot of concern surrounding such an approach (rightly so).

Why have the designers of such projects as JPA/EJB and Hibernate selected to go with an unchecked exception model? Is there a very good justification for it? What are the pros/cons. Should developers using these frameworks still handle the runtime exceptions close to where they are thrown with something like adapter wrappers?

I hope answers to these questions can help us to make the "right" decision regarding our own service layer.

回答1:

Though I agree with the view that unchecked exceptions make for a more convenient API, that's not to my mind the most important benefit. Instead it's this:

Throwing unchecked exceptions helps you avoid serious bugs.

Here's why. Given ten developers forced to deal with checked exceptions, you are going to get twenty different strategies for dealing with them, many of which are totally inappropriate. Here are some of the more common bad approaches:

  • Swallow. Catch the exception and completely ignore it. Keep on going as if nothing happened even though the app is now in an unstable state.
  • Log and swallow. Catch the exception and log it, thinking that now we're being responsible. Then keep on going as if nothing happened.
  • Mystery default. Catch the exception, and set some field to some default value, usually without telling the user about it. For example, if you can't load some user's roles, just pick a low-privilege role and assign it. User wonders what is going on.
  • Silly/dangerous mystery default. Catch the exception, and set some field to some really bad default value. One example I saw in real life: can't load a user's roles, so go ahead and assume the best (i.e. give them a high-privilege role so as not to inconvenience anybody).
  • Misreport. The developer doesn't know what the exception means and so just comes up with his own idea. IOException becomes "Can't connect to the server" even if establishing a connection has nothing to do with the issue.
  • Mask via broad catch, and misreport. Try to clean up the code by catching Exception or (ugh) Throwable instead of the two checked exceptions that the method actually throws. The exception handling code makes no attempt to distinguish (say) resource availability issues (e.g. some IOExceptions) from outright code errors (e.g. NullPointerException). Indeed it often picks one of the exceptions arbitrarily and misreports every exception as being of that type.
  • Mask via huge try, and misreport. A variant of the previous strategy is to put a whole bunch of exception-declaring calls in the scope of a single large try block, and then catch either Exception or Throwable because there's nothing else that would handle all the exceptions being thrown.
  • Abstraction-inappropriate rethrow. Rethrow the exception even if the exception is inappropriate to the abstraction (e.g. rethrowing a resource-related exception from a service interface that's supposed to hide the resource).
  • Rethrow without wrapping. Rethrow an exception (either unchecked or else abstraction-appropriate checked), but simply drop the nested exception that would give anybody a chance to actually figure out what is going on.
  • Dramatic response. Respond to a nonfatal exception by exiting the JVM. (Thanks to this blog post for this one.)

In my experience it is quite a bit more common to see the approaches above than it is to see a correct response. Many developers--even "senior" developers--have this idea that exceptions must be suppressed at all costs, even if it means running the app in an unstable state. That is dangerously wrong.

Unchecked exceptions help avoid this issue. Developers who don't know how to handle exceptions tend to see exceptions as inconveniences to overcome, and they don't go out of their way to catch them. So the exceptions just bubble up to the top where they offer up a stack trace and where they can be handled in a consistent fashion. In the rare case where there's actually something better to do than let the exception bubble up, there's nothing stopping you.



回答2:

I could be wrong, but it might be connected to the way EJB container handles exceptions. From Best practices in EJB exception handling:

To use the EJB container's internal housekeeping, you will have to have your checked exceptions thrown as unchecked exceptions.



回答3:

I see the checked/unchecked exception in following perspectives, (taking clue from the standard JDK)

  1. Use Checked exception if you code/library depends on some resource which is external to the JVM (e.g .file/socket/remote api etc i.e JVM has no control over it). Your code MUST handle all the possible error conditions that source may result. This is to make sure that your program is robust and fails gracefully in case something is wrong with that resource.

  2. Unchecked exception are the serious error conditions OR the real BUGS in your code which generally should not be handled AFTER it has occurred. You could correct your code so that this error does not show in the first place. (Eq. NLP, Class cast, Divide by Zero etc)



回答4:

Generally it is a good practice to throw a RuntimeException when your code at the higher level can not handle it and can not do anything about the exception.

In your case your API, will return some result if result is successfull, and should return null if there are no results found, and then throw an exception if there is anything that went wrong while performing the operation.

The exception can be Unchecked, if you are just showing the error message to the user and doing nothing more about the problem.

On the otherhand if you plan to take some alternate steps in case of problem, then you should be throwing an checked exception.

For example, I have a code which is like -

Deduct the payment and then Send shipment details. If(sending shipment details is not successfull) then refund the payment else send success mail.

In the above logic, my logic of sending the shipping details should throw an checked exception becase, if there is any problem, then I have to refund the amount to the customer. If in this case I throw an unchecked exception, the only thing that happens that the user might get the error message.(Unless you catch the runtimeexception - Which is bad.)

Thanks, Sathwick



回答5:

Unchecked exceptions help avoid code clutter. For example consider this code

    try
    {
        File file = new File("file");
        FileReader fileReader = new FileReader(file);
        BufferedReader bufferedReader = new BufferedReader(fileReader);
        String temp = "";
        StringBuilder result = new StringBuilder("");
        while ((temp = bufferedReader.readLine()) != null)
        {
            result.append(temp);
        }
        System.out.println(result);
    }
    catch (FileNotFoundException e)
    {
        e.printStackTrace();
    }
    catch (IOException e)
    {
        e.printStackTrace();
    }

We handle two exception types, but if there is no actionable event that can occur by handling these exceptions, why catch them in the first place ? The answer is not to throw the checked exceptions, since eventually some class up the call hierarchy will have to handle it (Or it gets thrown to the JVM). If the developer is really interested in handling these, the catch the specific RuntimeException class that addresses the problem.

Catch blocks that hold multiple exceptions go some way in reducing this clutter