[JIRA-OS] Created: (WW-291) Too sophisticated exception handling in servlet dispatcher

Message:

  A new issue has been created in JIRA.

---------------------------------------------------------------------
View the issue:

  http://jira.opensymphony.com/secure/ViewIssue.jspa?key=WW-291


Here is an overview of the issue:
---------------------------------------------------------------------
        Key: WW-291
    Summary: Too sophisticated exception handling in servlet dispatcher
       Type: Bug

     Status: Assigned
   Priority: Major

    Project: WebWork
   Versions:
             2.0-beta1

   Assignee: Patrick Lightbody
   Reporter: Mike Mosiewicz

    Created: Thu, 4 Sep 2003 6:14 PM
    Updated: Thu, 4 Sep 2003 6:14 PM

Description:
The servlet spec assumes that servlet's primary method to signal exceptional situation is to throw ServletException. 

SendError usage is different. It is used to signal a non-OK response, but not necessarily caused by exceptional situation. It is rather assumed that the code that executes send error executes properly.

I think that internal exception should be signaled as it is assumed in the spec - i.e. by wrapping them into a ServletException, not by assuming any exact "visible handling".

Moreover - there is a bad pattern. Error logging should be the first line after catch. Specifically you can't put it after calls that could probably throw an error. response.sendError may potentially throw IllegalStateException, and in fact - it throws it when an error occurs in included (dispatched) request. In such case it is never logged.

I suggest a simpler, more native and bugfree solution.


Index: dispatcher/ServletDispatcher.java
===================================================================
RCS file: /cvs/webwork/src/java/com/opensymphony/webwork/dispatcher/ServletDispatcher.java,v
retrieving revision 1.2
diff -u -r1.2 ServletDispatcher.java
--- dispatcher/ServletDispatcher.java	1 Aug 2003 20:44:32 -0000	1.2
+++ dispatcher/ServletDispatcher.java	4 Sep 2003 23:03:02 -0000
@@ -21,7 +21,6 @@
 
 import java.io.File;
 import java.io.IOException;
-import java.io.PrintWriter;
 
 import java.util.HashMap;
 
@@ -164,30 +163,8 @@
             request.setAttribute("webwork.valueStack", proxy.getValueStack());
             proxy.execute();
         } catch (Exception e) {
-            try {
-                // send a http 500 INTERNAL SERVER ERROR to use the servlet defined error handler
-                // make the exception availible to the web.xml defined error page
-                request.setAttribute("javax.servlet.error.exception", e);
-
-                // for compatibility 
-                request.setAttribute("javax.servlet.jsp.jspException", e);
-
-                // send the error response
-                response.sendError(HttpServletResponse.SC_INTERNAL_SERVER_ERROR, e.getMessage());
-
-                /*
-                                response.setContentType("text/html");
-                                response.setLocale(Configuration.getLocale());
-
-                                PrintWriter writer = response.getWriter();
-                                writer.write("Error executing action: " + e.getMessage());
-                                writer.println("<pre>\n");
-                                e.printStackTrace(response.getWriter());
-                                writer.print("</pre>\n");
-                */
-                log.error("Could not execute action", e);
-            } catch (IOException e1) {
-            }
+			log.error("Could not execute action", e);
+            throw new ServletException("Could not execute action becouse exception occured", e);
         }
     }



---------------------------------------------------------------------
JIRA INFORMATION:
This message is automatically generated by JIRA.

If you think it was sent incorrectly contact one of the administrators:
   http://jira.opensymphony.com/secure/Administrators.jspa

If you want more information on JIRA, or have a bug to report see:
   http://www.atlassian.com/software/jira



-------------------------------------------------------
This sf.net email is sponsored by:ThinkGeek
Welcome to geek heaven.
http://thinkgeek.com/sf