Repository: square/retrofit
Issue: MockRestAdapter ignores ErrorHandler configuration

Steps to reproduce:
1. Create  a `RestAdapter` with an `ErrorHandler` set.
2. Create a mock of your adapter using `MockRestAdapter mockRestAdapter = MockRestAdapter.from(restAdapter);`.
3. Create a concrete instance of you API interface using the mock adapter.
4. Call a method which return an error.
5. Notice that the `ErrorHandler` is not called.


Issue comments:
Yep. Will fix.


Pull request review comments:

Patch that resolved the issue:
diff --git a/retrofit-mock/src/main/java/retrofit/MockRestAdapter.java b/retrofit-mock/src/main/java/retrofit/MockRestAdapter.java
index d0db49bf98..b0bb6d8834 100644
--- a/retrofit-mock/src/main/java/retrofit/MockRestAdapter.java
+++ b/retrofit-mock/src/main/java/retrofit/MockRestAdapter.java
@@ -17,6 +17,7 @@
 import rx.schedulers.Schedulers;
 
 import static retrofit.RestAdapter.LogLevel;
+import static retrofit.RetrofitError.unexpectedError;
 
 /**
  * Wraps mocks implementations of API interfaces so that they exhibit the delay and error
@@ -239,7 +240,16 @@ public MockHandler(Object mockService, Map<Method, RestMethodInfo> methodInfoCac
       final RestMethodInfo methodInfo = RestAdapter.getMethodInfo(methodInfoCache, method);
 
       if (methodInfo.isSynchronous) {
-        return invokeSync(methodInfo, restAdapter.requestInterceptor, args);
+        try {
+          return invokeSync(methodInfo, restAdapter.requestInterceptor, args);
+        } catch (RetrofitError error) {
+          Throwable newError = restAdapter.errorHandler.handleError(error);
+          if (newError == null) {
+            throw new IllegalStateException("Error handler returned null for wrapped exception.",
+                error);
+          }
+          throw newError;
+        }
       }
 
       if (restAdapter.httpExecutor == null || restAdapter.callbackExecutor == null) {
@@ -367,13 +377,16 @@ private void invokeAsync(RestMethodInfo methodInfo, RequestInterceptor intercept
 
       if (calculateIsFailure()) {
         sleep(calculateDelayForError());
-        final IOException exception = new IOException("Mock network error!");
+        IOException exception = new IOException("Mock network error!");
         if (restAdapter.logLevel.log()) {
           restAdapter.logException(exception, url);
         }
+        RetrofitError error = RetrofitError.networkError(url, exception);
+        Throwable cause = restAdapter.errorHandler.handleError(error);
+        final RetrofitError e = cause == error ? error : unexpectedError(error.getUrl(), cause);
         restAdapter.callbackExecutor.execute(new Runnable() {
           @Override public void run() {
-            realCallback.failure(RetrofitError.networkError(url, exception));
+            realCallback.failure(e);
           }
         });
         return;
@@ -416,10 +429,12 @@ private void invokeAsync(RestMethodInfo methodInfo, RequestInterceptor intercept
           }
         }
 
-        final RetrofitError error = new MockHttpRetrofitError(url, response, httpEx.responseBody);
+        RetrofitError error = new MockHttpRetrofitError(url, response, httpEx.responseBody);
+        Throwable cause = restAdapter.errorHandler.handleError(error);
+        final RetrofitError e = cause == error ? error : unexpectedError(error.getUrl(), cause);
         restAdapter.callbackExecutor.execute(new Runnable() {
           @Override public void run() {
-            realCallback.failure(error);
+            realCallback.failure(e);
           }
         });
       }
@@ -511,9 +526,11 @@ private static long uptimeMillis() {
   /** Indirection to avoid VerifyError if RxJava isn't present. */
   private static class MockRxSupport {
     private final Scheduler scheduler;
+    private final ErrorHandler errorHandler;
 
     MockRxSupport(RestAdapter restAdapter) {
       scheduler = Schedulers.executor(restAdapter.httpExecutor);
+      errorHandler = restAdapter.errorHandler;
     }
 
     Observable createMockObservable(final MockHandler mockHandler, final RestMethodInfo methodInfo,
@@ -525,8 +542,10 @@ Observable createMockObservable(final MockHandler mockHandler, final RestMethodI
                 (Observable) mockHandler.invokeSync(methodInfo, interceptor, args);
             //noinspection unchecked
             observable.subscribe(subscriber);
-          } catch (Throwable throwable) {
-            Observable.error(throwable).subscribe(subscriber);
+          } catch (RetrofitError e) {
+            subscriber.onError(errorHandler.handleError(e));
+          } catch (Throwable e) {
+            subscriber.onError(e);
           }
         }
       }).subscribeOn(scheduler);


Test patch:
diff --git a/retrofit-mock/src/test/java/retrofit/MockRestAdapterTest.java b/retrofit-mock/src/test/java/retrofit/MockRestAdapterTest.java
index a2f53e353c..311680f099 100644
--- a/retrofit-mock/src/test/java/retrofit/MockRestAdapterTest.java
+++ b/retrofit-mock/src/test/java/retrofit/MockRestAdapterTest.java
@@ -2,6 +2,7 @@
 package retrofit;
 
 import java.io.IOException;
+import java.util.concurrent.CountDownLatch;
 import java.util.concurrent.Executor;
 import java.util.concurrent.TimeUnit;
 import java.util.concurrent.atomic.AtomicBoolean;
@@ -16,6 +17,7 @@
 import rx.Observable;
 import rx.functions.Action1;
 
+import static java.util.concurrent.TimeUnit.SECONDS;
 import static org.assertj.core.api.Assertions.assertThat;
 import static org.junit.Assert.fail;
 import static org.mockito.Matchers.any;
@@ -46,6 +48,7 @@ interface ObservableExample {
   private Executor callbackExecutor;
   private MockRestAdapter mockRestAdapter;
   private ValueChangeListener valueChangeListener;
+  private Throwable nextError;
 
   @Before public void setUp() throws IOException {
     Client client = mock(Client.class);
@@ -59,6 +62,16 @@ interface ObservableExample {
         .setExecutors(httpExecutor, callbackExecutor)
         .setEndpoint("http://example.com")
         .setLogLevel(RestAdapter.LogLevel.NONE)
+        .setErrorHandler(new ErrorHandler() {
+          @Override public Throwable handleError(RetrofitError cause) {
+            if (nextError != null) {
+              Throwable error = nextError;
+              nextError = null;
+              return error;
+            }
+            return cause;
+          }
+        })
         .build();
 
     valueChangeListener = mock(ValueChangeListener.class);
@@ -476,4 +489,86 @@ class MockAsyncExample implements AsyncExample {
           "Calling failure directly is not supported. Throw MockHttpException instead.");
     }
   }
+
+  @Test public void syncErrorUsesErrorHandler() {
+    mockRestAdapter.setDelay(100);
+    mockRestAdapter.setVariancePercentage(0);
+    mockRestAdapter.setErrorPercentage(0);
+
+    class MockSyncExample implements SyncExample {
+      @Override public Object doStuff() {
+        throw MockHttpException.newNotFound(new Object());
+      }
+    }
+
+    SyncExample mockService = mockRestAdapter.create(SyncExample.class, new MockSyncExample());
+    nextError = new IllegalArgumentException("Test");
+
+    try {
+      mockService.doStuff();
+      fail();
+    } catch (IllegalArgumentException e) {
+      assertThat(e).hasMessage("Test");
+    }
+  }
+
+  @Test public void asyncErrorUsesErrorHandler() throws InterruptedException {
+    mockRestAdapter.setDelay(100);
+    mockRestAdapter.setVariancePercentage(0);
+    mockRestAdapter.setErrorPercentage(0);
+
+    class MockAsyncExample implements AsyncExample {
+      @Override public void doStuff(Callback<Object> cb) {
+        throw MockHttpException.newNotFound(new Object());
+      }
+    }
+
+    AsyncExample mockService = mockRestAdapter.create(AsyncExample.class, new MockAsyncExample());
+    nextError = new IllegalArgumentException("Test");
+
+    final CountDownLatch latch = new CountDownLatch(1);
+    mockService.doStuff(new Callback<Object>() {
+      @Override public void success(Object o, Response response) {
+        throw new AssertionError();
+      }
+
+      @Override public void failure(RetrofitError error) {
+        assertThat(error.getCause()).hasMessage("Test");
+        latch.countDown();
+      }
+    });
+    latch.await(5, SECONDS);
+  }
+
+  @Test public void observableErrorUsesErrorHandler() throws InterruptedException {
+    mockRestAdapter.setDelay(100);
+    mockRestAdapter.setVariancePercentage(0);
+    mockRestAdapter.setErrorPercentage(0);
+
+    class MockObservableExample implements ObservableExample {
+      @Override public Observable<Object> doStuff() {
+        throw MockHttpException.newNotFound(new Object());
+      }
+    }
+
+    ObservableExample mockService =
+        mockRestAdapter.create(ObservableExample.class, new MockObservableExample());
+    nextError = new IllegalArgumentException("Test");
+
+    final CountDownLatch latch = new CountDownLatch(1);
+    final AtomicBoolean called = new AtomicBoolean();
+    mockService.doStuff().subscribe(new Action1<Object>() {
+      @Override public void call(Object o) {
+        throw new AssertionError();
+      }
+    }, new Action1<Throwable>() {
+      @Override public void call(Throwable error) {
+        assertThat(error).hasMessage("Test");
+        called.set(true);
+        latch.countDown();
+      }
+    });
+    latch.await(5, SECONDS);
+    assertThat(called.get()).isTrue();
+  }
 }