Return-Path: 
 <sling-commits-return-942-apmail-incubator-sling-commits-archive=incubator.apache.org@incubator.apache.org>
Delivered-To: apmail-incubator-sling-commits-archive@locus.apache.org
Received: (qmail 84269 invoked from network); 14 Jan 2008 14:23:19 -0000
Received: from hermes.apache.org (HELO mail.apache.org) (140.211.11.2)
  by minotaur.apache.org with SMTP; 14 Jan 2008 14:23:19 -0000
Received: (qmail 7375 invoked by uid 500); 14 Jan 2008 14:23:08 -0000
Delivered-To: apmail-incubator-sling-commits-archive@incubator.apache.org
Received: (qmail 7341 invoked by uid 500); 14 Jan 2008 14:23:08 -0000
Mailing-List: contact sling-commits-help@incubator.apache.org; run by ezmlm
Precedence: bulk
List-Help: <mailto:sling-commits-help@incubator.apache.org>
List-Unsubscribe: <mailto:sling-commits-unsubscribe@incubator.apache.org>
List-Post: <mailto:sling-commits@incubator.apache.org>
List-Id: <sling-commits.incubator.apache.org>
Reply-To: sling-dev@incubator.apache.org
Delivered-To: mailing list sling-commits@incubator.apache.org
Received: (qmail 7332 invoked by uid 99); 14 Jan 2008 14:23:08 -0000
Received: from nike.apache.org (HELO nike.apache.org) (192.87.106.230)
    by apache.org (qpsmtpd/0.29) with ESMTP; Mon, 14 Jan 2008 06:23:08 -0800
X-ASF-Spam-Status: No, hits=-100.0 required=10.0
	tests=ALL_TRUSTED
X-Spam-Check-By: apache.org
Received: from [140.211.11.3] (HELO eris.apache.org) (140.211.11.3)
    by apache.org (qpsmtpd/0.29) with ESMTP; Mon, 14 Jan 2008 14:22:54 +0000
Received: by eris.apache.org (Postfix, from userid 65534)
	id 97BAA1A9832; Mon, 14 Jan 2008 06:22:45 -0800 (PST)
Content-Type: text/plain; charset="utf-8"
MIME-Version: 1.0
Content-Transfer-Encoding: 7bit
Subject: svn commit: r611810 - in
 /incubator/sling/whiteboard/fmeschbe/resource/sling/core/src/main/java/org/apache/sling/core:
 impl/SlingMainServlet.java impl/request/RequestData.java
 servlets/ErrorHandler.java
Date: Mon, 14 Jan 2008 14:22:38 -0000
To: sling-commits@incubator.apache.org
From: fmeschbe@apache.org
X-Mailer: svnmailer-1.0.8
Message-Id: <20080114142245.97BAA1A9832@eris.apache.org>
X-Virus-Checked: Checked by ClamAV on apache.org

Author: fmeschbe
Date: Mon Jan 14 06:22:35 2008
New Revision: 611810

URL: http://svn.apache.org/viewvc?rev=611810&view=rev
Log:
Improve error handling

Modified:
    incubator/sling/whiteboard/fmeschbe/resource/sling/core/src/main/java/org/apache/sling/core/impl/SlingMainServlet.java
    incubator/sling/whiteboard/fmeschbe/resource/sling/core/src/main/java/org/apache/sling/core/impl/request/RequestData.java
    incubator/sling/whiteboard/fmeschbe/resource/sling/core/src/main/java/org/apache/sling/core/servlets/ErrorHandler.java

Modified: incubator/sling/whiteboard/fmeschbe/resource/sling/core/src/main/java/org/apache/sling/core/impl/SlingMainServlet.java
URL: http://svn.apache.org/viewvc/incubator/sling/whiteboard/fmeschbe/resource/sling/core/src/main/java/org/apache/sling/core/impl/SlingMainServlet.java?rev=611810&r1=611809&r2=611810&view=diff
==============================================================================
--- incubator/sling/whiteboard/fmeschbe/resource/sling/core/src/main/java/org/apache/sling/core/impl/SlingMainServlet.java (original)
+++ incubator/sling/whiteboard/fmeschbe/resource/sling/core/src/main/java/org/apache/sling/core/impl/SlingMainServlet.java Mon Jan 14 06:22:35 2008
@@ -24,6 +24,7 @@
 
 import java.io.IOException;
 import java.io.PrintWriter;
+import java.net.SocketException;
 import java.net.URL;
 import java.security.AccessControlException;
 import java.util.ArrayList;
@@ -79,7 +80,7 @@
 
 /**
  * The <code>SlingMainServlet</code> TODO
- *
+ * 
  * @scr.component immediate="true" label="%sling.name"
  *                description="%sling.description"
  * @scr.property name="sling.root" value="/" private="true"
@@ -136,7 +137,7 @@
 
     /** @scr.reference cardinality="0..1" policy="dynamic" */
     private ErrorHandler errorHandler;
-    
+
     private SlingFilterChainHelper requestFilterChain = new SlingFilterChainHelper();
 
     private SlingFilterChainHelper innerFilterChain = new SlingFilterChainHelper();
@@ -150,12 +151,51 @@
     // ---------- Servlet API -------------------------------------------------
 
     public void service(ServletRequest req, ServletResponse res)
-            throws ServletException, IOException {
+            throws ServletException {
 
         if (req instanceof HttpServletRequest
             && res instanceof HttpServletResponse) {
 
-            this.service((HttpServletRequest) req, (HttpServletResponse) res);
+            try {
+                
+                // real request handling for HTTP requests
+                service((HttpServletRequest) req, (HttpServletResponse) res);
+                
+            } catch (IOException ioe) {
+                
+                // unwrap any causes (Jetty wraps SocketException in EofException)
+                Throwable cause = ioe;
+                while (cause.getCause() != null) {
+                    cause = cause.getCause();
+                }
+             
+                if (cause instanceof SocketException) {
+
+                    // if the cause is a SocketException, the client most probably
+                    // aborted the request, we do not fill the log with errors
+                    // in this case
+                    log.debug(
+                        "service: Socketexception (Client abort or network problem",
+                        ioe);
+                    
+                } else {
+                    
+                    // otherwise we want to know why the servlet failed
+                    log.error(
+                        "service: Uncaught IO Problem while handling the request",
+                        ioe);
+                }
+                
+            } catch (Throwable t) {
+                
+                // some failure while handling the request, log the issue
+                // and terminate. We do not call error handling here, because
+                // we assume the real request handling would have done this.
+                // So here we just log
+                
+                log.error("service: Uncaught Problem handling the request", t);
+            }
+
         } else {
             throw new ServletException(
                 "Apache Sling must be run in an HTTP servlet environment.");
@@ -164,65 +204,80 @@
 
     // ---------- Request Handling on behalf of the servlet -------------------
 
-    public void service(HttpServletRequest clientRequest,
-            HttpServletResponse clientResponse) throws ServletException, IOException {
+    public void service(HttpServletRequest servletRequest,
+            HttpServletResponse servletResponse) throws IOException {
 
-        Session session = (Session) clientRequest.getAttribute(SlingHttpContext.SESSION);
-        if (session != null) {
-            RequestData requestData = null;
-            try {
+        // the resource manager factory may be missing
+        if (getResourceResolverFactory() == null) {
+            log.error("service: Cannot handle requests without a ResourceResolverFactory");
+            sendError(HttpServletResponse.SC_NOT_FOUND,
+                "No resource can be found", null, servletRequest,
+                servletResponse);
+            return;
+        }
 
-                // prepare internal request stuff
-                requestData = new RequestData(this, session, clientRequest,
-                    clientResponse);
-                clientRequest = requestData.getSlingRequest();
-                clientResponse = requestData.getSlingResponse();
-
-                Filter[] filters = requestFilterChain.getFilters();
-                if (filters != null) {
-                    FilterChain processor = new RequestSlingFilterChain(this,
-                        filters);
+        Session session = (Session) servletRequest.getAttribute(SlingHttpContext.SESSION);
+        if (session == null) {
+            log.error("service: Cannot handle request: Missing JCR Session");
+            sendError(HttpServletResponse.SC_INTERNAL_SERVER_ERROR,
+                "Missing JCR Session", null, servletRequest, servletResponse);
+            return;
+        }
 
-                    processor.doFilter(clientRequest, clientResponse);
+        // setting the Sling request and response
+        RequestData requestData = new RequestData(this,
+            getResourceResolverFactory().getResourceResolver(session),
+            servletRequest, servletResponse);
+        SlingHttpServletRequest request = requestData.getSlingRequest();
+        SlingHttpServletResponse response = requestData.getSlingResponse();
 
-                } else {
-                    log.error("service: No Request Handling filters, cannot process request");
-                    clientResponse.sendError(SC_INTERNAL_SERVER_ERROR,
-                        "Cannot process Request");
-                }
+        try {
 
-            } catch (AccessControlException ace) {
+            // initialize the request data - resolve resource and servlet
+            requestData.init();
 
-                // try to request authentication fail, if not possible
-                getSlingAuthenticator().requestAuthentication(clientRequest,
-                    clientResponse);
-
-            } catch (HttpStatusCodeException hsce) {
-                // convert the status code exception to sendError
-                getErrorHandler().handleError(hsce.getStatusCode(),
-                    hsce.getMessage(), clientRequest, clientResponse);
+            Filter[] filters = requestFilterChain.getFilters();
+            if (filters != null) {
+                FilterChain processor = new RequestSlingFilterChain(this,
+                    filters);
+
+                processor.doFilter(request, response);
+
+            } else {
+                log.error("service: No Request Handling filters, cannot process request");
+                response.sendError(SC_INTERNAL_SERVER_ERROR,
+                    "Cannot process Request");
+            }
 
-            } catch (Throwable t) {
-                
-                // if we have request data and a non-null active servlet name
-                // we assume, that this is the name of the causing servlet
-                if (requestData != null
-                    && requestData.getActiveServletName() != null) {
-                    clientRequest.setAttribute(ERROR_SERVLET_NAME,
-                        requestData.getActiveServletName());
-                }
-                
-                getErrorHandler().handleError(t, clientRequest, clientResponse);
+        } catch (AccessControlException ace) {
 
-            } finally {
-                if (requestData != null) {
-                    requestData.dispose();
-                }
+            // try to request authentication fail, if not possible
+            getSlingAuthenticator().requestAuthentication(request, response);
 
-                session.logout();
+        } catch (HttpStatusCodeException hsce) {
+            // convert the status code exception to sendError
+            getErrorHandler().handleError(hsce.getStatusCode(),
+                hsce.getMessage(), request, response);
+
+        } catch (Throwable t) {
+
+            // if we have request data and a non-null active servlet name
+            // we assume, that this is the name of the causing servlet
+            if (requestData != null
+                && requestData.getActiveServletName() != null) {
+                request.setAttribute(ERROR_SERVLET_NAME,
+                    requestData.getActiveServletName());
             }
-        }
 
+            getErrorHandler().handleError(t, request, response);
+
+        } finally {
+            if (requestData != null) {
+                requestData.dispose();
+            }
+
+            session.logout();
+        }
     }
 
     // ---------- Generic Content Request processor ----------------------------
@@ -288,67 +343,39 @@
     }
 
     // ---------- ErrorHandler interface (default implementation) --------------
-    
+
     // reset the response, set the status and write a simple message
     public void handleError(int status, String message,
-            HttpServletRequest request, HttpServletResponse response)
+            SlingHttpServletRequest request, SlingHttpServletResponse response)
             throws IOException {
 
         if (message == null) {
-            message = String.valueOf(status);
+            message = "HTTP ERROR:" + String.valueOf(status);
         } else {
-            message = status + " - " + message;
+            message = "HTTP ERROR:" + status + " - " + message;
         }
-        
-        if (response.isCommitted()) {
-            log.error("handleError: Response already committed; cannot send error "
-                + status + message);
-        } else {
-        
-            // error situation
-            String servletName = (String) request.getAttribute(ERROR_SERVLET_NAME);
-            String requestURI = (String) request.getAttribute(ERROR_REQUEST_URI);
-            if (requestURI == null) {
-                requestURI = request.getRequestURI();
-            }
-
-            // reset anything in the response first
-            response.reset();
-            
-            // set the status, content type and encoding
-            response.setStatus(status);
-            response.setContentType("text/html; charset=UTF-8");
-            
-            PrintWriter pw = response.getWriter();
-            pw.println("<html><head><title>");
-            pw.println(message);
-            pw.println("</title></head><body><h1>");
-            pw.println("HTTP ERROR:" + message);
-            pw.println("</h1><p>");
-            pw.println("RequestURI=" + requestURI);
-            if (servletName != null) {
-                pw.println("</p>Servlet=" + servletName + "<p>");
-            }
-            pw.println("</p><hr /><address>");
-            pw.println(getServerInfo());
-            pw.println("</address></body></html>");
 
-            // commit the response
-            response.flushBuffer();
-
-        }
+        sendError(status, message, null, request, response);
     }
 
     // just rethrow the exception as explained in the class comment
-    public void handleError(Throwable throwable, HttpServletRequest request,
-            HttpServletResponse response) throws IOException {
+    public void handleError(Throwable throwable,
+            SlingHttpServletRequest request, SlingHttpServletResponse response)
+            throws IOException {
+        sendError(HttpServletResponse.SC_INTERNAL_SERVER_ERROR,
+            throwable.getMessage(), throwable, request, response);
+    }
+
+    private void sendError(int status, String message, Throwable throwable,
+            HttpServletRequest request, HttpServletResponse response)
+            throws IOException {
 
         if (response.isCommitted()) {
             log.error(
-                "handleError: Response already committed; cannot send error",
-                throwable);
+                "handleError: Response already committed; cannot send error "
+                    + status + message, throwable);
         } else {
-        
+
             // error situation
             String servletName = (String) request.getAttribute(ERROR_SERVLET_NAME);
             String requestURI = (String) request.getAttribute(ERROR_REQUEST_URI);
@@ -358,14 +385,14 @@
 
             // reset anything in the response first
             response.reset();
-            
+
             // set the status, content type and encoding
-            response.setStatus(HttpServletResponse.SC_INTERNAL_SERVER_ERROR);
+            response.setStatus(status);
             response.setContentType("text/html; charset=UTF-8");
 
             PrintWriter pw = response.getWriter();
             pw.println("<html><head><title>");
-            pw.println(throwable.getMessage());
+            pw.println(message);
             pw.println("</title></head><body><h1>");
             pw.println(throwable.toString());
             pw.println("</h1><p>");
@@ -373,18 +400,24 @@
             if (servletName != null) {
                 pw.println("</p>Servlet=" + servletName + "<p>");
             }
-            pw.println("</p><pre>");
-            throwable.printStackTrace(pw);
-            pw.println("</pre><hr /><address>");
+            pw.println("</p>");
+
+            if (throwable != null) {
+                pw.println("<pre>");
+                throwable.printStackTrace(pw);
+                pw.println("</pre>");
+            }
+
+            pw.println("<hr /><address>");
             pw.println(getServerInfo());
             pw.println("</address></body></html>");
-            
+
             // commit the response
             response.flushBuffer();
 
         }
     }
-    
+
     // ---------- Internal helper ----------------------------------------------
 
     public String getServerInfo() {

Modified: incubator/sling/whiteboard/fmeschbe/resource/sling/core/src/main/java/org/apache/sling/core/impl/request/RequestData.java
URL: http://svn.apache.org/viewvc/incubator/sling/whiteboard/fmeschbe/resource/sling/core/src/main/java/org/apache/sling/core/impl/request/RequestData.java?rev=611810&r1=611809&r2=611810&view=diff
==============================================================================
--- incubator/sling/whiteboard/fmeschbe/resource/sling/core/src/main/java/org/apache/sling/core/impl/request/RequestData.java (original)
+++ incubator/sling/whiteboard/fmeschbe/resource/sling/core/src/main/java/org/apache/sling/core/impl/request/RequestData.java Mon Jan 14 06:22:35 2008
@@ -125,9 +125,9 @@
      */
     private String activeServletName;
 
-    public RequestData(SlingMainServlet slingMainServlet, Session session,
-            HttpServletRequest request, HttpServletResponse response)
-            throws ServletException, IOException {
+    public RequestData(SlingMainServlet slingMainServlet,
+            ResourceResolver resourceResolver, HttpServletRequest request,
+            HttpServletResponse response) {
 
         this.slingMainServlet = slingMainServlet;
 
@@ -141,22 +141,15 @@
 
         this.requestProgressTracker = new SlingRequestProgressTracker();
 
-        // the resource manager factory may be missing
-        JcrResourceResolverFactory rmf = slingMainServlet.getResourceResolverFactory();
-        if (rmf == null) {
-            log.error("RequestData: Missing JcrResourceResolverFactory");
-            throw new HttpStatusCodeException(HttpServletResponse.SC_NOT_FOUND,
-                "No resource can be found");
-        }
-
-        // officially, getting the manager may fail, but not i this
-        // implementation
-        this.resourceResolver = rmf.getResourceResolver(session);
+        this.resourceResolver = resourceResolver;
+    }
+    
+    public void init() throws ServletException {
 
         // resolve the resource and the request path info, will never be null
-        Resource resource = resourceResolver.resolve(request);
+        Resource resource = resourceResolver.resolve(getServletRequest());
         RequestPathInfo requestPathInfo = new SlingRequestPathInfo(resource,
-            request.getPathInfo());
+            getServletRequest().getPathInfo());
         ContentData contentData = pushContent(resource, requestPathInfo);
 
         // finally resolve the servlet for the resource

Modified: incubator/sling/whiteboard/fmeschbe/resource/sling/core/src/main/java/org/apache/sling/core/servlets/ErrorHandler.java
URL: http://svn.apache.org/viewvc/incubator/sling/whiteboard/fmeschbe/resource/sling/core/src/main/java/org/apache/sling/core/servlets/ErrorHandler.java?rev=611810&r1=611809&r2=611810&view=diff
==============================================================================
--- incubator/sling/whiteboard/fmeschbe/resource/sling/core/src/main/java/org/apache/sling/core/servlets/ErrorHandler.java (original)
+++ incubator/sling/whiteboard/fmeschbe/resource/sling/core/src/main/java/org/apache/sling/core/servlets/ErrorHandler.java Mon Jan 14 06:22:35 2008
@@ -21,9 +21,9 @@
 import java.io.IOException;
 
 import javax.servlet.ServletException;
-import javax.servlet.http.HttpServletRequest;
-import javax.servlet.http.HttpServletResponse;
-import javax.servlet.http.HttpServletResponseWrapper;
+
+import org.apache.sling.api.SlingHttpServletRequest;
+import org.apache.sling.api.SlingHttpServletResponse;
 
 /**
  * The <code>ErrorHandler</code> defines the interface of the service used by
@@ -48,8 +48,9 @@
      * @throws IOException May be thrown if an error occurrs sending the
      *             response.
      */
-    void handleError(int status, String message, HttpServletRequest request,
-            HttpServletResponse response) throws IOException;
+    void handleError(int status, String message,
+            SlingHttpServletRequest request, SlingHttpServletResponse response)
+            throws IOException;
 
     /**
      * Called to render a response for an uncaught <code>Throwable</code>.
@@ -66,7 +67,7 @@
      * @throws IOException May be thrown if an error occurrs sending the
      *             response.
      */
-    void handleError(Throwable throwable, HttpServletRequest request,
-            HttpServletResponse response) throws ServletException, IOException;
+    void handleError(Throwable throwable, SlingHttpServletRequest request,
+            SlingHttpServletResponse response) throws IOException;
 
 }