Service Layer Exceptions

[ramoq]

- I had a question regarding architecture(design practices) of the service layers.
- Generally speaking, is it best for services to throw exceptions(checked or unchecked), or to catch them and return back a value (false or null upon error)?
- So the service would catch exceptions then return back the false or null etc.
- My guess is that it should ALWAYS throw exceptions back to the controller(in my case). Returning back false or null does not provide enough information when an error occurs.

Thanks! spring rocks.

[constv]

If you look around you will see that we've spent quite a bit of time and energy discussing exceptions. Just read the previous thread.

1) NO, do not use 0 or 1, or TRUE and FALSE as return codes to indicate success/failure of your service call. That's not good in most cases.

2) Remember that exceptions are for "exceptional" conditions. Errors are always exceptional. Business conditions that are considered exceptional by one client may not be by the other. This does not mean that you should never use exceptions to indicate business conditions. Just exercise caution when you do that - to make sure that you really need to get advantage of the call stack unwinding instantly all the way to the place where you will trigger the alternative action. Since it's usually unfair to your clients to write your service in a way that it "assumes" what's good for them and when and how they should deal with the results of the service calls, you need to think twice before throwing an exception in response to a legitimate business use case (e.g. "account already exists", etc.) Any module that is designed to be used in various contexts by different clients should not expose a checked exception to the client. (Look at Spring!) Your services may throw RTEs, but in each case you should decide individually whether it is ok to throw an exception, or to communicate the operation status via a domain object. For example, off the top of my head, you may have some "MyOrder placeOrder(MyOrder order, ...)" API method on your service that returns the domain object to your controller, e.g. the updated order object. That order object may contain some "status" property that was "PENDING" before, and would be changed to something else after the API returns. If the service fails to save the order due to a serious condition, then throw an exception. If some legitimate business condition was determined, than, perhaps it is more appropriate to reflect it in the order's status rather than exception. The "order" may not be a great example, but you get the drift, right? You need to be passing your domain entities between your controller and services, so take advantage of that in cases when exceptions are not the best solution.

Good luck.

[ramoq]

Thanks constv,

Since i'm a little new to this whole Exception Handling foray, I was wondering if you could clarify some points

1) I'm doing a basic web-app, my service layer(with transactions, dao calls etc) is called directly and only from the controller. Should I go the route of only RTE's and wrap every call to my Service's with a try/catch in the controller so I can correctly display errors to the user(web browser)?? Essentially my controller is the barrier, I dont' want exceptions going further then this. (I may slip in a few checked exceptions)

2) "Any module that is designed to be used in various contexts by different clients should not expose a checked exception to the client" - i'm not getting this fully.


Thanks in advance

[constv]

I'm doing a basic web-app, my service layer(with transactions, dao calls etc) is called directly and only from the controller. Should I go the route of only RTE's and wrap every call to my Service's with a try/catch in the controller so I can correctly display errors to the user(web browser)?? Essentially my controller is the barrier, I dont' want exceptions going further then this. (I may slip in a few checked exceptions)

I would use only RTEs and register an exception resolver (or several, depending on how many different things you need to do in cases of exceptions) that would map exceptions to the appropriate messages and views. That's all. Your controller would have absolutely no error-handling logic. All RTEs would freely propagate through whatever methods are in the call stack and will be caught by the designated exception resolver. (See the Spring reference guide for the info on resolvers.) I assume you are using Spring MVC, but you can do similar stuff with other web frameworks. Here's an example (I copied it from one of my older posts that is buried in some other thread.)

package com.yourcompany.common.web.springmvc;

/**
 * This class implements an exception resolver that redirects to the designated view in cases of any exception that
 * <i>has not been previously caught and handled</i> by the application or framework (including any Spring Webflow error
 * handlers registered with the application). Those are any system and programming errors that should result in a
 * graceful termination of the application.
 * <p/>
 * A bean of this class must be registered with the Spring application context with the order number the highest amongst
 * all error resolvers. By default, the prioritization order value for this error handler is set to {@link
 * Integer#MAX_VALUE} to ensure that this resolver is evaluated <i>after</i> all other registered resolvers have been
 * considered for any exceptions that may require more specific handling. The value may be changed via the {@link
 * #setOrder} method, however, in most cases, that should not be done.
 */
public class SystemErrorResolver implements HandlerExceptionResolver, Ordered {

    private int order = Integer.MAX_VALUE; // prioritization order number for this Ordered object
    private String errorViewName;
    private static final Logger logger = Logger.getLogger(SystemErrorResolver.class);
    private static final String ERR_MESSAGE =
            "Unrecoverable error occured in the application. See the nested exception stack trace for details. Redirecting to the Error View."
            ;

    /**
     * Constructs a SystemErrorResolver object with the given name of the view to render in the case of a system error.
     * This view name must be properly resolved by the Spring MVC DispatcherServlet's view resolver.
     *
     * @param viewName name of the View to render, to be resolved by the DispatcherServlet's ViewResolver
     */
    public SystemErrorResolver(String viewName) {
        errorViewName = viewName;
    }

    public ModelAndView resolveException(HttpServletRequest req, HttpServletResponse response, Object handler,
                                         Exception e) {
        // return the Error Page view in the case of any unhandled exception
        if (e != null) {
            logger.error(ERR_MESSAGE, e);
            
            // add anything else here - whatever you require...

            return new ModelAndView(errorViewName);
        } else {
            return null;
        }
    }

    public int getOrder() {
        return order;
    }

    /**
     * Sets the prioritization order number for this Ordered object. Spring looks at all handler exception resolvers
     * (beans implementing the appropriate HandlerExceptionResolver interface) in the application context. Any instances
     * that implement the Ordered interface are sorted (lowest order first), and others are added at the end of the
     * list. The resolvers are considered in the order they are listed.
     * <p/>
     * NOTE: By default, the prioritization order value for this error handler is set to {@link Integer#MAX_VALUE} to
     * ensure that this resolver is evaluated <i>after</i> all other registered resolvers have been considered for any
     * exceptions that may require more specific handling. <b>Do not call this method unless this behavior must
     * change.</b>
     *
     * @param num order number
     */
    public void setOrder(int num) {
        this.order = num;
    }
}

This resolver is the "last line of defense" for all previously uncaught RTEs, but you can register other resolvers with lower prioritization orders that would intercept your more specific exceptions that require distinct handling. You can read the rest of the post and additional suggestions here:
http://forum.springframework.org/sho...561#post214561

Maybe some other people have other suggestions, but this is how I usually do it. Similar stuff works in Spring WebFlow too (I'd posted an example in one of the threads as well.)

The idea here is to take error handling completely out of your business logic and consolidate it in a single resolver or several resolvers that would catch all possible exceptions and treat them in a consistent way. You can put any logic you want into the handler - to map a particular exception class to a different message from your message resource, or whatever you need to do. You can use a separate resolver for each specific business exception you need to handle distinctly, and then use some generic resolver that would handle all the remaining Throwables in a generic way. This way, nothing will slip through the cracks. All exceptions must be RTEs, of course, in order to propagate to the registered resolvers without interruption.

Since you said you were new to the concept:

When the exception mechanism triggers an exception, the instance of that exception class is broadcast to the methods on the call chain that eventually resulted in the error. The exception travels "back" from one method to another until it gets caught. This means that any of those methods can potentially be coded to catch and handle the exception, if necessary, while others can ignore it. Naturally, there is always one such method in the call chain that is better suited to handle the error than the rest. Which one? The correct answer can only be provided by the application designer, the programmer that is. No tool - a compiler or IDE - can accurately, with 100% certainty determine where in the call stack the given error must be caught and handled. The decision usually depends on the circumstances, such as the application's or use case requirements, etc. So, it is the programmer who has the responsibility to carefully consider all those factors. The error handling constructs must be implemented in the module that contains the sufficient contextual knowledge and ability to take the appropriate corrective action in response to the condition. In the very best case, if possible, error handling should be consolidated in as few places as possible. That's why Spring suggests implementing resolvers that you can register with the framework and map to exception types and "ignore" those exceptions in your business code. The exceptions will fly through your methods and will be caught by the resolvers (the framework will ensure that.) Your job is to ensure that the exceptions are unchecked and to implement the appropriate logic in the resolvers.

Also, in a web application, don't forget to define an exception filter in your web.xml that would ensure that any remaining exceptions in JSPs (tags, etc.) will be caught and forwarded to the error page:

<error-page>
        <exception-type>java.lang.Throwable</exception-type>
        <location>/WEB-INF/jsp/error.jsp</location>
    </error-page>

This way you will end up with a robust web application that never blows up in the user's face. As you test and tune your application you may discover that your generic error page gets displayed in some cases when you would prefer a different behavior/view. That means, a particular exception has propagated too far and, instead of being handled earlier, was handled by your ultimate catch-all resolver. Check your log files, see which exception it is, and map it to the resolver that would forward the logic appropriately. That's very easy.
Read the thread I have mentioned earlier, there's more stuff that you might find useful - in addition to the excerpts above. Just skip over the parts that are not relevant to your case.

[constv]

2) "Any module that is designed to be used in various contexts by different clients should not expose a checked exception to the client" - i'm not getting this fully.

Regarding your second question... When a module throws a checked exception it "assumes" that its client must be forced to do something about it. Generally, it is very rare when you can safely and justifiably make such assumption. It is more appropriate to simply make the information available for those who are interested in it - without forcing it on everybody. So, unless you are putting together a very tight sub-system where you are 100% sure that if A calls B then A must handle exceptions inside B, then checked exceptions will do the job by forcing A to do something. In a vast majority of cases (especially, in the case of frameworks and services that are designed to be used by different clients in potentially different contexts) it is not appropriate to force the immediate caller to handle the exception because the exception most likely should propagate to a handler further up the call stack. At the same time, a specific exception in the method signature is an implementation detail of the method. Consider a typical example: A calls B, and B calls C on some generic service, and C declares a checked exception. B does not know what to do with that exception, so the only correct thing to do is to propagate it to A and let A decide whether it can handle it. The simplest thing is to add that exception to the signature of B without writing any try/catches. But that would expose C's implementation detail to A, and A should not even know that C exist! So, such simple delegation via adding a "throws" clause to B would be inappropriate. The only proper solution in that case would be for B to catch the exception, wrap it into a new exception that is specific to the tier in which B lives and A is aware of (since A is aware of B), and then re-throw. That involves unnecessary useless code clutter - for no reason - since B couldn't do anything about the exception anyway. If the re-thrown exception is checked, the whole thing has to be repeated in A, and so on... That is why, in most cases, the cleanest thing you can do is to throw a RTE and let those who are interested decide for themselves. Yes, the responsibility is still on the programmer, no one has changed that. But it is easy to detect unhandled RTEs and their origins - unlike chasing subtle problems caused by mishandled checked exceptions. One thing you should remember: all methods may fail, not only those that throw checked exceptions. It may sound like a revelation to some folks who were raised on checked exceptions, but it is a very simple fact of life. And when you know that anything may fail, RTEs are working for you, not against.

That said, there are still a lot of people who defend the wider use of checked exceptions. I have expressed my professional opinion.


Hope this helps.
Constantine

[al0]

A very good explanations with only minor drawback - it assumes that exception thrown in C represents some implementation detail rather then business policy. It is not always so. E. g. in order example from one of the previous Constantine's posts "order already exists" represents rather business policy which should be client-independent (such denoting major difference between business services and frameworks like Spring and Hibernate - the main purpose of business services is to implement business rules and polices common for all their clients, violation of such rules definitely denotes exception).

If you offload enforcing of rules to clients then as soon you will have at least 2 different clients you end up with inconsistent (from business point of view) data.

End never say never - even if you have only one client now you can not be sure that you would not have 2 or 3 in a year.

Regards.
Oleksandr

Regarding your second question... When a module throws a checked exception it "assumes" that its client must be forced to do something about it. Generally, it is very rare when you can safely and justifiably make such assumption. It is more appropriate to simply make the information available for those who are interested in it - without forcing it on everybody. So, unless you are putting together a very tight sub-system where you are 100% sure that if A calls B then A must handle exceptions inside B, then checked exceptions will do the job by forcing A to do something. In a vast majority of cases (especially, in the case of frameworks and services that are designed to be used by different clients in potentially different contexts) it is not appropriate to force the immediate caller to handle the exception because the exception most likely should propagate to a handler further up the call stack. At the same time, a specific exception in the method signature is an implementation detail of the method. Consider a typical example: A calls B, and B calls C on some generic service, and C declares a checked exception. B does not know what to do with that exception, so the only correct thing to do is to propagate it to A and let A decide whether it can handle it. The simplest thing is to add that exception to the signature of B without writing any try/catches. But that would expose C's implementation detail to A, and A should not even know that C exist! So, such simple delegation via adding a "throws" clause to B would be inappropriate. The only proper solution in that case would be for B to catch the exception, wrap it into a new exception that is specific to the tier in which B lives and A is aware of (since A is aware of B), and then re-throw. That involves unnecessary useless code clutter - for no reason - since B couldn't do anything about the exception anyway. If the re-thrown exception is checked, the whole thing has to be repeated in A, and so on... That is why, in most cases, the cleanest thing you can do is to throw a RTE and let those who are interested decide for themselves. Yes, the responsibility is still on the programmer, no one has changed that. But it is easy to detect unhandled RTEs and their origins - unlike chasing subtle problems caused by mishandled checked exceptions. One thing you should remember: all methods may fail, not only those that throw checked exceptions. It may sound like a revelation to some folks who were raised on checked exceptions, but it is a very simple fact of life. And when you know that anything may fail, RTEs are working for you, not against.

That said, there are still a lot of people who defend the wider use of checked exceptions. I have expressed my professional opinion.


Hope this helps.
Constantine

[ramoq]

Mamy thanks for everyones feedback. I've started to do some reading into a topic I alwasy took for granted.

One thing that still boggles my mind is the case allowing runtime exceptions to be thrown. I only say this because throughout my formal education and working experience (albeit short) we were groomed to alwasy check everything!

So now when I am coding something like this:

  
public void transferFunds(Account source, Account destination, int amount){
    if(null != source && null != destiation){
        //logic + dao calls, even some more checking to verify amount is in an valid range, not negative        
    }     
}

in order to allow RTE's to be thrown, I woudl totally ignore most if not all superflous checking(null checks, amount verification) as above and only write the logic? (Of course except for contingency cases where we want to throw back a valid checked exception). It feels like my code is vulnerable to breaking?

[al0]

I|m sorry, but your formal education was wrong. To make each and every exception checked is one of the most severe architectural/design errors possible.

Take a look on the following discussion between Constantine and me http://forum.springframework.org/showthread.php?t=67379.

Note that I have advocated checked exceptions .
Rule of thumb is "in case of any doubt make exception unchecked". Make checked exception only if it is part of a business contract of the method.

Regards,
Oleksandr

Mamy thanks for everyones feedback. I've started to do some reading into a topic I alwasy took for granted.

One thing that still boggles my mind is the case allowing runtime exceptions to be thrown. I only say this because throughout my formal education and working experience (albeit short) we were groomed to alwasy check everything!

So now when I am coding something like this:

  
public void transferFunds(Account source, Account destination, int amount){
    if(null != source && null != destiation){
        //logic + dao calls, even some more checking to verify amount is in an valid range, not negative        
    }     
}

in order to allow RTE's to be thrown, I woudl totally ignore most if not all superflous checking(null checks, amount verification) as above and only write the logic? (Of course except for contingency cases where we want to throw back a valid checked exception). It feels like my code is vulnerable to breaking?

[constv]

I|m sorry, but your formal education was wrong. To make each and every exception checked is one of the most severe architectural/design errors possible.

Take a look on the following discussion between Constantine and me http://forum.springframework.org/showthread.php?t=67379.

Note that I have advocated checked exceptions .
Rule of thumb is "in case of any doubt make exception unchecked". Make checked exception only if it is part of a business contract of the method.

Regards,
Oleksandr

+1. Well said. Such "formal education", unfortunately, has caused many people to unjustifiably fear runtime exceptions. Among those who misunderstood checked exceptions and started using them indiscreetly "for everything" it has become typical to handle checked exceptions and completely disregard unchecked exceptions, only to act surprised when the application suddenly crashes. That, in turn, has generated a phobia of unchecked/run-time exceptions among inexperienced programmers. Those who mistakenly believe that run-time exceptions should not be handled, blame them (not themselves!) for application crashes. As the result, a large number of Java programmers today does not even realize that it is their responsibility to design for error handling. A runtime exception is a very useful carrier of the exceptional event information that is openly available for anyone in the call chain who is interested. Unfortunately, many developers show no interest at all in listening for what's out there for them to use. Instead, they all but thoughtlessly react to a compiler alert (often, with annoyance) and do something just to make the compiler error go away, and rest happily thinking that the job is done. Checked or unchecked, programmer's thinking and competence is still the #1 requirement. :-)

[constv]

in order to allow RTE's to be thrown, I woudl totally ignore most if not all superflous checking(null checks, amount verification) as above and only write the logic?

Not necessarily... In some cases, where you feel appropriate, your "business" logic may (or even will need to) actually include checking for nulls or other inconsistencies with your business rules that you (your application) might want to distinguish. See the difference? If the "null" value, for example, tells you that some essential application parameter was not properly configured or not supplied at all, you may check for that condition and throw a more meaningful custom RTE, such as ConfigurationException with a detail message indicating which parameter was null or had another out-of-wack, completely illegal value. Such exception, most likely, should go all the way to the top-level resolver and result in the generic error page (assuming that in this example the parameter is so essential that the application does not have a fall-back behavior for the case of a bad parameter value.) The log file will show you what needs to be fixed, e.g. you need to fix the config files and redeploy the app, or something like that...

HTH,
Constantine