Repository: square/retrofit
Issue: Only one HTTP method is allowed. Found: POST and POST

Same as  #473 , the real cause (malformed interface) is thrown the first time and then the code continues with half initialised state leading to a misleading exception.

actual exception thrown once

```
07-09 10:37:21.792  31834-32136/com.xxx.client D/MyRestClient﹕ java.lang.IllegalArgumentException:MyRestApi.postRawData: No Retrofit annotation found. (parameter #1)
            at retrofit.RestMethodInfo.methodError(RestMethodInfo.java:123)
            at retrofit.RestMethodInfo.parameterError(RestMethodInfo.java:127)
            at retrofit.RestMethodInfo.parseParameters(RestMethodInfo.java:450)
            at retrofit.RestMethodInfo.init(RestMethodInfo.java:134)
            at retrofit.RestHandler$RestHandler.invokeRequest(RestAdapter.java:294)
```

misleading exception thrown on subsequent calls

```
07-09 10:37:22.792  31834-32137/com.xxx.client D/MyRestClient﹕ java.lang.IllegalArgumentException: MyRestApi.postRawData: Only one HTTP method is allowed. Found: POST and POST.
            at retrofit.RestMethodInfo.methodError(RestMethodInfo.java:123)
            at retrofit.RestMethodInfo.parseMethodAnnotations(RestMethodInfo.java:155)
            at retrofit.RestMethodInfo.init(RestMethodInfo.java:133)
            at retrofit.RestAdapter$RestHandler.invokeRequest(RestAdapter.java:294)
```


Issue comments:
The first exception should have crashed your app.

It should indeed. Unfortunately there is no direct way to check for "RetrofitError.unexpectedError()", so I'm now using something like this in my callbacks:

``` java
/**
     * Makes sure unexpected errors crash the app
     * https://github.com/square/retrofit/issues/560
     * @param retrofitError the retrofit error
     */
    private static void failOnUnexpectedError(RetrofitError retrofitError) {
        // see RetrofitError.unexpectedError() for reasoning
        if(!retrofitError.isNetworkError() && retrofitError.getResponse() == null){
            throw new RuntimeException(retrofitError.getMessage(), retrofitError);
        }
    }
```

Would you propose something else? If not I suppose this can be closed.

Yeah that's unfortunately the best way right now. Let's leave this issue open and we can look into a better way of identifying what type of failure happened to make things like this easier.


Pull request review comments:
Maybe split this into 4xx, 5xx status codes? Or the caller can do that with access to the code anyway

Should this be a RetrofitError or a RuntimeException?

Does this include malformed HTTP? It probably does!

Should these be RetrofitError subclasses? NetworkError, HttpError, ConversionError ?

(it guarantees that you'll get the problem kind in your crash logs!)

It should be a `RuntimeException` but we're type-limited to `RetrofitError` because of `Callback`'s API signature.

All these things we can address in v2, though!

We don't hand you any of the response body for network errors, even if the `IOException` happens on the very last byte.

Hmm, possibly. I generally fear the type explosion, but we seem pretty stable on the number of these things. Pre-1.0 Retrofit disambiguated the 4xx vs. 5xx stuff which I collapsed for the 1.0 release. I'll see what this looks like.

Ahh.


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 8fff9d212c..8cc23faece 100644
--- a/retrofit-mock/src/main/java/retrofit/MockHttpRetrofitError.java
+++ b/retrofit-mock/src/main/java/retrofit/MockHttpRetrofitError.java
@@ -9,7 +9,7 @@ class MockHttpRetrofitError extends RetrofitError {
 
   MockHttpRetrofitError(String message, String url, Response response, Object body,
       Type responseType) {
-    super(message, url, response, null, responseType, false, null);
+    super(message, url, response, null, responseType, Kind.HTTP, null);
     this.body = body;
   }
 
diff --git a/retrofit/src/main/java/retrofit/RetrofitError.java b/retrofit/src/main/java/retrofit/RetrofitError.java
index 9b335c8424..584b40664c 100644
--- a/retrofit/src/main/java/retrofit/RetrofitError.java
+++ b/retrofit/src/main/java/retrofit/RetrofitError.java
@@ -24,39 +24,56 @@
 
 public class RetrofitError extends RuntimeException {
   public static RetrofitError networkError(String url, IOException exception) {
-    return new RetrofitError(exception.getMessage(), url, null, null, null, true, exception);
+    return new RetrofitError(exception.getMessage(), url, null, null, null, Kind.NETWORK,
+        exception);
   }
 
   public static RetrofitError conversionError(String url, Response response, Converter converter,
       Type successType, ConversionException exception) {
-    return new RetrofitError(exception.getMessage(), url, response, converter, successType, false,
-        exception);
+    return new RetrofitError(exception.getMessage(), url, response, converter, successType,
+        Kind.CONVERSION, exception);
   }
 
   public static RetrofitError httpError(String url, Response response, Converter converter,
       Type successType) {
     String message = response.getStatus() + " " + response.getReason();
-    return new RetrofitError(message, url, response, converter, successType, false, null);
+    return new RetrofitError(message, url, response, converter, successType, Kind.HTTP, null);
   }
 
   public static RetrofitError unexpectedError(String url, Throwable exception) {
-    return new RetrofitError(exception.getMessage(), url, null, null, null, false, exception);
+    return new RetrofitError(exception.getMessage(), url, null, null, null, Kind.UNEXPECTED,
+        exception);
+  }
+
+  /** Identifies the event kind which triggered a {@link RetrofitError}. */
+  public enum Kind {
+    /** An {@link IOException} occurred while communicating to the server. */
+    NETWORK,
+    /** An exception was thrown while (de)serializing a body. */
+    CONVERSION,
+    /** A non-200 HTTP status code was received from the server. */
+    HTTP,
+    /**
+     * An internal error occurred while attempting to execute a request. It is best practice to
+     * re-throw this exception so your application crashes.
+     */
+    UNEXPECTED
   }
 
   private final String url;
   private final Response response;
   private final Converter converter;
   private final Type successType;
-  private final boolean networkError;
+  private final Kind kind;
 
   RetrofitError(String message, String url, Response response, Converter converter,
-      Type successType, boolean networkError, Throwable exception) {
+      Type successType, Kind kind, Throwable exception) {
     super(message, exception);
     this.url = url;
     this.response = response;
     this.converter = converter;
     this.successType = successType;
-    this.networkError = networkError;
+    this.kind = kind;
   }
 
   /** The request URL which produced the error. */
@@ -69,14 +86,23 @@ public Response getResponse() {
     return response;
   }
 
-  /** 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 {@link #getKind() getKind() == Kind.NETWORK}.
+   */
+  @Deprecated public boolean isNetworkError() {
+    return kind == Kind.NETWORK;
+  }
+
+  /** The event kind which triggered this error. */
+  public Kind getKind() {
+    return kind;
   }
 
   /**
-   * 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. {@code null} if there is no
+   * 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. {@code null} if there is no
    * response.
    *
    * @throws RuntimeException if unable to convert the body to the {@link #getSuccessType() success


Test patch:
diff --git a/retrofit-mock/src/test/java/retrofit/MockRestAdapterTest.java b/retrofit-mock/src/test/java/retrofit/MockRestAdapterTest.java
index f2c894e6b9..412cf7d5ba 100644
--- a/retrofit-mock/src/test/java/retrofit/MockRestAdapterTest.java
+++ b/retrofit-mock/src/test/java/retrofit/MockRestAdapterTest.java
@@ -232,7 +232,7 @@ class MockSyncExample implements SyncExample {
       mockService.doStuff();
       fail();
     } catch (RetrofitError e) {
-      assertThat(e.isNetworkError()).isTrue();
+      assertThat(e.getKind()).isEqualTo(RetrofitError.Kind.NETWORK);
       assertThat(e.getCause()).hasMessage("Mock network error!");
     }
   }
@@ -264,7 +264,7 @@ class MockAsyncExample implements AsyncExample {
     verify(callbackExecutor).execute(any(Runnable.class));
 
     RetrofitError error = errorRef.get();
-    assertThat(error.isNetworkError()).isTrue();
+    assertThat(error.getKind()).isEqualTo(RetrofitError.Kind.NETWORK);
     assertThat(error.getCause()).hasMessage("Mock network error!");
   }
 
@@ -395,7 +395,7 @@ class MockSyncExample implements SyncExample {
     } catch (RetrofitError e) {
       long tookMs = TimeUnit.NANOSECONDS.toMillis(System.nanoTime() - startNanos);
       assertThat(tookMs).isGreaterThanOrEqualTo(100);
-      assertThat(e.isNetworkError()).isFalse();
+      assertThat(e.getKind()).isEqualTo(RetrofitError.Kind.HTTP);
       assertThat(e.getResponse().getStatus()).isEqualTo(404);
       assertThat(e.getResponse().getReason()).isEqualTo("Not Found");
       assertThat(e.getBody()).isSameAs(expected);
@@ -438,7 +438,7 @@ class MockAsyncExample implements AsyncExample {
 
     RetrofitError error = errorRef.get();
     assertThat(tookMs.get()).isGreaterThanOrEqualTo(100);
-    assertThat(error.isNetworkError()).isFalse();
+    assertThat(error.getKind()).isEqualTo(RetrofitError.Kind.HTTP);
     assertThat(error.getResponse().getStatus()).isEqualTo(404);
     assertThat(error.getResponse().getReason()).isEqualTo("Not Found");
     assertThat(error.getBody()).isSameAs(expected);
@@ -482,7 +482,7 @@ class MockObservableExample implements ObservableExample {
 
     RetrofitError error = errorRef.get();
     assertThat(tookMs.get()).isGreaterThanOrEqualTo(100);
-    assertThat(error.isNetworkError()).isFalse();
+    assertThat(error.getKind()).isEqualTo(RetrofitError.Kind.HTTP);
     assertThat(error.getResponse().getStatus()).isEqualTo(404);
     assertThat(error.getResponse().getReason()).isEqualTo("Not Found");
     assertThat(error.getBody()).isSameAs(expected);
@@ -523,7 +523,7 @@ class MockObservableExample implements ObservableExample {
 
     RetrofitError error = errorRef.get();
     assertThat(tookMs.get()).isGreaterThanOrEqualTo(100);
-    assertThat(error.isNetworkError()).isFalse();
+    assertThat(error.getKind()).isEqualTo(RetrofitError.Kind.HTTP);
     assertThat(error.getResponse().getStatus()).isEqualTo(400);
     assertThat(error.getResponse().getReason()).isEqualTo("Bad Request");
     assertThat(error.getBody()).isNull();
diff --git a/retrofit/src/test/java/retrofit/RestAdapterTest.java b/retrofit/src/test/java/retrofit/RestAdapterTest.java
index f56b3d20ad..c303788b74 100644
--- a/retrofit/src/test/java/retrofit/RestAdapterTest.java
+++ b/retrofit/src/test/java/retrofit/RestAdapterTest.java
@@ -446,6 +446,7 @@ private interface InvalidExample extends Example {
       example.something();
       fail("RetrofitError expected on malformed response body.");
     } catch (RetrofitError e) {
+      assertThat(e.getKind()).isEqualTo(RetrofitError.Kind.CONVERSION);
       assertThat(e.getResponse().getStatus()).isEqualTo(200);
       assertThat(e.getCause()).isInstanceOf(ConversionException.class);
       assertThat(e.getResponse().getBody()).isNull();
@@ -460,6 +461,7 @@ private interface InvalidExample extends Example {
       example.something();
       fail("RetrofitError expected on non-2XX response code.");
     } catch (RetrofitError e) {
+      assertThat(e.getKind()).isEqualTo(RetrofitError.Kind.HTTP);
       assertThat(e.getResponse().getStatus()).isEqualTo(500);
       assertThat(e.getSuccessType()).isEqualTo(String.class);
     }
@@ -553,6 +555,7 @@ private interface InvalidExample extends Example {
       example.something();
       fail("RetrofitError expected when client throws exception.");
     } catch (RetrofitError e) {
+      assertThat(e.getKind()).isEqualTo(RetrofitError.Kind.NETWORK);
       assertThat(e.getCause()).isSameAs(exception);
     }
   }
@@ -573,6 +576,7 @@ private interface InvalidExample extends Example {
       example.something();
       fail("RetrofitError expected on malformed response body.");
     } catch (RetrofitError e) {
+      assertThat(e.getKind()).isEqualTo(RetrofitError.Kind.NETWORK);
       assertThat(e.isNetworkError());
       assertThat(e.getCause()).isInstanceOf(IOException.class);
       assertThat(e.getCause()).hasMessage("I'm broken!");
@@ -587,6 +591,7 @@ private interface InvalidExample extends Example {
       example.something();
       fail("RetrofitError expected when unexpected exception thrown.");
     } catch (RetrofitError e) {
+      assertThat(e.getKind()).isEqualTo(RetrofitError.Kind.UNEXPECTED);
       assertThat(e.getCause()).isSameAs(exception);
     }
   }