propagate error message on rate limit exception (#579)

src/main/java/com/auth0/exception/RateLimitException.java

@@ -1,5 +1,7 @@
 package com.auth0.exception;
 
+import java.util.Map;
+
 /**
  * Represents a server error when a rate limit has been exceeded.
  * <p>
@@ -16,6 +18,13 @@ public class RateLimitException extends APIException {
 
     private static final int STATUS_CODE_TOO_MANY_REQUEST = 429;
 
+    public RateLimitException(long limit, long remaining, long reset, Map<String, Object> values) {
+        super(values, STATUS_CODE_TOO_MANY_REQUEST);
+        this.limit = limit;
+        this.remaining = remaining;
+        this.reset = reset;
+    }
+
     public RateLimitException(long limit, long remaining, long reset) {
         super("Rate limit reached", STATUS_CODE_TOO_MANY_REQUEST, null);
         this.limit = limit;

src/main/java/com/auth0/net/BaseRequest.java

@@ -218,7 +218,15 @@ private RateLimitException createRateLimitException(Auth0HttpResponse response)
         long limit = Long.parseLong(response.getHeader("x-ratelimit-limit", "-1"));
         long remaining = Long.parseLong(response.getHeader("x-ratelimit-remaining", "-1"));
         long reset = Long.parseLong(response.getHeader("x-ratelimit-reset", "-1"));
-        return new RateLimitException(limit, remaining, reset);
+
+        String payload = response.getBody();
+        MapType mapType = mapper.getTypeFactory().constructMapType(HashMap.class, String.class, Object.class);
+        try {
+            Map<String, Object> values = mapper.readValue(payload, mapType);
+            return new RateLimitException(limit, remaining, reset, values);
+        } catch (IOException e) {
+            return new RateLimitException(limit, remaining, reset);
+        }
     }
 
     /**

src/main/java/com/auth0/net/RateLimitInterceptor.java

@@ -67,24 +67,24 @@ public Response intercept(@NotNull Chain chain) throws IOException {
             .withMaxRetries(maxRetries)
             .withBackoff(INITIAL_INTERVAL, MAX_INTERVAL, ChronoUnit.MILLIS)
             .withJitter(JITTER)
-            .handleResultIf(response -> {
-                if (response.code() == 429) {
-                    response.close();
-                    return true;
-                }
-                return false;
-            });
+            .handleResultIf(response -> response.code() == 429);
 
         // For testing purposes only, allow test to hook into retry listener to enable verification of retry backoff
         if (retryListener != null) {
             retryPolicy.onRetry(retryListener);
         }
 
         try {
+            return Failsafe.with(retryPolicy).get((context) -> {
+                // ensure response of last recorded response prior to retry is closed
+                if (context.getLastResult() != null) {
+                    context.getLastResult().close();;
+                }
+                return chain.proceed(chain.request());
+            });
+        } catch (FailsafeException fe) {
             // throw Auth0Exception instead of FailSafe exception on error
             // see https://github.com/auth0/auth0-java/issues/483
-            return Failsafe.with(retryPolicy).get(() -> chain.proceed(chain.request()));
-        } catch (FailsafeException fe) {
             throw new Auth0Exception("Failed to execute request", fe.getCause());
         }
     }

src/main/java/com/auth0/net/client/DefaultHttpClient.java

@@ -100,6 +100,8 @@ public void onResponse(@NotNull Call call, @NotNull Response response) {
                     future.complete(buildResponse(response));
                 } catch (IOException e) {
                     future.completeExceptionally(e);
+                } finally {
+                    response.close();
                 }
             }
         });
@@ -138,9 +140,7 @@ private Auth0HttpResponse buildResponse(Response okResponse) throws IOException
         ResponseBody responseBody = okResponse.body();
         String content = null;
 
-        // The RateLimitInterceptor needs to close the response; we don't need the body in that case and trying to
-        // get the body will result in an exception because the responsebody has been closed
-        if (Objects.nonNull(responseBody) && okResponse.code() != 429) {
+        if (Objects.nonNull(responseBody)) {
             content = responseBody.string();
         }
         return Auth0HttpResponse.newBuilder()

src/test/java/com/auth0/client/MockServer.java

@@ -144,6 +144,7 @@ public class MockServer {
     public static final String KEY_LIST = "src/test/resources/mgmt/key_list.json";
     public static final String KEY_REVOKE = "src/test/resources/mgmt/key_revoke.json";
     public static final String KEY_ROTATE = "src/test/resources/mgmt/key_rotate.json";
+    public static final String RATE_LIMIT_ERROR = "src/test/resources/mgmt/rate_limit_error.json";
 
     private final MockWebServer server;
 
@@ -184,7 +185,11 @@ public void noContentResponse() {
         server.enqueue(response);
     }
 
-    public void rateLimitReachedResponse(long limit, long remaining, long reset) {
+    public void rateLimitReachedResponse(long limit, long remaining, long reset) throws IOException {
+        rateLimitReachedResponse(limit, remaining, reset, null);
+    }
+
+    public void rateLimitReachedResponse(long limit, long remaining, long reset, String path) throws IOException {
         MockResponse response = new MockResponse().setResponseCode(429);
         if (limit != -1) {
             response.addHeader("x-ratelimit-limit", String.valueOf(limit));
@@ -195,6 +200,11 @@ public void rateLimitReachedResponse(long limit, long remaining, long reset) {
         if (reset != -1) {
             response.addHeader("x-ratelimit-reset", String.valueOf(reset));
         }
+        if (path != null) {
+                response
+                    .addHeader("Content-Type", "application/json")
+                    .setBody(readTextFile(path));
+        }
         server.enqueue(response);
     }
 

src/test/java/com/auth0/net/BaseRequestTest.java

@@ -401,9 +401,9 @@ public void shouldParsePlainTextErrorResponse() throws Exception {
     }
 
     @Test
-    public void shouldParseRateLimitsHeaders() {
+    public void shouldParseRateLimitException() throws Exception {
         BaseRequest<List> request = new BaseRequest<>(client, tokenProvider, server.getBaseUrl(),  HttpMethod.GET, listType);
-        server.rateLimitReachedResponse(100, 10, 5);
+        server.rateLimitReachedResponse(100, 10, 5, RATE_LIMIT_ERROR);
         Exception exception = null;
         try {
             request.execute().getBody();
@@ -414,10 +414,10 @@ public void shouldParseRateLimitsHeaders() {
         assertThat(exception, Matchers.is(notNullValue()));
         assertThat(exception, Matchers.is(Matchers.instanceOf(RateLimitException.class)));
         assertThat(exception.getCause(), Matchers.is(nullValue()));
-        assertThat(exception.getMessage(), Matchers.is("Request failed with status code 429: Rate limit reached"));
+        assertThat(exception.getMessage(), Matchers.is("Request failed with status code 429: Global limit has been reached"));
         RateLimitException rateLimitException = (RateLimitException) exception;
-        assertThat(rateLimitException.getDescription(), Matchers.is("Rate limit reached"));
-        assertThat(rateLimitException.getError(), Matchers.is(nullValue()));
+        assertThat(rateLimitException.getDescription(), Matchers.is("Global limit has been reached"));
+        assertThat(rateLimitException.getError(), Matchers.is("too_many_requests"));
         assertThat(rateLimitException.getValue("non_existing_key"), Matchers.is(nullValue()));
         assertThat(rateLimitException.getStatusCode(), Matchers.is(429));
         assertThat(rateLimitException.getLimit(), Matchers.is(100L));
@@ -426,7 +426,7 @@ public void shouldParseRateLimitsHeaders() {
     }
 
     @Test
-    public void shouldDefaultRateLimitsHeadersWhenMissing() {
+    public void shouldDefaultRateLimitsHeadersWhenMissing() throws Exception {
         BaseRequest<List> request = new BaseRequest<>(client, tokenProvider, server.getBaseUrl(),  HttpMethod.GET, listType);
         server.rateLimitReachedResponse(-1, -1, -1);
         Exception exception = null;

src/test/java/com/auth0/net/MultipartRequestTest.java

@@ -335,7 +335,7 @@ public void shouldParsePlainTextErrorResponse() throws Exception {
     }
 
     @Test
-    public void shouldParseRateLimitsHeaders() {
+    public void shouldParseRateLimitsHeaders() throws Exception {
         MultipartRequest<List> request = new MultipartRequest<>(client, tokenProvider, server.getBaseUrl(), HttpMethod.POST, listType);
         request.addPart("non_empty", "body");
         server.rateLimitReachedResponse(100, 10, 5);
@@ -361,7 +361,7 @@ public void shouldParseRateLimitsHeaders() {
     }
 
     @Test
-    public void shouldDefaultRateLimitsHeadersWhenMissing() {
+    public void shouldDefaultRateLimitsHeadersWhenMissing() throws Exception {
         MultipartRequest<List> request = new MultipartRequest<>(client, tokenProvider, server.getBaseUrl(), HttpMethod.POST, listType);
         request.addPart("non_empty", "body");
         server.rateLimitReachedResponse(-1, -1, -1);

src/test/java/com/auth0/net/RateLimitInterceptorTest.java

@@ -108,9 +108,7 @@ public void shouldReturnResponseWhenMaxRetriesHit() throws Exception {
 
         int index = 0;
         for (int i = 0; i < maxRetries + 1; i++) {
-            System.out.println("about to take request " + i);
             index = server.takeRequest().getSequenceNumber();
-            System.out.println("took request " + i);
         }
 
         assertThat(response.code(), is(429));

src/test/java/com/auth0/net/client/DefaultHttpClientTest.java

@@ -462,6 +462,31 @@ public void alwaysCloseResponseOnSuccessfulRequest() throws IOException {
         verify(response, times(1)).close();
     }
 
+    @Test
+    public void alwaysCloseResponseOnSuccessfulRequestAsync() throws Exception {
+        OkHttpClient okClient = Mockito.mock(OkHttpClient.class);
+        okhttp3.Response response = Mockito.mock(okhttp3.Response.class);
+        Call call = Mockito.mock(Call.class);
+
+        doReturn(call).when(okClient).newCall(any());
+
+        doAnswer(invocation -> {
+            ((Callback) invocation.getArgument(0)).onResponse(call, response);
+            return null;
+        }).when(call).enqueue(any(Callback.class));
+
+        Headers headers = Mockito.mock(Headers.class);
+        when(response.headers()).thenReturn(headers);
+
+        DefaultHttpClient client = new DefaultHttpClient(okClient);
+        Auth0HttpRequest request = Auth0HttpRequest.newBuilder(server.url("/users/").toString(), HttpMethod.POST)
+            .withBody(HttpRequestBody.create("application/json", "{}".getBytes()))
+            .build();
+
+        client.sendRequestAsync(request).get();
+        verify(response, times(1)).close();
+    }
+
     @Test
     public void closesResponseOnAPIError() throws Exception {
         okhttp3.Response response = Mockito.mock(okhttp3.Response.class);
@@ -490,6 +515,36 @@ public void closesResponseOnAPIError() throws Exception {
         verify(response, times(1)).close();
     }
 
+    @Test
+    public void closesResponseOnAPIErrorAsync() throws Exception {
+        OkHttpClient okClient = Mockito.mock(OkHttpClient.class);
+        okhttp3.Response response = Mockito.mock(okhttp3.Response.class);
+        Call call = Mockito.mock(Call.class);
+
+        doReturn(call).when(okClient).newCall(any());
+
+        doAnswer(invocation -> {
+            ((Callback) invocation.getArgument(0)).onResponse(call, response);
+            return null;
+        }).when(call).enqueue(any(Callback.class));
+
+        Headers headers = Mockito.mock(Headers.class);
+        when(response.headers()).thenReturn(headers);
+
+        DefaultHttpClient client = new DefaultHttpClient(okClient);
+        Auth0HttpRequest request = Auth0HttpRequest.newBuilder(server.url("/users/").toString(), HttpMethod.POST)
+            .withBody(HttpRequestBody.create("application/json", "{}".getBytes()))
+            .build();
+
+        MockResponse mockResponse = new MockResponse()
+            .setResponseCode(500)
+            .setBody("server error");
+
+        server.enqueue(mockResponse);
+        client.sendRequestAsync(request).get();
+        verify(response, times(1)).close();
+    }
+
     @Test
     public void makesFormBodyPostRequest() throws Exception {
         Map<String, Object> params = new HashMap<>();

src/test/resources/mgmt/rate_limit_error.json

@@ -0,0 +1,6 @@
+{
+  "statusCode":429,
+  "error":"Too Many Requests",
+  "message":"Global limit has been reached",
+  "errorCode":"too_many_requests"
+}