From 1ab086360755fcb13356617fe6d738171889d810 Mon Sep 17 00:00:00 2001
From: jwilson <jwilson@squareup.com>
Date: Sun, 15 Mar 2015 13:45:22 -0400
Subject: [PATCH] Add a test for interceptors throwing unchecked exceptions.

Our behavior here is not good. Right now it looks like we're leaking
resources; nothing is releasing the socket in a 'finally' clause when
interceptors crash. We're also not notifying the callback that the
call has failed; that's left hanging.

That said, we don't expect applications to recover from these
exceptions.

Closes https://github.com/square/okhttp/issues/1482
---
 .../com/squareup/okhttp/InterceptorTest.java  | 97 +++++++++++++++++++
 1 file changed, 97 insertions(+)

diff --git a/okhttp-tests/src/test/java/com/squareup/okhttp/InterceptorTest.java b/okhttp-tests/src/test/java/com/squareup/okhttp/InterceptorTest.java
index d186f4af2aac..8cd8d97bdb44 100644
--- a/okhttp-tests/src/test/java/com/squareup/okhttp/InterceptorTest.java
+++ b/okhttp-tests/src/test/java/com/squareup/okhttp/InterceptorTest.java
@@ -23,6 +23,11 @@
 import java.util.Arrays;
 import java.util.List;
 import java.util.Locale;
+import java.util.concurrent.BlockingQueue;
+import java.util.concurrent.LinkedBlockingQueue;
+import java.util.concurrent.SynchronousQueue;
+import java.util.concurrent.ThreadPoolExecutor;
+import java.util.concurrent.TimeUnit;
 import okio.Buffer;
 import okio.BufferedSink;
 import okio.ForwardingSink;
@@ -430,6 +435,73 @@ private void asyncInterceptors(List<Interceptor> interceptors) throws Exception
     callbackB.await(requestB.url()).assertBody("b");
   }
 
+  @Test public void applicationkInterceptorThrowsRuntimeExceptionSynchronous() throws Exception {
+    interceptorThrowsRuntimeExceptionSynchronous(client.interceptors());
+  }
+
+  @Test public void networkInterceptorThrowsRuntimeExceptionSynchronous() throws Exception {
+    interceptorThrowsRuntimeExceptionSynchronous(client.networkInterceptors());
+  }
+
+  /**
+   * When an interceptor throws an unexpected exception, synchronous callers can catch it and deal
+   * with it.
+   *
+   * TODO(jwilson): test that resources are not leaked when this happens.
+   */
+  private void interceptorThrowsRuntimeExceptionSynchronous(
+      List<Interceptor> interceptors) throws Exception {
+    interceptors.add(new Interceptor() {
+      @Override public Response intercept(Chain chain) throws IOException {
+        throw new RuntimeException("boom!");
+      }
+    });
+
+    Request request = new Request.Builder()
+        .url(server.getUrl("/"))
+        .build();
+
+    try {
+      client.newCall(request).execute();
+      fail();
+    } catch (RuntimeException expected) {
+      assertEquals("boom!", expected.getMessage());
+    }
+  }
+
+  @Test public void applicationInterceptorThrowsRuntimeExceptionAsynchronous() throws Exception {
+    interceptorThrowsRuntimeExceptionAsynchronous(client.interceptors());
+  }
+
+  @Test public void networkInterceptorThrowsRuntimeExceptionAsynchronous() throws Exception {
+    interceptorThrowsRuntimeExceptionAsynchronous(client.networkInterceptors());
+  }
+
+  /**
+   * When an interceptor throws an unexpected exception, asynchronous callers are left hanging. The
+   * exception goes to the uncaught exception handler.
+   *
+   * TODO(jwilson): test that resources are not leaked when this happens.
+   */
+  private void interceptorThrowsRuntimeExceptionAsynchronous(
+        List<Interceptor> interceptors) throws Exception {
+    interceptors.add(new Interceptor() {
+      @Override public Response intercept(Chain chain) throws IOException {
+        throw new RuntimeException("boom!");
+      }
+    });
+
+    ExceptionCatchingExecutor executor = new ExceptionCatchingExecutor();
+    client.setDispatcher(new Dispatcher(executor));
+
+    Request request = new Request.Builder()
+        .url(server.getUrl("/"))
+        .build();
+    client.newCall(request).enqueue(callback);
+
+    assertEquals("boom!", executor.takeException().getMessage());
+  }
+
   private RequestBody uppercase(final RequestBody original) {
     return new RequestBody() {
       @Override public MediaType contentType() {
@@ -480,4 +552,29 @@ private Buffer gzip(String data) throws IOException {
     sink.close();
     return result;
   }
+
+  /** Catches exceptions that are otherwise headed for the uncaught exception handler. */
+  private static class ExceptionCatchingExecutor extends ThreadPoolExecutor {
+    private final BlockingQueue<Exception> exceptions = new LinkedBlockingQueue<>();
+
+    public ExceptionCatchingExecutor() {
+      super(1, 1, 0, TimeUnit.SECONDS, new SynchronousQueue<Runnable>());
+    }
+
+    @Override public void execute(final Runnable runnable) {
+      super.execute(new Runnable() {
+        @Override public void run() {
+          try {
+            runnable.run();
+          } catch (Exception e) {
+            exceptions.add(e);
+          }
+        }
+      });
+    }
+
+    public Exception takeException() throws InterruptedException {
+      return exceptions.take();
+    }
+  }
 }