views:

245

answers:

2

I recently inherited an application developed with bare servlets and JSPs (i.e.: no frameworks). I've been tasked with cleaning up the error-handling workflow. Currently, each <form> in the workflow submits to a servlet, and based on the result of the form submission, the servlet does one of two things:

  1. If everything is OK, the servlet either forwards or redirects to the next page in the workflow.
  2. If there's a problem, such as an invalid username or password, the servlet forwards to a page specific to the problem condition. For example, there are pages such as AccountDisabled.jsp, AccountExpired.jsp, AuthenticationFailed.jsp, SecurityQuestionIncorrect.jsp, etc.

I need to redesign this system to centralize how problem conditions are handled. So far, I've considered two possible solutions:

  • Exceptions
    • Create an exception class specific to my needs, such as AuthException. Inherit from this class to be more specific when necessary (e.g.: InvalidUsernameException, InvalidPasswordException, AccountDisabledException, etc.). Whenever there's a problem condition, throw an exception specific to the condition. Catch all exceptions via web.xml and route them to the appropriate page(s) with the <error-page> tag.
  • enums
    • Adopt an error code approach, with an enum keeping track of the error code and description. The descriptions can be read from a resource bundle in the finished product.

I'm leaning more toward the enum approach, as an authentication failure isn't really an "exceptional condition" and I don't see any benefit in adding clutter to the server logs. Plus, I'd just be replacing one maintenance headache with another. Instead of separate JSPs to maintain, I'd have separate Exception classes.

I'm planning on implementing "error" handling in a servlet that I'm writing specifically for this purpose. I'm also going to eliminate all of the separate error pages, instead setting an error request attribute with the error message to display to the user and forwarding back to the referrer. Each target servlet (Logon, ChangePassword, AnswerProfileQuestions, etc.) would add an error code to the request and redirect to my new servlet in the event of a problem. My new servlet would look something like this:

public enum Error {
    INVALID_PASSWORD(5000, "You have entered an invalid password."),
    ACCOUNT_DISABLED(5002, "Your account has been disabled."),
    SESSION_EXPIRED(5003, "Your session has expired. Please log in again."),
    INVALID_SECURITY_QUESTION(5004, "You have answered a security question incorrectly.");

    private final int code;
    private final String description;

    Error(int code, String description) {
        this.code = code;
        this.description = description;
    }

    public int getCode() {
        return code;
    }

    public String getDescription() {
        return description;
    }
};

protected void doGet(HttpServletRequest request, HttpServletResponse response)
        throws ServletException, IOException {
    String sendTo = "UnknownError.jsp";
    String message = "An unknown error has occurred.";

    int errorCode = Integer.parseInt((String)request.getAttribute("errorCode"), 10);

    Error errors[] = Error.values();
    Error error = null;

    for (int i = 0; error == null && i < errors.length; i++) {
        if (errors[i].getCode() == errorCode) {
            error = errors[i];
        }
    }

    if (error != null) {
        sendTo = request.getHeader("referer");
        message = error.getDescription();
    }

    request.setAttribute("error", message);

    request.getRequestDispatcher(sendTo).forward(request, response);
}

protected void doPost(HttpServletRequest request, HttpServletResponse response)
        throws ServletException, IOException {
    doGet(request, response);
}

Being fairly inexperienced with Java EE (this is my first real exposure to JSPs and servlets), I'm sure there's something I'm missing, or my approach is suboptimal. Am I on the right track, or do I need to rethink my strategy?

+1  A: 

If you use Exceptions, you can reuse the default ErrorHandling infrastructure. Throwing exceptions doesn't have to litter your server log if you bubble them up the stack, or configure your log to either not log them, or not log stack traces.

Your error handler can then display an appropriate message/navigation/resumption based on Exception type.

jayshao
Sure, it will work, but I agree with the @Christopher Parker that this is a misuse of exceptions -- many of the scenarios that will result in a redirect are not really exceptional but are a common and expected path (just one that is invalid).
JacobM
I don't think I agree - even the language of the post:"If there's a problem, such as an invalid username or password, the servlet forwards to a page specific to the problem condition" - though I suppose it's questionable whether a user entering an invalid password is an exceptional condition - in effect this is error-handling. Just because users do something frequently doesn't make it not-exceptional - only if it deviates from the standard flow due to some error, esp. one that the local code has no context to interpret.
jayshao
+2  A: 

Exception/error handling in (web) applications is a sensitive subject. Some may opt for throwing hard exceptions and catching them in a single place, others may opt for passing around a collection of error messages.

I myself prefer throwing exceptions. This is more clear and concise and better reuseable, maintainable and testable. Design a Validator interface with a validate() method which throws ValidatorException. Implement the desired validators accordingly. Collect the validators and run them one by one in a try/catch block and collect the exceptions there. E.g.

Map<String, String> messages = new HashMap<String, String>();
for (Validator validator : validators) {
    try {
        validator.validate(value);
    } catch (ValidatorException e) { 
        messages.add(fieldname, e.getMessage());
    }
}

And then usually forward the request back to the same page (where you entered the form) and display the error message somewhere next to the input fields or in top or bottom of the form. That's more user-friendly than a different error page which would require the user to click one more button/link to get the form back and the user has to remember/figure what the error actually was.

All standard error messages can of course be stored in an enum, or more preferably in an external resource file, e.g. a properties file. This way it's easier to maintain and also easier to add multiple languages to your webapp.

For unrecoverable errors, such as a dead database or a bug in the code (runtime errors, internal server errors, etc), I'd just let the exception go through all the layers so that you can "catch" it by a generic and custom error page which you can define as <error-page> in web.xml. You can define a separate error page for each type of Exception and/or HTTP status code.

This all is by the way also how the average MVC framework works.

BalusC