execute shouldn't throw Exceptions, close AsyncHttpClient#710

Author: Stephane Landelle

api/src/main/java/org/asynchttpclient/AsyncHttpClient.java

@@ -17,7 +17,6 @@
 package org.asynchttpclient;
 
 import java.io.Closeable;
-import java.io.IOException;
 import java.util.concurrent.Future;
 
 /**
@@ -251,16 +250,14 @@ public interface AsyncHttpClient extends Closeable {
      * @param handler an instance of {@link AsyncHandler}
      * @param <T>     Type of the value that will be returned by the associated {@link java.util.concurrent.Future}
      * @return a {@link Future} of type T
-     * @throws IOException
      */
-    <T> ListenableFuture<T> executeRequest(Request request, AsyncHandler<T> handler) throws IOException;
+    <T> ListenableFuture<T> executeRequest(Request request, AsyncHandler<T> handler);
 
     /**
      * Execute an HTTP request.
      *
      * @param request {@link Request}
      * @return a {@link Future} of type Response
-     * @throws IOException
      */
-    ListenableFuture<Response> executeRequest(Request request) throws IOException;
+    ListenableFuture<Response> executeRequest(Request request);
 }

api/src/main/java/org/asynchttpclient/AsyncHttpProvider.java

@@ -16,7 +16,6 @@
 package org.asynchttpclient;
 
 import java.io.Closeable;
-import java.io.IOException;
 
 /**
  * Interface to be used when implementing custom asynchronous I/O HTTP client.
@@ -28,9 +27,8 @@ public interface AsyncHttpProvider extends Closeable {
      *
      * @param handler an instance of {@link AsyncHandler}
      * @return a {@link ListenableFuture} of Type T.
-     * @throws IOException
      */
-    <T> ListenableFuture<T> execute(Request request, AsyncHandler<T> handler) throws IOException;
+    <T> ListenableFuture<T> execute(Request request, AsyncHandler<T> handler);
 
     /**
      * Close the current underlying TCP/HTTP connection.

api/src/main/java/org/asynchttpclient/BoundRequestBuilder.java

@@ -15,7 +15,6 @@
 import org.asynchttpclient.cookie.Cookie;
 import org.asynchttpclient.multipart.Part;
 
-import java.io.IOException;
 import java.io.InputStream;
 import java.util.Collection;
 import java.util.List;
@@ -35,11 +34,11 @@ public BoundRequestBuilder(AsyncHttpClient client, Request prototype) {
         this.client = client;
     }
 
-    public <T> ListenableFuture<T> execute(AsyncHandler<T> handler) throws IOException {
+    public <T> ListenableFuture<T> execute(AsyncHandler<T> handler) {
         return client.executeRequest(build(), handler);
     }
 
-    public ListenableFuture<Response> execute() throws IOException {
+    public ListenableFuture<Response> execute() {
         return client.executeRequest(build(), new AsyncCompletionHandlerBase());
     }
 

api/src/main/java/org/asynchttpclient/DefaultAsyncHttpClient.java

@@ -16,19 +16,18 @@
  */
 package org.asynchttpclient;
 
+import java.lang.reflect.InvocationTargetException;
+import java.util.concurrent.ExecutorService;
+import java.util.concurrent.Executors;
+import java.util.concurrent.atomic.AtomicBoolean;
+
 import org.asynchttpclient.filter.FilterContext;
 import org.asynchttpclient.filter.FilterException;
 import org.asynchttpclient.filter.RequestFilter;
 import org.asynchttpclient.resumable.ResumableAsyncHandler;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
-import java.io.IOException;
-import java.lang.reflect.InvocationTargetException;
-import java.util.concurrent.ExecutorService;
-import java.util.concurrent.Executors;
-import java.util.concurrent.atomic.AtomicBoolean;
-
 public class DefaultAsyncHttpClient implements AsyncHttpClient {
 
     /**
@@ -225,21 +224,26 @@ public BoundRequestBuilder prepareRequest(Request request) {
     }
 
     @Override
-    public <T> ListenableFuture<T> executeRequest(Request request, AsyncHandler<T> handler) throws IOException {
+    public <T> ListenableFuture<T> executeRequest(Request request, AsyncHandler<T> handler) {
 
         if (config.getRequestFilters().isEmpty()) {
             return httpProvider.execute(request, handler);
 
         } else {
             FilterContext<T> fc = new FilterContext.FilterContextBuilder<T>().asyncHandler(handler).request(request).build();
-            fc = preProcessRequest(fc);
+            try {
+                fc = preProcessRequest(fc);
+            } catch (Exception e) {
+                handler.onThrowable(e);
+                return new ListenableFuture.CompletedFailure<T>("preProcessRequest failed", e);
+            }
 
             return httpProvider.execute(fc.getRequest(), fc.getAsyncHandler());
         }
     }
 
     @Override
-    public ListenableFuture<Response> executeRequest(Request request) throws IOException {
+    public ListenableFuture<Response> executeRequest(Request request) {
         return executeRequest(request, new AsyncCompletionHandlerBase());
     }
 
@@ -249,17 +253,11 @@ public ListenableFuture<Response> executeRequest(Request request) throws IOExcep
      * @param fc {@link FilterContext}
      * @return {@link FilterContext}
      */
-    private <T> FilterContext<T> preProcessRequest(FilterContext<T> fc) throws IOException {
+    private <T> FilterContext<T> preProcessRequest(FilterContext<T> fc) throws FilterException {
         for (RequestFilter asyncFilter : config.getRequestFilters()) {
-            try {
-                fc = asyncFilter.filter(fc);
-                if (fc == null) {
-                    throw new NullPointerException("FilterContext is null");
-                }
-            } catch (FilterException e) {
-                IOException ex = new IOException();
-                ex.initCause(e);
-                throw ex;
+            fc = asyncFilter.filter(fc);
+            if (fc == null) {
+                throw new NullPointerException("FilterContext is null");
             }
         }
 

api/src/main/java/org/asynchttpclient/ListenableFuture.java

@@ -30,8 +30,11 @@
  */
 package org.asynchttpclient;
 
+import java.util.concurrent.ExecutionException;
 import java.util.concurrent.Executor;
 import java.util.concurrent.Future;
+import java.util.concurrent.TimeUnit;
+import java.util.concurrent.TimeoutException;
 
 /**
  * Extended {@link Future}
@@ -79,4 +82,60 @@ public interface ListenableFuture<V> extends Future<V> {
      *                              immediately but the executor rejected it.
      */
     ListenableFuture<V> addListener(Runnable listener, Executor exec);
+
+    public class CompletedFailure<T> implements ListenableFuture<T>{
+
+        private final ExecutionException e;
+
+        public CompletedFailure(Throwable t) {
+            e = new ExecutionException(t);
+        }
+
+        public CompletedFailure(String message, Throwable t) {
+            e = new ExecutionException(message, t);
+        }
+
+        @Override
+        public boolean cancel(boolean mayInterruptIfRunning) {
+            return true;
+        }
+
+        @Override
+        public boolean isCancelled() {
+            return false;
+        }
+
+        @Override
+        public boolean isDone() {
+            return true;
+        }
+
+        @Override
+        public T get() throws InterruptedException, ExecutionException {
+            throw e;
+        }
+
+        @Override
+        public T get(long timeout, TimeUnit unit) throws InterruptedException, ExecutionException, TimeoutException {
+            throw e;
+        }
+
+        @Override
+        public void done() {
+        }
+
+        @Override
+        public void abort(Throwable t) {
+        }
+
+        @Override
+        public void touch() {
+        }
+
+        @Override
+        public ListenableFuture<T> addListener(Runnable listener, Executor exec) {
+            exec.execute(listener);
+            return this;
+        }
+    }
 }

api/src/test/java/org/asynchttpclient/async/FilterTest.java

@@ -12,9 +12,19 @@
  */
 package org.asynchttpclient.async;
 
-import static org.testng.Assert.assertEquals;
-import static org.testng.Assert.assertNotNull;
-import static org.testng.Assert.fail;
+import static org.testng.Assert.*;
+
+import java.io.IOException;
+import java.util.ArrayList;
+import java.util.Enumeration;
+import java.util.List;
+import java.util.concurrent.ExecutionException;
+import java.util.concurrent.Future;
+import java.util.concurrent.atomic.AtomicBoolean;
+
+import javax.servlet.ServletException;
+import javax.servlet.http.HttpServletRequest;
+import javax.servlet.http.HttpServletResponse;
 
 import org.asynchttpclient.AsyncHttpClient;
 import org.asynchttpclient.AsyncHttpClientConfig;
@@ -28,17 +38,6 @@
 import org.eclipse.jetty.server.handler.AbstractHandler;
 import org.testng.annotations.Test;
 
-import javax.servlet.ServletException;
-import javax.servlet.http.HttpServletRequest;
-import javax.servlet.http.HttpServletResponse;
-
-import java.io.IOException;
-import java.util.ArrayList;
-import java.util.Enumeration;
-import java.util.List;
-import java.util.concurrent.Future;
-import java.util.concurrent.atomic.AtomicBoolean;
-
 public abstract class FilterTest extends AbstractBasicTest {
 
     private static class BasicHandler extends AbstractHandler {
@@ -111,11 +110,10 @@ public void maxConnectionsText() throws Exception {
         AsyncHttpClient c = getAsyncHttpClient(b.build());
 
         try {
-            /* Response response = */c.preparePost(getTargetUrl()).execute().get();
+            c.preparePost(getTargetUrl()).execute().get();
             fail("Should have timed out");
-        } catch (IOException ex) {
-            assertNotNull(ex);
-            assertEquals(ex.getCause().getClass(), FilterException.class);
+        } catch (ExecutionException ex) {
+            assertTrue(ex.getCause() instanceof FilterException);
         } finally {
             c.close();
         }
@@ -126,7 +124,7 @@ public void basicResponseFilterTest() throws Exception {
         AsyncHttpClientConfig.Builder b = new AsyncHttpClientConfig.Builder();
         b.addResponseFilter(new ResponseFilter() {
 
-            // @Override
+            @Override
             public <T> FilterContext<T> filter(FilterContext<T> ctx) throws FilterException {
                 return ctx;
             }
@@ -139,8 +137,6 @@ public <T> FilterContext<T> filter(FilterContext<T> ctx) throws FilterException
 
             assertNotNull(response);
             assertEquals(response.getStatusCode(), 200);
-        } catch (IOException ex) {
-            fail("Should have timed out");
         } finally {
             c.close();
         }
@@ -171,8 +167,6 @@ public <T> FilterContext<T> filter(FilterContext<T> ctx) throws FilterException
             assertNotNull(response);
             assertEquals(response.getStatusCode(), 200);
             assertEquals(response.getHeader("X-Replay"), "true");
-        } catch (IOException ex) {
-            fail("Should have timed out");
         } finally {
             c.close();
         }
@@ -203,8 +197,6 @@ public <T> FilterContext<T> filter(FilterContext<T> ctx) throws FilterException
             assertNotNull(response);
             assertEquals(response.getStatusCode(), 200);
             assertEquals(response.getHeader("X-Replay"), "true");
-        } catch (IOException ex) {
-            fail("Should have timed out");
         } finally {
             c.close();
         }
@@ -236,8 +228,6 @@ public <T> FilterContext<T> filter(FilterContext<T> ctx) throws FilterException
             assertNotNull(response);
             assertEquals(response.getStatusCode(), 200);
             assertEquals(response.getHeader("Ping"), "Pong");
-        } catch (IOException ex) {
-            fail("Should have timed out");
         } finally {
             c.close();
         }