[GEOS-7318] A HTTPErrorCodeException gets ignored when wrapped inside…

… a ServiceException

src/ows/src/main/java/org/geoserver/ows/Dispatcher.java

@@ -1607,7 +1607,9 @@ Map readOpPost(BufferedReader input) throws Exception {
 
     void exception(Throwable t, Service service, Request request) {
         Throwable current = t;
-        while (current != null && !(current instanceof ClientStreamAbortedException) && !(isSecurityException(current))) {
+        while (current != null && !(current instanceof ClientStreamAbortedException) 
+                && !isSecurityException(current)
+                && !(current instanceof HttpErrorCodeException)) {
             if(current instanceof SAXException)
                 current = ((SAXException) current).getException();
             else
@@ -1617,46 +1619,53 @@ void exception(Throwable t, Service service, Request request) {
             logger.log(Level.FINER, "Client has closed stream", t);
             return;
         }
-        if ( isSecurityException(current))
+        if ( isSecurityException(current)) {
             throw (RuntimeException) current;
-
-
-        //unwind the exception stack until we find one we know about 
-        Throwable cause = t;
-        while( cause != null ) {
-            if ( cause instanceof ServiceException ) {
-                break;
-            }
-            if ( cause instanceof HttpErrorCodeException ) {
-                break;
-            }
-            if ( isSecurityException(cause) ) {
-                break;
-            }
-
-            cause = cause.getCause();
-        }
-
-        if ( cause == null ) {
-            //did not fine a "special" exception, create a service exception
-            // by default
-            cause = new ServiceException(t);
         }
 
-        if (!(cause instanceof HttpErrorCodeException)) {
-            logger.log(Level.SEVERE, "", t);
-        } else {
-            int errorCode = ((HttpErrorCodeException)cause).getErrorCode();
+        if (current instanceof HttpErrorCodeException) {
+            HttpErrorCodeException ece = (HttpErrorCodeException) current;
+            int errorCode = ece.getErrorCode();
             if (errorCode < 199 || errorCode > 299) {
                 logger.log(Level.FINE, "", t);
+            } else {
+                logger.log(Level.FINER, "", t);
             }
-            else{
+
+            try {
+                if(ece.getMessage() != null) {
+                    request.getHttpResponse().sendError(ece.getErrorCode(),ece.getMessage());
+                } else {
+                    request.getHttpResponse().sendError(ece.getErrorCode());
+                }
+                if (ece.getErrorCode() < 400) {
+                    // gwc returns an HttpErrorCodeException for 304s
+                    // we don't want to flag these as errors for upstream filters, ie the monitoring extension
+                    t = null;
+                } 
+            } catch (IOException e) {
+                // means the resposne was already commited something
                 logger.log(Level.FINER, "", t);
             }
-        }
-
+        } else {
+            logger.log(Level.SEVERE, "", t);
 
-        if ( cause instanceof ServiceException ) {
+            //unwind the exception stack until we find one we know about 
+            Throwable cause = t;
+            while( cause != null ) {
+                if ( cause instanceof ServiceException ) {
+                    break;
+                }
+
+                cause = cause.getCause();
+            }
+
+            if ( cause == null ) {
+                // did not fine a "special" exception, create a service exception by default
+                cause = new ServiceException(t);
+            }
+
+            // at this point we're sure it'a service exception
             ServiceException se = (ServiceException) cause;
             if ( cause != t ) {
                 //copy the message, code + locator, but set cause equal to root
@@ -1665,28 +1674,6 @@ void exception(Throwable t, Service service, Request request) {
 
             handleServiceException(se,service,request);
         }
-        else if ( cause instanceof HttpErrorCodeException ) {
-            //TODO: log the exception stack trace
-
-            //set the error code
-            HttpErrorCodeException ece = (HttpErrorCodeException) cause;
-            try {
-            	if(ece.getMessage() != null) {
-                	request.getHttpResponse().sendError(ece.getErrorCode(),ece.getMessage());
-            	} else {
-            		request.getHttpResponse().sendError(ece.getErrorCode());
-            	}
-                if (ece.getErrorCode() < 400) {
-                    // gwc returns an HttpErrorCodeException for 304s
-                    // we don't want to flag these as errors for upstream filters, ie the monitoring extension
-                    t = null;
-                }
-            } 
-            catch (IOException e) {
-                //means the resposne was already commited
-                //TODO: something
-            }
-        }
 
         request.error = t;
     }

src/ows/src/test/java/org/geoserver/ows/DispatcherTest.java

@@ -342,6 +342,14 @@ public int available(){
     }
 
     public void testHttpErrorCodeException() throws Exception {
+    	assertHttpErrorCode("httpErrorCodeException");
+    }
+
+    public void testWrappedHttpErrorCodeException() throws Exception {
+        assertHttpErrorCode("wrappedHttpErrorCodeException");
+    }
+
+    private void assertHttpErrorCode(String requestType) throws Exception {
         URL url = getClass().getResource("applicationContext.xml");
 
         FileSystemXmlApplicationContext context = new FileSystemXmlApplicationContext(url.toString());
@@ -373,7 +381,7 @@ public void setCharacterEncoding(String encoding) {
         CodeExpectingHttpServletResponse response = new CodeExpectingHttpServletResponse(new MockHttpServletResponse());
 
         request.setupAddParameter("service", "hello");
-        request.setupAddParameter("request", "httpErrorCodeException");
+		request.setupAddParameter("request", requestType);
         request.setupAddParameter("version", "1.0.0");
 
         request.setRequestURI(
@@ -382,7 +390,7 @@ public void setCharacterEncoding(String encoding) {
 
         dispatcher.handleRequest(request, response);
         assertEquals(HttpServletResponse.SC_NO_CONTENT, response.getStatusCode());
-    }
+	}
 
     /**
      * Assert that if the service bean implements the optional {@link DirectInvocationService}

src/ows/src/test/java/org/geoserver/ows/HelloWorld.java

@@ -1,4 +1,4 @@
-/* (c) 2014 Open Source Geospatial Foundation - all rights reserved
+/* (c) 2014 - 2015 Open Source Geospatial Foundation - all rights reserved
  * (c) 2001 - 2013 OpenPlans
  * This code is licensed under the GPL 2.0 license, available at the root
  * application directory.
@@ -7,6 +7,8 @@
 
 import javax.servlet.http.HttpServletResponse;
 
+import org.geoserver.platform.ServiceException;
+
 public class HelloWorld {
     Message message;
 
@@ -17,4 +19,12 @@ public Message hello(Message message) {
     public void httpErrorCodeException() {
         throw new HttpErrorCodeException( HttpServletResponse.SC_NO_CONTENT );
     }
+
+    public void wrappedHttpErrorCodeException() {
+        try {
+        	throw new HttpErrorCodeException( HttpServletResponse.SC_NO_CONTENT );
+        } catch(Exception e) {
+        	throw new ServiceException("Wrapping code error", e);
+        }
+    }
 }

src/ows/src/test/java/org/geoserver/ows/applicationContext.xml

@@ -37,6 +37,7 @@
 		   <list>
 		     <value>hello</value>
 		     <value>httpErrorCodeException</value>
+		     <value>wrappedHttpErrorCodeException</value>
 		   </list>
 		</constructor-arg>
 	</bean>