hc-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From ol...@apache.org
Subject svn commit: r1567578 - in /httpcomponents/httpcore/branches/4.3.x/httpcore-nio/src: main/java/org/apache/http/nio/protocol/HttpAsyncService.java test/java/org/apache/http/nio/protocol/TestHttpAsyncService.java
Date Wed, 12 Feb 2014 09:50:06 GMT
Author: olegk
Date: Wed Feb 12 09:50:05 2014
New Revision: 1567578

URL: http://svn.apache.org/r1567578
Log:
Improved exception handling in HttpAsyncService

Modified:
    httpcomponents/httpcore/branches/4.3.x/httpcore-nio/src/main/java/org/apache/http/nio/protocol/HttpAsyncService.java
    httpcomponents/httpcore/branches/4.3.x/httpcore-nio/src/test/java/org/apache/http/nio/protocol/TestHttpAsyncService.java

Modified: httpcomponents/httpcore/branches/4.3.x/httpcore-nio/src/main/java/org/apache/http/nio/protocol/HttpAsyncService.java
URL: http://svn.apache.org/viewvc/httpcomponents/httpcore/branches/4.3.x/httpcore-nio/src/main/java/org/apache/http/nio/protocol/HttpAsyncService.java?rev=1567578&r1=1567577&r2=1567578&view=diff
==============================================================================
--- httpcomponents/httpcore/branches/4.3.x/httpcore-nio/src/main/java/org/apache/http/nio/protocol/HttpAsyncService.java
(original)
+++ httpcomponents/httpcore/branches/4.3.x/httpcore-nio/src/main/java/org/apache/http/nio/protocol/HttpAsyncService.java
Wed Feb 12 09:50:05 2014
@@ -221,46 +221,41 @@ public class HttpAsyncService implements
 
     public void exception(
             final NHttpServerConnection conn, final Exception cause) {
-        final State state = ensureNotNull(getState(conn));
-        if (state != null) {
-            state.setTerminated();
-            closeHandlers(state, cause);
-            final Cancellable cancellable = state.getCancellable();
-            if (cancellable != null) {
-                cancellable.cancel();
-            }
-            if (cause instanceof HttpException) {
-                if (conn.isResponseSubmitted()
-                        || state.getResponseState().compareTo(MessageState.INIT) > 0)
{
-                    // There is not much that we can do if a response
-                    // has already been submitted
-                    closeConnection(conn);
-                    log(cause);
+        final State state = getState(conn);
+        if (state == null) {
+            shutdownConnection(conn);
+            log(cause);
+            return;
+        }
+        state.setTerminated();
+        closeHandlers(state, cause);
+        final Cancellable cancellable = state.getCancellable();
+        if (cancellable != null) {
+            cancellable.cancel();
+        }
+        if (conn.isResponseSubmitted()
+                || state.getResponseState().compareTo(MessageState.INIT) > 0) {
+            // There is not much that we can do if a response
+            // has already been submitted
+            closeConnection(conn);
+        } else {
+            final HttpContext context = state.getContext();
+            final HttpAsyncResponseProducer responseProducer = handleException(
+                    cause, context);
+            state.setResponseProducer(responseProducer);
+            try {
+                final HttpResponse response = responseProducer.generateResponse();
+                state.setResponse(response);
+                commitFinalResponse(conn, state);
+            } catch (final Exception ex) {
+                shutdownConnection(conn);
+                closeHandlers(state);
+                if (ex instanceof RuntimeException) {
+                    throw (RuntimeException) ex;
                 } else {
-                    final HttpContext context = state.getContext();
-                    final HttpAsyncResponseProducer responseProducer = handleException(
-                            cause, context);
-                    state.setResponseProducer(responseProducer);
-                    try {
-                        final HttpResponse response = responseProducer.generateResponse();
-                        state.setResponse(response);
-                        commitFinalResponse(conn, state);
-                    } catch (final Exception ex) {
-                        shutdownConnection(conn);
-                        closeHandlers(state);
-                        if (ex instanceof RuntimeException) {
-                            throw (RuntimeException) ex;
-                        } else {
-                            log(ex);
-                        }
-                    }
+                    log(ex);
                 }
-            } else {
-                shutdownConnection(conn);
             }
-        } else {
-            shutdownConnection(conn);
-            log(cause);
         }
     }
 
@@ -376,14 +371,17 @@ public class HttpAsyncService implements
         responseProducer.produceContent(encoder, conn);
         state.setResponseState(MessageState.BODY_STREAM);
         if (encoder.isCompleted()) {
-            responseProducer.responseCompleted(context);
+            try {
+                responseProducer.responseCompleted(context);
+                state.reset();
+            } finally {
+                responseProducer.close();
+            }
             if (!this.connStrategy.keepAlive(response, context)) {
                 conn.close();
             } else {
                 conn.requestInput();
             }
-            closeHandlers(state);
-            state.reset();
         }
     }
 
@@ -503,13 +501,15 @@ public class HttpAsyncService implements
 
     protected HttpAsyncResponseProducer handleException(
             final Exception ex, final HttpContext context) {
-        int code = HttpStatus.SC_INTERNAL_SERVER_ERROR;
+        final int code;
         if (ex instanceof MethodNotSupportedException) {
             code = HttpStatus.SC_NOT_IMPLEMENTED;
         } else if (ex instanceof UnsupportedHttpVersionException) {
             code = HttpStatus.SC_HTTP_VERSION_NOT_SUPPORTED;
         } else if (ex instanceof ProtocolException) {
             code = HttpStatus.SC_BAD_REQUEST;
+        } else {
+            code = HttpStatus.SC_INTERNAL_SERVER_ERROR;
         }
         String message = ex.getMessage();
         if (message == null) {
@@ -534,31 +534,34 @@ public class HttpAsyncService implements
 
     private void processRequest(
             final NHttpServerConnection conn,
-            final State state) throws IOException {
-        final HttpAsyncRequestHandler<Object> handler = state.getRequestHandler();
+            final State state) throws IOException, HttpException {
         final HttpContext context = state.getContext();
-        final HttpAsyncRequestConsumer<?> consumer = state.getRequestConsumer();
-        consumer.requestCompleted(context);
+
         state.setRequestState(MessageState.COMPLETED);
         state.setResponseState(MessageState.INIT);
-        final Exception exception = consumer.getException();
-        if (exception != null) {
-            final HttpAsyncResponseProducer responseProducer = handleException(exception,
context);
-            state.setResponseProducer(responseProducer);
-            conn.requestOutput();
-        } else {
-            final HttpRequest request = state.getRequest();
-            final Object result = consumer.getResult();
+
+        final Object result;
+        final HttpAsyncRequestConsumer<?> consumer = state.getRequestConsumer();
+        try {
+            consumer.requestCompleted(context);
+            result = consumer.getResult();
+        } finally {
+            consumer.close();
+        }
+        if (result != null) {
             final HttpResponse response = this.responseFactory.newHttpResponse(HttpVersion.HTTP_1_1,
                     HttpStatus.SC_OK, context);
+            final HttpRequest request = state.getRequest();
             final Exchange httpexchange = new Exchange(request, response, state, conn);
-            try {
-                handler.handle(result, httpexchange, context);
-            } catch (final HttpException ex) {
-                final HttpAsyncResponseProducer responseProducer = handleException(ex, context);
-                state.setResponseProducer(responseProducer);
-                conn.requestOutput();
-            }
+            final HttpAsyncRequestHandler<Object> handler = state.getRequestHandler();
+            handler.handle(result, httpexchange, context);
+        } else {
+            final Exception exception = consumer.getException();
+            final HttpAsyncResponseProducer responseProducer = handleException(
+                    exception != null ? exception : new HttpException("Internal failure processing
request"),
+                    context);
+            state.setResponseProducer(responseProducer);
+            conn.requestOutput();
         }
     }
 
@@ -582,15 +585,18 @@ public class HttpAsyncService implements
 
         if (entity == null) {
             final HttpAsyncResponseProducer responseProducer = state.getResponseProducer();
-            responseProducer.responseCompleted(context);
+            try {
+                responseProducer.responseCompleted(context);
+                state.reset();
+            } finally {
+                responseProducer.close();
+            }
             if (!this.connStrategy.keepAlive(response, context)) {
                 conn.close();
             } else {
                 // Ready to process new request
                 conn.requestInput();
             }
-            closeHandlers(state);
-            state.reset();
         } else {
             state.setResponseState(MessageState.BODY_STREAM);
         }

Modified: httpcomponents/httpcore/branches/4.3.x/httpcore-nio/src/test/java/org/apache/http/nio/protocol/TestHttpAsyncService.java
URL: http://svn.apache.org/viewvc/httpcomponents/httpcore/branches/4.3.x/httpcore-nio/src/test/java/org/apache/http/nio/protocol/TestHttpAsyncService.java?rev=1567578&r1=1567577&r2=1567578&view=diff
==============================================================================
--- httpcomponents/httpcore/branches/4.3.x/httpcore-nio/src/test/java/org/apache/http/nio/protocol/TestHttpAsyncService.java
(original)
+++ httpcomponents/httpcore/branches/4.3.x/httpcore-nio/src/test/java/org/apache/http/nio/protocol/TestHttpAsyncService.java
Wed Feb 12 09:50:05 2014
@@ -36,7 +36,6 @@ import org.apache.http.HttpResponse;
 import org.apache.http.HttpResponseFactory;
 import org.apache.http.HttpStatus;
 import org.apache.http.HttpVersion;
-import org.apache.http.UnsupportedHttpVersionException;
 import org.apache.http.concurrent.Cancellable;
 import org.apache.http.impl.DefaultHttpResponseFactory;
 import org.apache.http.message.BasicHttpEntityEnclosingRequest;
@@ -140,7 +139,7 @@ public class TestHttpAsyncService {
     }
 
     @Test
-    public void testHttpExceptionHandling() throws Exception {
+    public void testExceptionHandling() throws Exception {
         final State state = new HttpAsyncService.State();
         state.setRequestState(MessageState.READY);
         state.setResponseState(MessageState.READY);
@@ -165,7 +164,7 @@ public class TestHttpAsyncService {
     }
 
     @Test
-    public void testHttpExceptionHandlingRuntimeException() throws Exception {
+    public void testExceptionHandlingRuntimeException() throws Exception {
         final State state = new HttpAsyncService.State();
         final HttpContext exchangeContext = state.getContext();
         state.setRequestState(MessageState.READY);
@@ -204,14 +203,14 @@ public class TestHttpAsyncService {
 
         Mockito.doThrow(new IOException()).when(this.httpProcessor).process(
                 Mockito.any(HttpResponse.class), Mockito.eq(exchangeContext));
-        final HttpException httpex = new HttpException();
+        final IOException ioex = new IOException();
 
-        this.protocolHandler.exception(this.conn, httpex);
+        this.protocolHandler.exception(this.conn, ioex);
 
         Mockito.verify(this.conn).shutdown();
-        Mockito.verify(this.requestConsumer).failed(httpex);
+        Mockito.verify(this.requestConsumer).failed(ioex);
         Mockito.verify(this.requestConsumer, Mockito.atLeastOnce()).close();
-        Mockito.verify(this.responseProducer).failed(httpex);
+        Mockito.verify(this.responseProducer).failed(ioex);
         Mockito.verify(this.responseProducer, Mockito.atLeastOnce()).close();
         Mockito.verify(this.cancellable).cancel();
     }
@@ -238,28 +237,7 @@ public class TestHttpAsyncService {
         Mockito.verify(this.responseProducer).close();
     }
 
-    @Test
-    public void testIOExceptionHandling() throws Exception {
-        final State state = new HttpAsyncService.State();
-        state.setRequestState(MessageState.READY);
-        state.setResponseState(MessageState.READY);
-        state.setRequestConsumer(this.requestConsumer);
-        state.setResponseProducer(this.responseProducer);
-        this.connContext.setAttribute(HttpAsyncService.HTTP_EXCHANGE_STATE, state);
-
-        final IOException httpex = new IOException();
-        this.protocolHandler.exception(this.conn, httpex);
-
-        Assert.assertEquals(MessageState.READY, state.getRequestState());
-        Assert.assertEquals(MessageState.READY, state.getResponseState());
-        Mockito.verify(this.conn).shutdown();
-        Mockito.verify(this.requestConsumer).failed(httpex);
-        Mockito.verify(this.requestConsumer).close();
-        Mockito.verify(this.responseProducer).failed(httpex);
-        Mockito.verify(this.responseProducer).close();
-    }
-
-    @Test
+   @Test
     public void testBasicRequest() throws Exception {
         final State state = new HttpAsyncService.State();
         final HttpContext exchangeContext = state.getContext();
@@ -568,38 +546,6 @@ public class TestHttpAsyncService {
     }
 
     @Test
-    public void testRequestHandlingHttpException() throws Exception {
-        final State state = new HttpAsyncService.State();
-        final HttpContext exchangeContext = state.getContext();
-        final BasicHttpEntityEnclosingRequest request = new BasicHttpEntityEnclosingRequest("POST",
"/",
-                HttpVersion.HTTP_1_1);
-        state.setRequestState(MessageState.BODY_STREAM);
-        state.setRequest(request);
-        state.setRequestConsumer(this.requestConsumer);
-        state.setRequestHandler(this.requestHandler);
-        this.connContext.setAttribute(HttpAsyncService.HTTP_EXCHANGE_STATE, state);
-        Mockito.when(this.decoder.isCompleted()).thenReturn(Boolean.TRUE);
-        Mockito.when(this.requestConsumer.getException()).thenReturn(null);
-        final Object data = new Object();
-        Mockito.when(this.requestConsumer.getResult()).thenReturn(data);
-        Mockito.doThrow(new UnsupportedHttpVersionException()).when(
-                this.requestHandler).handle(
-                        Mockito.eq(data),
-                        Mockito.any(HttpAsyncExchange.class),
-                        Mockito.eq(exchangeContext));
-
-        this.protocolHandler.inputReady(conn, this.decoder);
-
-        Assert.assertEquals(MessageState.COMPLETED, state.getRequestState());
-        Assert.assertEquals(MessageState.INIT, state.getResponseState());
-        Assert.assertNotNull(state.getResponseProducer());
-
-        Mockito.verify(this.requestConsumer).consumeContent(this.decoder, this.conn);
-        Mockito.verify(this.requestConsumer).requestCompleted(exchangeContext);
-        Mockito.verify(this.conn).requestOutput();
-    }
-
-    @Test
     public void testBasicResponse() throws Exception {
         final State state = new HttpAsyncService.State();
         final HttpContext exchangeContext = state.getContext();



Mime
View raw message