Repository: square/retrofit
Issue: Feature request: Add getSuccessType method to RetrofitError

For most cases, I'm using a generic implementation of `Callback`. It would be helpful to me if `successType` were exposed. Right now my implementation looks like this:

``` java
public class ResponseCallback<T extends ResponseBase> implements Callback<T> {
    public void success(T t, Response response) {
        // Wrap t if desired.
        // Post event (t or wrapped t).
    }

    public void failure(RetrofitError error) {
        // Use reflection to get successType.
        // Create new instance of successType - t.
        t.addError(error);
        // Wrap t if desired.
        // Post event (t or wrapped t).
    }
}

public abstract class ResponseBase {
    private transient List<RetrofitError> mErrors;

    public ResponseBase() {
    }

    public boolean successful() {
        return mErrors == null || mErrors.isEmpty();
    }

    public List<RetrofitError> getErrors() {
        return mErrors;
    }

    public void addError(RetrofitError error) {
        if (mErrors == null) {
            mErrors = new ArrayList<RetrofitError>();
        }
        mErrors.add(error);
    }
}
```

It would be nice to avoid the reflection in `failure()` by exposing `successType`. I realize that `ResponseCallback` could be constructed with the type, but the API is nicer without it and it's already in `RetrofitError`.

My implementation works as is (until `successType` is renamed), but I hope you'll consider it.


Issue comments:

Pull request review comments:
This is a bug that testing for this feature exposed.

?


Patch that resolved the issue:
diff --git a/retrofit-mock/src/main/java/retrofit/MockHttpRetrofitError.java b/retrofit-mock/src/main/java/retrofit/MockHttpRetrofitError.java
index 831f8a5a13..7c2809fd6a 100644
--- a/retrofit-mock/src/main/java/retrofit/MockHttpRetrofitError.java
+++ b/retrofit-mock/src/main/java/retrofit/MockHttpRetrofitError.java
@@ -8,7 +8,7 @@ class MockHttpRetrofitError extends RetrofitError {
   private final Object body;
 
   MockHttpRetrofitError(String message, String url, Response response, Object body) {
-    super(message, url, response, null, null, false, null);
+    super(message, url, response, null, body.getClass(), false, null);
     this.body = body;
   }
 
diff --git a/retrofit/src/main/java/retrofit/RestAdapter.java b/retrofit/src/main/java/retrofit/RestAdapter.java
index bdd1412f2c..0044157872 100644
--- a/retrofit/src/main/java/retrofit/RestAdapter.java
+++ b/retrofit/src/main/java/retrofit/RestAdapter.java
@@ -349,6 +349,9 @@ private Object invokeRequest(RequestInterceptor requestInterceptor, RestMethodIn
 
           TypedInput body = response.getBody();
           if (body == null) {
+            if (methodInfo.isSynchronous) {
+              return null;
+            }
             return new ResponseWrapper(response, null);
           }
 
diff --git a/retrofit/src/main/java/retrofit/RetrofitError.java b/retrofit/src/main/java/retrofit/RetrofitError.java
index 909c2f9fc5..7b923e8f35 100644
--- a/retrofit/src/main/java/retrofit/RetrofitError.java
+++ b/retrofit/src/main/java/retrofit/RetrofitError.java
@@ -83,6 +83,14 @@ public Object getBody() {
     return getBodyAs(successType);
   }
 
+  /**
+   * The type declared by either the interface method return type or the generic type of the
+   * supplied {@link Callback} parameter.
+   */
+  public Type getSuccessType() {
+    return successType;
+  }
+
   /**
    * HTTP response body converted to specified {@code type}. {@code null} if there is no response.
    */


Test patch:
diff --git a/retrofit-mock/src/test/java/retrofit/MockRestAdapterTest.java b/retrofit-mock/src/test/java/retrofit/MockRestAdapterTest.java
index b0fb6923a4..2eb1225bb3 100644
--- a/retrofit-mock/src/test/java/retrofit/MockRestAdapterTest.java
+++ b/retrofit-mock/src/test/java/retrofit/MockRestAdapterTest.java
@@ -37,7 +37,7 @@ interface SyncExample {
   }
 
   interface AsyncExample {
-    @GET("/") void doStuff(Callback<Object> cb);
+    @GET("/") void doStuff(Callback<String> cb);
   }
 
   interface AsyncCallbackSubtypeExample {
@@ -47,7 +47,7 @@ abstract class Foo implements Callback<String> {}
   }
 
   interface ObservableExample {
-    @GET("/") Observable<Object> doStuff();
+    @GET("/") Observable<String> doStuff();
   }
 
   private Executor httpExecutor;
@@ -242,7 +242,7 @@ class MockSyncExample implements SyncExample {
     mockRestAdapter.setErrorPercentage(100);
 
     class MockAsyncExample implements AsyncExample {
-      @Override public void doStuff(Callback<Object> cb) {
+      @Override public void doStuff(Callback<String> cb) {
         throw new AssertionError();
       }
     }
@@ -250,8 +250,8 @@ class MockAsyncExample implements AsyncExample {
     AsyncExample mockService = mockRestAdapter.create(AsyncExample.class, new MockAsyncExample());
 
     final AtomicReference<RetrofitError> errorRef = new AtomicReference<RetrofitError>();
-    mockService.doStuff(new Callback<Object>() {
-      @Override public void success(Object o, Response response) {
+    mockService.doStuff(new Callback<String>() {
+      @Override public void success(String o, Response response) {
         throw new AssertionError();
       }
 
@@ -298,9 +298,11 @@ class MockSyncExample implements SyncExample {
     mockRestAdapter.setVariancePercentage(0);
     mockRestAdapter.setErrorPercentage(0);
 
-    final Object expected = new Object();
+    @SuppressWarnings("RedundantStringConstructorCall") // Allocated on-heap.
+    final String expected = new String("Hi");
+
     class MockAsyncExample implements AsyncExample {
-      @Override public void doStuff(Callback<Object> cb) {
+      @Override public void doStuff(Callback<String> cb) {
         cb.success(expected, null);
       }
     }
@@ -310,8 +312,8 @@ class MockAsyncExample implements AsyncExample {
     final long startNanos = System.nanoTime();
     final AtomicLong tookMs = new AtomicLong();
     final AtomicReference<Object> actual = new AtomicReference<Object>();
-    mockService.doStuff(new Callback<Object>() {
-      @Override public void success(Object result, Response response) {
+    mockService.doStuff(new Callback<String>() {
+      @Override public void success(String result, Response response) {
         tookMs.set(TimeUnit.NANOSECONDS.toMillis(System.nanoTime() - startNanos));
         actual.set(result);
       }
@@ -333,9 +335,11 @@ class MockAsyncExample implements AsyncExample {
     mockRestAdapter.setVariancePercentage(0);
     mockRestAdapter.setErrorPercentage(0);
 
-    final Object expected = new Object();
+    @SuppressWarnings("RedundantStringConstructorCall") // Allocated on-heap.
+    final String expected = new String("Hello");
+
     class MockObservableExample implements ObservableExample {
-      @Override public Observable<Object> doStuff() {
+      @Override public Observable<String> doStuff() {
         return Observable.from(expected);
       }
     }
@@ -373,9 +377,11 @@ class MockObservableExample implements ObservableExample {
     mockRestAdapter.setVariancePercentage(0);
     mockRestAdapter.setErrorPercentage(0);
 
-    final Object expected = new Object();
+    @SuppressWarnings("RedundantStringConstructorCall") // Allocated on-heap.
+    final String expected = new String("Hello");
+
     class MockSyncExample implements SyncExample {
-      @Override public Object doStuff() {
+      @Override public String doStuff() {
         throw new MockHttpException(404, "Not Found", expected);
       }
     }
@@ -393,6 +399,7 @@ class MockSyncExample implements SyncExample {
       assertThat(e.getResponse().getStatus()).isEqualTo(404);
       assertThat(e.getResponse().getReason()).isEqualTo("Not Found");
       assertThat(e.getBody()).isSameAs(expected);
+      assertThat(e.getSuccessType()).isEqualTo(String.class);
     }
   }
 
@@ -401,9 +408,11 @@ class MockSyncExample implements SyncExample {
     mockRestAdapter.setVariancePercentage(0);
     mockRestAdapter.setErrorPercentage(0);
 
-    final Object expected = new Object();
+    @SuppressWarnings("RedundantStringConstructorCall") // Allocated on-heap.
+    final String expected = new String("Greetings");
+
     class MockAsyncExample implements AsyncExample {
-      @Override public void doStuff(Callback<Object> cb) {
+      @Override public void doStuff(Callback<String> cb) {
         throw new MockHttpException(404, "Not Found", expected);
       }
     }
@@ -413,8 +422,8 @@ class MockAsyncExample implements AsyncExample {
     final long startNanos = System.nanoTime();
     final AtomicLong tookMs = new AtomicLong();
     final AtomicReference<RetrofitError> errorRef = new AtomicReference<RetrofitError>();
-    mockService.doStuff(new Callback<Object>() {
-      @Override public void success(Object o, Response response) {
+    mockService.doStuff(new Callback<String>() {
+      @Override public void success(String o, Response response) {
         throw new AssertionError();
       }
 
@@ -433,6 +442,7 @@ class MockAsyncExample implements AsyncExample {
     assertThat(error.getResponse().getStatus()).isEqualTo(404);
     assertThat(error.getResponse().getReason()).isEqualTo("Not Found");
     assertThat(error.getBody()).isSameAs(expected);
+    assertThat(error.getSuccessType()).isEqualTo(String.class);
   }
 
   @Test public void observableHttpExceptionBecomesError() {
@@ -440,9 +450,11 @@ class MockAsyncExample implements AsyncExample {
     mockRestAdapter.setVariancePercentage(0);
     mockRestAdapter.setErrorPercentage(0);
 
-    final Object expected = new Object();
+    @SuppressWarnings("RedundantStringConstructorCall") // Allocated on-heap.
+    final String expected = new String("Hi");
+
     class MockObservableExample implements ObservableExample {
-      @Override public Observable<Object> doStuff() {
+      @Override public Observable<String> doStuff() {
         throw new MockHttpException(404, "Not Found", expected);
       }
     }
@@ -474,6 +486,7 @@ class MockObservableExample implements ObservableExample {
     assertThat(error.getResponse().getStatus()).isEqualTo(404);
     assertThat(error.getResponse().getReason()).isEqualTo("Not Found");
     assertThat(error.getBody()).isSameAs(expected);
+    assertThat(error.getSuccessType()).isEqualTo(String.class);
   }
 
   @Test public void syncErrorUsesErrorHandler() {
@@ -504,7 +517,7 @@ class MockSyncExample implements SyncExample {
     mockRestAdapter.setErrorPercentage(0);
 
     class MockAsyncExample implements AsyncExample {
-      @Override public void doStuff(Callback<Object> cb) {
+      @Override public void doStuff(Callback<String> cb) {
         throw MockHttpException.newNotFound(new Object());
       }
     }
@@ -513,8 +526,8 @@ class MockAsyncExample implements AsyncExample {
     nextError = new IllegalArgumentException("Test");
 
     final CountDownLatch latch = new CountDownLatch(1);
-    mockService.doStuff(new Callback<Object>() {
-      @Override public void success(Object o, Response response) {
+    mockService.doStuff(new Callback<String>() {
+      @Override public void success(String o, Response response) {
         throw new AssertionError();
       }
 
@@ -532,7 +545,7 @@ class MockAsyncExample implements AsyncExample {
     mockRestAdapter.setErrorPercentage(0);
 
     class MockObservableExample implements ObservableExample {
-      @Override public Observable<Object> doStuff() {
+      @Override public Observable<String> doStuff() {
         throw MockHttpException.newNotFound(new Object());
       }
     }
diff --git a/retrofit/src/test/java/retrofit/RestAdapterTest.java b/retrofit/src/test/java/retrofit/RestAdapterTest.java
index ffcfb3bd65..4dda2761b8 100644
--- a/retrofit/src/test/java/retrofit/RestAdapterTest.java
+++ b/retrofit/src/test/java/retrofit/RestAdapterTest.java
@@ -34,9 +34,7 @@
 import static org.assertj.core.api.Assertions.fail;
 import static org.mockito.Matchers.any;
 import static org.mockito.Matchers.anyInt;
-import static org.mockito.Matchers.anyString;
 import static org.mockito.Matchers.eq;
-import static org.mockito.Matchers.isA;
 import static org.mockito.Matchers.same;
 import static org.mockito.Mockito.atLeastOnce;
 import static org.mockito.Mockito.doReturn;
@@ -68,16 +66,16 @@ public class RestAdapterTest {
     }
 
     @Override public InputStream in() throws IOException {
-      return new ByteArrayInputStream("{}".getBytes("UTF-8"));
+      return new ByteArrayInputStream("Hi".getBytes("UTF-8"));
     }
   };
 
   private interface Example {
     @Headers("Foo: Bar")
-    @GET("/") Object something();
+    @GET("/") String something();
     @Headers("Foo: Bar")
     @POST("/") Object something(@Body TypedOutput body);
-    @GET("/") void something(Callback<Object> callback);
+    @GET("/") void something(Callback<String> callback);
     @GET("/") Response direct();
     @GET("/") void direct(Callback<Response> callback);
     @POST("/") Observable<String> observable(@Body String body);
@@ -127,7 +125,7 @@ private interface InvalidExample extends Example {
     Object data = new Object();
     when(mockProfiler.beforeCall()).thenReturn(data);
     when(mockClient.execute(any(Request.class))) //
-        .thenReturn(new Response("http://example.com/", 200, "OK", NO_HEADERS, null));
+        .thenReturn(new Response("http://example.com/", 200, "OK", NO_HEADERS, new TypedString("Hey")));
 
     example.something();
 
@@ -155,7 +153,7 @@ public void log(String message) {
         .create(Example.class);
 
     when(mockClient.execute(any(Request.class))) //
-        .thenReturn(new Response("http://example.com/", 200, "OK", TWO_HEADERS, new TypedString("{}")));
+        .thenReturn(new Response("http://example.com/", 200, "OK", TWO_HEADERS, new TypedString("Hi")));
 
     example.something();
     assertThat(logMessages).hasSize(2);
@@ -182,7 +180,7 @@ public void log(String message) {
         .create(Example.class);
 
     when(mockClient.execute(any(Request.class))) //
-        .thenReturn(new Response("http://example.com/", 200, "OK", TWO_HEADERS, new TypedString("{}")));
+        .thenReturn(new Response("http://example.com/", 200, "OK", TWO_HEADERS, new TypedString("Hi")));
 
     example.something();
     assertThat(logMessages).hasSize(7);
@@ -302,13 +300,13 @@ public void log(String message) {
     assertThat(logMessages.get(4)).isEqualTo("Content-Type: application/json");
     assertThat(logMessages.get(5)).isEqualTo("Content-Length: 42");
     assertThat(logMessages.get(6)).isEqualTo("");
-    assertThat(logMessages.get(7)).isEqualTo("{}");
+    assertThat(logMessages.get(7)).isEqualTo("Hi");
     assertThat(logMessages.get(8)).isEqualTo("<--- END HTTP (2-byte body)");
   }
 
   @Test public void synchronousDoesNotUseExecutors() throws Exception {
     when(mockClient.execute(any(Request.class))) //
-        .thenReturn(new Response("http://example.com/", 200, "OK", NO_HEADERS, null));
+        .thenReturn(new Response("http://example.com/", 200, "OK", NO_HEADERS, new TypedString("Hi")));
 
     example.something();
 
@@ -317,15 +315,15 @@ public void log(String message) {
   }
 
   @Test public void asynchronousUsesExecutors() throws Exception {
-    Response response = new Response("http://example.com/", 200, "OK", NO_HEADERS, new TypedString("{}"));
+    Response response = new Response("http://example.com/", 200, "OK", NO_HEADERS, new TypedString("Hey"));
     when(mockClient.execute(any(Request.class))).thenReturn(response);
-    Callback<Object> callback = mock(Callback.class);
+    Callback<String> callback = mock(Callback.class);
 
     example.something(callback);
 
     verify(mockRequestExecutor).execute(any(CallbackRunnable.class));
     verify(mockCallbackExecutor).execute(any(Runnable.class));
-    verify(callback).success(anyString(), same(response));
+    verify(callback).success(eq("Hey"), same(response));
   }
 
   @Test public void malformedResponseThrowsConversionException() throws Exception {
@@ -351,6 +349,7 @@ public void log(String message) {
       fail("RetrofitError expected on non-2XX response code.");
     } catch (RetrofitError e) {
       assertThat(e.getResponse().getStatus()).isEqualTo(500);
+      assertThat(e.getSuccessType()).isEqualTo(String.class);
     }
   }
 
@@ -392,7 +391,7 @@ public void log(String message) {
     assertThat(logMessages.get(4)).isEqualTo("Content-Type: application/json");
     assertThat(logMessages.get(5)).isEqualTo("Content-Length: 42");
     assertThat(logMessages.get(6)).isEqualTo("");
-    assertThat(logMessages.get(7)).isEqualTo("{}");
+    assertThat(logMessages.get(7)).isEqualTo("Hi");
     assertThat(logMessages.get(8)).isEqualTo("<--- END HTTP (2-byte body)");
   }
 
@@ -510,7 +509,7 @@ public void log(String message) {
   }
 
   @Test public void getResponseDirectlyAsync() throws Exception {
-    Response response = new Response("http://example.com/", 200, "OK", NO_HEADERS, null);
+    Response response = new Response("http://example.com/", 200, "OK", NO_HEADERS, new TypedString("Hey"));
     when(mockClient.execute(any(Request.class))) //
         .thenReturn(response);
     Callback<Response> callback = mock(Callback.class);
@@ -522,6 +521,43 @@ public void log(String message) {
     verify(callback).success(eq(response), same(response));
   }
 
+  @Test public void getAsync() throws Exception {
+    Response response = new Response("http://example.com/", 200, "OK", NO_HEADERS, new TypedString("Hey"));
+    when(mockClient.execute(any(Request.class))) //
+        .thenReturn(response);
+    Callback<String> callback = mock(Callback.class);
+
+    example.something(callback);
+
+    verify(mockRequestExecutor).execute(any(CallbackRunnable.class));
+    verify(mockCallbackExecutor).execute(any(Runnable.class));
+
+    ArgumentCaptor<String> responseCaptor = ArgumentCaptor.forClass(String.class);
+    verify(callback).success(responseCaptor.capture(), same(response));
+    assertThat(responseCaptor.getValue()).isEqualTo("Hey");
+  }
+
+
+  @Test public void errorAsync() throws Exception {
+    Response response = new Response("http://example.com/", 500, "Broken!", NO_HEADERS, new TypedString("Hey"));
+    when(mockClient.execute(any(Request.class))) //
+        .thenReturn(response);
+    Callback<String> callback = mock(Callback.class);
+
+    example.something(callback);
+
+    verify(mockRequestExecutor).execute(any(CallbackRunnable.class));
+    verify(mockCallbackExecutor).execute(any(Runnable.class));
+
+    ArgumentCaptor<RetrofitError> errorCaptor = ArgumentCaptor.forClass(RetrofitError.class);
+    verify(callback).failure(errorCaptor.capture());
+    RetrofitError error = errorCaptor.getValue();
+    assertThat(error.getResponse().getStatus()).isEqualTo(500);
+    assertThat(error.getResponse().getReason()).isEqualTo("Broken!");
+    assertThat(error.getSuccessType()).isEqualTo(String.class);
+    assertThat(error.getBody()).isEqualTo("Hey");
+  }
+
   @Test public void observableCallsOnNext() throws Exception {
     when(mockClient.execute(any(Request.class))) //
         .thenReturn(new Response("http://example.com/", 200, "OK", NO_HEADERS, new TypedString("hello")));
@@ -537,7 +573,11 @@ public void log(String message) {
     Action1<Throwable> onError = mock(Action1.class);
     example.observable("Howdy").subscribe(onSuccess, onError);
     verifyZeroInteractions(onSuccess);
-    verify(onError).call(isA(RetrofitError.class));
+
+    ArgumentCaptor<RetrofitError> errorCaptor = ArgumentCaptor.forClass(RetrofitError.class);
+    verify(onError).call(errorCaptor.capture());
+    RetrofitError value = errorCaptor.getValue();
+    assertThat(value.getSuccessType()).isEqualTo(String.class);
   }
 
   @Test public void observableHandlesParams() throws Exception {