Repository: square/retrofit
Issue: RetrofitError is a nightmare to work with.

How do you know if it was a conversion error, network error, random error inside Retrofit? Yuck. This API interaction needs rethought.


Issue comments:
Grabbing a response body out of an error is also a pain. Some APIs return error status codes and include an error message body. In this case you need to grab the response body out of `RetrofitError`. There was even a pull request to try and take care of the ugly object casting code necessary to accomplish this: https://github.com/square/retrofit/pull/243

That's what `getBodyAs(Type)` is for. If you want to convert the body to a string then just read the body's input stream to a string.

Discussion at #297.

When I try to make a POST request with Retrofit in Android application, I face this error:

failure : retrofit.RetrofitError: 307 Temporary Redirect

Please help me to resolve this error


Pull request review comments:

Patch that resolved the issue:
diff --git a/retrofit/src/main/java/retrofit/RetrofitConversionError.java b/retrofit/src/main/java/retrofit/RetrofitConversionError.java
new file mode 100644
index 0000000000..97155a6b8e
--- /dev/null
+++ b/retrofit/src/main/java/retrofit/RetrofitConversionError.java
@@ -0,0 +1,13 @@
+package retrofit;
+
+import retrofit.client.Response;
+import retrofit.converter.Converter;
+
+import java.lang.reflect.Type;
+
+public class RetrofitConversionError extends RetrofitResponseError {
+  protected RetrofitConversionError(String url, Response response, Converter converter,
+                                    Type successType, Throwable exception) {
+    super(url, response, converter, successType, exception);
+  }
+}
diff --git a/retrofit/src/main/java/retrofit/RetrofitError.java b/retrofit/src/main/java/retrofit/RetrofitError.java
index 4a8b2cff0d..d867406fdc 100644
--- a/retrofit/src/main/java/retrofit/RetrofitError.java
+++ b/retrofit/src/main/java/retrofit/RetrofitError.java
@@ -15,89 +15,87 @@
  */
 package retrofit;
 
-import java.io.IOException;
-import java.lang.reflect.Type;
 import retrofit.client.Response;
 import retrofit.converter.ConversionException;
 import retrofit.converter.Converter;
-import retrofit.mime.TypedInput;
 
-public class RetrofitError extends RuntimeException {
-  public static RetrofitError networkError(String url, IOException exception) {
-    return new RetrofitError(url, null, null, null, true, exception);
+import java.io.IOException;
+import java.lang.reflect.Type;
+
+public abstract class RetrofitError extends RuntimeException {
+  public static RetrofitNetworkError networkError(String url, IOException exception) {
+    return new RetrofitNetworkError(url, exception);
   }
 
-  public static RetrofitError conversionError(String url, Response response, Converter converter,
-      Type successType, ConversionException exception) {
-    return new RetrofitError(url, response, converter, successType, false, exception);
+  public static RetrofitConversionError conversionError(String url, Response response,
+                                                        Converter converter, Type successType,
+                                                        ConversionException exception) {
+    return new RetrofitConversionError(url, response, converter, successType, exception);
   }
 
-  public static RetrofitError httpError(String url, Response response, Converter converter,
-      Type successType) {
-    return new RetrofitError(url, response, converter, successType, false, null);
+  public static RetrofitHttpError httpError(String url, Response response, Converter converter,
+                                            Type successType) {
+    return new RetrofitHttpError(url, response, converter, successType);
   }
 
-  public static RetrofitError unexpectedError(String url, Throwable exception) {
-    return new RetrofitError(url, null, null, null, false, exception);
+  public static UnexpectedRetrofitError unexpectedError(String url, Throwable exception) {
+    return new UnexpectedRetrofitError(url, exception);
   }
 
   private final String url;
-  private final Response response;
-  private final Converter converter;
-  private final Type successType;
-  private final boolean networkError;
 
-  private RetrofitError(String url, Response response, Converter converter, Type successType,
-      boolean networkError, Throwable exception) {
+  protected RetrofitError(String message, String url) {
+    super(message);
+    this.url = url;
+  }
+
+  protected RetrofitError(String url, Throwable exception) {
     super(exception);
     this.url = url;
-    this.response = response;
-    this.converter = converter;
-    this.successType = successType;
-    this.networkError = networkError;
   }
 
-  /** The request URL which produced the error. */
+  protected RetrofitError(String message, String url, Throwable exception) {
+    super(message, exception);
+    this.url = url;
+  }
+
+  /**
+   * The request URL which produced the error.
+   */
   public String getUrl() {
     return url;
   }
 
-  /** Response object containing status code, headers, body, etc. */
-  public Response getResponse() {
-    return response;
-  }
+  /**
+   * Response object containing status code, headers, body, etc. if present, null otherwise.
+   *
+   * @deprecated Will be moved to {@link RetrofitResponseError}.
+   */
+  @Deprecated
+  public abstract Response getResponse();
 
-  /** Whether or not this error was the result of a network error. */
-  public boolean isNetworkError() {
-    return networkError;
-  }
+  /**
+   * Whether or not this error was the result of a network error.
+   *
+   * @deprecated Use the instanceof operator instead and test for a {@link RetrofitNetworkError}.
+   */
+  @Deprecated
+  public abstract boolean isNetworkError();
 
   /**
    * HTTP response body converted to the type declared by either the interface method return type or
-   * the generic type of the supplied {@link Callback} parameter.
+   * the generic type of the supplied {@link Callback} parameter. Null if no body is present.
+   *
+   * @deprecated Will be moved to {@link RetrofitResponseError}.
    */
-  public Object getBody() {
-    TypedInput body = response.getBody();
-    if (body == null) {
-      return null;
-    }
-    try {
-      return converter.fromBody(body, successType);
-    } catch (ConversionException e) {
-      throw new RuntimeException(e);
-    }
-  }
+  @Deprecated
+  public abstract Object getBody();
 
-  /** HTTP response body converted to specified {@code type}. */
-  public Object getBodyAs(Type type) {
-    TypedInput body = response.getBody();
-    if (body == null) {
-      return null;
-    }
-    try {
-      return converter.fromBody(body, type);
-    } catch (ConversionException e) {
-      throw new RuntimeException(e);
-    }
-  }
+  /**
+   * HTTP response body converted to specified {@code type}. Null if no body is present.
+   *
+   * @deprecated Will be moved to {@link RetrofitResponseError}.
+   */
+  @Deprecated
+  public abstract Object getBodyAs(Type type);
 }
diff --git a/retrofit/src/main/java/retrofit/RetrofitHttpError.java b/retrofit/src/main/java/retrofit/RetrofitHttpError.java
new file mode 100644
index 0000000000..2bd80334c4
--- /dev/null
+++ b/retrofit/src/main/java/retrofit/RetrofitHttpError.java
@@ -0,0 +1,13 @@
+package retrofit;
+
+import retrofit.client.Response;
+import retrofit.converter.Converter;
+
+import java.lang.reflect.Type;
+
+public class RetrofitHttpError extends RetrofitResponseError {
+  protected RetrofitHttpError(String url, Response response, Converter converter,
+                              Type successType) {
+    super(url, response, converter, successType);
+  }
+}
diff --git a/retrofit/src/main/java/retrofit/RetrofitNetworkError.java b/retrofit/src/main/java/retrofit/RetrofitNetworkError.java
new file mode 100644
index 0000000000..2b94d3d451
--- /dev/null
+++ b/retrofit/src/main/java/retrofit/RetrofitNetworkError.java
@@ -0,0 +1,36 @@
+package retrofit;
+
+import retrofit.client.Response;
+
+import java.lang.reflect.Type;
+
+public class RetrofitNetworkError extends RetrofitError {
+
+  protected RetrofitNetworkError(String url, Throwable exception) {
+    super("Request to " + url + " failed.", url, exception);
+  }
+
+  @Override
+  @Deprecated
+  public Response getResponse() {
+    return null;
+  }
+
+  @Override
+  @Deprecated
+  public boolean isNetworkError() {
+    return true;
+  }
+
+  @Override
+  @Deprecated
+  public Object getBody() {
+    return null;
+  }
+
+  @Override
+  @Deprecated
+  public Object getBodyAs(Type type) {
+    return null;
+  }
+}
diff --git a/retrofit/src/main/java/retrofit/RetrofitResponseError.java b/retrofit/src/main/java/retrofit/RetrofitResponseError.java
new file mode 100644
index 0000000000..a56286db12
--- /dev/null
+++ b/retrofit/src/main/java/retrofit/RetrofitResponseError.java
@@ -0,0 +1,90 @@
+package retrofit;
+
+import retrofit.client.Response;
+import retrofit.converter.ConversionException;
+import retrofit.converter.Converter;
+import retrofit.mime.TypedInput;
+
+import java.lang.reflect.Type;
+
+public abstract class RetrofitResponseError extends RetrofitError {
+
+  protected Response response;
+  protected Converter converter;
+  protected Type successType;
+
+  protected RetrofitResponseError(String url, Response response, Converter converter,
+                                  Type successType, Throwable exception) {
+    super("Request to " + url + " failed; response code is "
+        + ((response == null) ? "unknown" : response.getStatus() + " " + response.getReason())
+        + ".", url, exception);
+    this.response = response;
+    this.converter = converter;
+    this.successType = successType;
+  }
+
+  protected RetrofitResponseError(String url, Response response, Converter converter,
+                                  Type successType) {
+    super("Request to " + url + " failed; response code is "
+        + ((response == null) ? "unknown" : response.getStatus() + " " + response.getReason())
+        + ".", url);
+    this.response = response;
+    this.converter = converter;
+    this.successType = successType;
+  }
+
+  @Override
+  @Deprecated
+  public boolean isNetworkError() {
+    return false;
+  }
+
+  @Override
+  public Response getResponse() {
+    return response;
+  }
+
+  @Override
+  public Object getBody() {
+    if (response == null) {
+      return null;
+    }
+
+    TypedInput body = response.getBody();
+    if (body == null) {
+      return null;
+    }
+
+    if (converter == null) {
+      throw new RuntimeException("Cannot convert body, supplied converter is null.");
+    }
+
+    try {
+      return converter.fromBody(body, successType);
+    } catch (ConversionException e) {
+      throw new RuntimeException(e);
+    }
+  }
+
+  @Override
+  public Object getBodyAs(Type type) {
+    if (response == null) {
+      return null;
+    }
+
+    TypedInput body = response.getBody();
+    if (body == null) {
+      return null;
+    }
+
+    if (converter == null) {
+      throw new RuntimeException("Cannot convert body, supplied converter is null.");
+    }
+
+    try {
+      return converter.fromBody(body, type);
+    } catch (ConversionException e) {
+      throw new RuntimeException(e);
+    }
+  }
+}
diff --git a/retrofit/src/main/java/retrofit/UnexpectedRetrofitError.java b/retrofit/src/main/java/retrofit/UnexpectedRetrofitError.java
new file mode 100644
index 0000000000..eb46806487
--- /dev/null
+++ b/retrofit/src/main/java/retrofit/UnexpectedRetrofitError.java
@@ -0,0 +1,35 @@
+package retrofit;
+
+import retrofit.client.Response;
+
+import java.lang.reflect.Type;
+
+public class UnexpectedRetrofitError extends RetrofitError {
+  protected UnexpectedRetrofitError(String url, Throwable exception) {
+    super("Request to " + url + " failed.", url, exception);
+  }
+
+  @Override
+  @Deprecated
+  public Response getResponse() {
+    return null;
+  }
+
+  @Override
+  @Deprecated
+  public boolean isNetworkError() {
+    return false;
+  }
+
+  @Override
+  @Deprecated
+  public Object getBody() {
+    return null;
+  }
+
+  @Override
+  @Deprecated
+  public Object getBodyAs(Type type) {
+    return null;
+  }
+}


Test patch:
diff --git a/retrofit/src/test/java/retrofit/RetrofitErrorTest.java b/retrofit/src/test/java/retrofit/RetrofitErrorTest.java
new file mode 100644
index 0000000000..9fe0e2c8f7
--- /dev/null
+++ b/retrofit/src/test/java/retrofit/RetrofitErrorTest.java
@@ -0,0 +1,213 @@
+package retrofit;
+
+import com.google.gson.Gson;
+import org.junit.Test;
+import retrofit.client.Header;
+import retrofit.client.Response;
+import retrofit.converter.ConversionException;
+import retrofit.converter.Converter;
+import retrofit.converter.GsonConverter;
+import retrofit.mime.TypedInput;
+import retrofit.mime.TypedString;
+
+import java.io.IOException;
+import java.lang.reflect.Type;
+import java.util.ArrayList;
+
+import static org.fest.assertions.api.Assertions.assertThat;
+import static org.fest.assertions.api.Assertions.fail;
+import static org.mockito.Matchers.any;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.when;
+
+@SuppressWarnings("ThrowableResultOfMethodCallIgnored")
+public class RetrofitErrorTest {
+
+  @Test
+  public void everyRetrofitErrorHasAMessage() throws Exception {
+    String url = "http://example.com/";
+    Response response = new Response(200, "OK", new ArrayList<Header>(),
+        new TypedString("\"foo\""));
+    Converter conv = new GsonConverter(new Gson());
+
+    assertThat(RetrofitError.networkError(url, new IOException()).getMessage())
+        .isNotNull()
+        .isNotEmpty()
+        .contains(url);
+
+    assertThat(RetrofitError.conversionError(url, response, conv, String.class, null).getMessage())
+        .isNotNull()
+        .isNotEmpty()
+        .contains(url)
+        .contains("200 OK");
+
+    assertThat(RetrofitError.httpError(url, response, conv, String.class).getMessage())
+        .isNotNull()
+        .isNotEmpty()
+        .contains(url)
+        .contains("200 OK");
+
+    assertThat(RetrofitError.unexpectedError(url, new IOException()).getMessage())
+        .isNotNull()
+        .isNotEmpty()
+        .contains(url);
+  }
+
+  @Test
+  public void isNetworkErrorIsOnlyTrueForNetworkErrors() throws Exception {
+    assertThat(RetrofitError.networkError(null, null).isNetworkError()).isTrue();
+    assertThat(RetrofitError.conversionError(null, null, null, null, null).isNetworkError())
+        .isFalse();
+    assertThat(RetrofitError.httpError(null, null, null, null).isNetworkError()).isFalse();
+    assertThat(RetrofitError.unexpectedError(null, null).isNetworkError()).isFalse();
+  }
+
+  @Test
+  public void getBodyReturnsNullIfResponseIsNull() throws Exception {
+    assertThat(RetrofitError.networkError(null, null).getBody()).isNull();
+    assertThat(RetrofitError.conversionError(null, null, null, null, null).getBody()).isNull();
+    assertThat(RetrofitError.httpError(null, null, null, null).getBody()).isNull();
+    assertThat(RetrofitError.unexpectedError(null, null).getBody()).isNull();
+  }
+
+  @Test
+  public void getBodyReturnsNullIfResponseBodyIsNull() throws Exception {
+    Response dummyResponse = new Response(204, "No Content", new ArrayList<Header>(), null);
+    assertThat(RetrofitError.conversionError(null, dummyResponse, null, null, null).getBody())
+        .isNull();
+    assertThat(RetrofitError.httpError(null, dummyResponse, null, null).getBody()).isNull();
+  }
+
+  @Test
+  public void getBodyThrowsRuntimeExceptionIfBodyIsPresentButConverterIsNot() throws Exception {
+    Response dummyResponse = new Response(200, "OK", new ArrayList<Header>(),
+        new TypedString("\"foo\""));
+
+    try {
+      RetrofitError.conversionError(null, dummyResponse, null, null, null).getBody();
+      fail("A RuntimeException should have been thrown.");
+    } catch (RuntimeException e) {
+      // ok
+    }
+
+    try {
+      RetrofitError.httpError(null, dummyResponse, null, null).getBody();
+      fail("A RuntimeException should have been thrown.");
+    } catch (RuntimeException e) {
+      // ok
+    }
+  }
+
+  @Test
+  public void getBodyReturnsConvertedBody() throws Exception {
+    Response dummyResponse = new Response(200, "OK", new ArrayList<Header>(),
+        new TypedString("\"foo\""));
+    Converter converter = new GsonConverter(new Gson());
+
+    assertThat((String) RetrofitError.conversionError(null, dummyResponse, converter,
+        String.class, null).getBody())
+        .isEqualTo("foo");
+
+    assertThat((String) RetrofitError.httpError(null, dummyResponse, converter, String.class)
+        .getBody()).isEqualTo("foo");
+  }
+
+  @Test
+  public void getBodyThrowsRuntimeExceptionIfConversionExceptionWasThrown() throws Exception {
+    Response dummyResponse = new Response(200, "OK", new ArrayList<Header>(),
+        new TypedString("\"foo\""));
+
+    Converter converter = mock(Converter.class);
+    when(converter.fromBody(any(TypedInput.class), any(Type.class)))
+        .thenThrow(new ConversionException("Expected."));
+
+    try {
+      RetrofitError.conversionError(null, dummyResponse, converter, String.class, null).getBody();
+      fail("A RuntimeException should have been thrown.");
+    } catch (RuntimeException e) {
+      // ok
+    }
+
+    try {
+      RetrofitError.httpError(null, dummyResponse, converter, String.class).getBody();
+      fail("A RuntimeException should have been thrown.");
+    } catch (RuntimeException e) {
+      // ok
+    }
+  }
+
+  @Test
+  public void getBodyAsReturnsNullIfResponseIsNull() throws Exception {
+    assertThat(RetrofitError.networkError(null, null).getBodyAs(String.class)).isNull();
+    assertThat(RetrofitError.conversionError(null, null, null, null, null).getBodyAs(String.class))
+        .isNull();
+    assertThat(RetrofitError.httpError(null, null, null, null).getBodyAs(String.class)).isNull();
+    assertThat(RetrofitError.unexpectedError(null, null).getBodyAs(String.class)).isNull();
+  }
+
+  @Test
+  public void getBodyAsReturnsNullIfResponseBodyIsNull() throws Exception {
+    Response dummyResponse = new Response(204, "No Content", new ArrayList<Header>(), null);
+    assertThat(RetrofitError.conversionError(null, dummyResponse, null, null, null)
+        .getBodyAs(String.class)).isNull();
+    assertThat(RetrofitError.httpError(null, dummyResponse, null, null).getBodyAs(String.class)).isNull();
+  }
+
+  @Test
+  public void getBodyAsThrowsRuntimeExceptionIfBodyIsPresentButConverterIsNot() throws Exception {
+    Response dummyResponse = new Response(204, "No Content", new ArrayList<Header>(),
+        new TypedString("\"foo\""));
+
+    try {
+      RetrofitError.conversionError(null, dummyResponse, null, null, null).getBodyAs(String.class);
+      fail("A RuntimeException should have been thrown.");
+    } catch (RuntimeException e) {
+      // ok
+    }
+
+    try {
+      RetrofitError.httpError(null, dummyResponse, null, null).getBodyAs(String.class);
+      fail("A RuntimeException should have been thrown.");
+    } catch (RuntimeException e) {
+      // ok
+    }
+  }
+
+  @Test
+  public void getBodyAsReturnsConvertedBody() throws Exception {
+    Response response = new Response(200, "OK", new ArrayList<Header>(),
+        new TypedString("\"foo\""));
+    Converter converter = new GsonConverter(new Gson());
+
+    assertThat((String) RetrofitError.conversionError(null, response, converter, String.class, null)
+        .getBodyAs(String.class)).isEqualTo("foo");
+
+    assertThat((String) RetrofitError.httpError(null, response, converter, String.class)
+        .getBodyAs(String.class)).isEqualTo("foo");
+  }
+
+  @Test
+  public void getBodyAsThrowsRuntimeExceptionIfConversionExceptionWasThrown() throws Exception {
+    Response dummyResponse = new Response(200, "OK", new ArrayList<Header>(),
+        new TypedString("\"foo\""));
+
+    Converter converter = mock(Converter.class);
+    when(converter.fromBody(any(TypedInput.class), any(Type.class)))
+        .thenThrow(new ConversionException("Expected."));
+
+    try {
+      RetrofitError.conversionError(null, dummyResponse, converter, String.class, null)
+          .getBodyAs(String.class);
+      fail("A RuntimeException should have been thrown.");
+    } catch (RuntimeException e) {
+      // ok
+    }
+
+    try {
+      RetrofitError.httpError(null, dummyResponse, converter, String.class).getBodyAs(String.class);
+      fail("A RuntimeException should have been thrown.");
+    } catch (RuntimeException e) {
+      // ok
+    }
+  }
+}