diff --git a/src/main/java/com/auth0/exception/RateLimitException.java b/src/main/java/com/auth0/exception/RateLimitException.java index c5e79e81..738a941f 100644 --- a/src/main/java/com/auth0/exception/RateLimitException.java +++ b/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. *

@@ -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 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; diff --git a/src/main/java/com/auth0/net/BaseRequest.java b/src/main/java/com/auth0/net/BaseRequest.java index e124ea12..44f5e1b4 100644 --- a/src/main/java/com/auth0/net/BaseRequest.java +++ b/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 values = mapper.readValue(payload, mapType); + return new RateLimitException(limit, remaining, reset, values); + } catch (IOException e) { + return new RateLimitException(limit, remaining, reset); + } } /** diff --git a/src/main/java/com/auth0/net/RateLimitInterceptor.java b/src/main/java/com/auth0/net/RateLimitInterceptor.java index 24dd69ad..842f143a 100644 --- a/src/main/java/com/auth0/net/RateLimitInterceptor.java +++ b/src/main/java/com/auth0/net/RateLimitInterceptor.java @@ -67,13 +67,7 @@ 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) { @@ -81,10 +75,16 @@ public Response intercept(@NotNull Chain chain) throws IOException { } 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()); } } diff --git a/src/main/java/com/auth0/net/client/DefaultHttpClient.java b/src/main/java/com/auth0/net/client/DefaultHttpClient.java index 2785a67c..8daf3cea 100644 --- a/src/main/java/com/auth0/net/client/DefaultHttpClient.java +++ b/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() diff --git a/src/test/java/com/auth0/client/MockServer.java b/src/test/java/com/auth0/client/MockServer.java index a9d5caa5..26a1bcac 100644 --- a/src/test/java/com/auth0/client/MockServer.java +++ b/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); } diff --git a/src/test/java/com/auth0/net/BaseRequestTest.java b/src/test/java/com/auth0/net/BaseRequestTest.java index b66f21c0..5fe9f002 100644 --- a/src/test/java/com/auth0/net/BaseRequestTest.java +++ b/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 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 request = new BaseRequest<>(client, tokenProvider, server.getBaseUrl(), HttpMethod.GET, listType); server.rateLimitReachedResponse(-1, -1, -1); Exception exception = null; diff --git a/src/test/java/com/auth0/net/MultipartRequestTest.java b/src/test/java/com/auth0/net/MultipartRequestTest.java index 670a704c..9eb1f61f 100644 --- a/src/test/java/com/auth0/net/MultipartRequestTest.java +++ b/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 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 request = new MultipartRequest<>(client, tokenProvider, server.getBaseUrl(), HttpMethod.POST, listType); request.addPart("non_empty", "body"); server.rateLimitReachedResponse(-1, -1, -1); diff --git a/src/test/java/com/auth0/net/RateLimitInterceptorTest.java b/src/test/java/com/auth0/net/RateLimitInterceptorTest.java index f737578c..f05ea72b 100644 --- a/src/test/java/com/auth0/net/RateLimitInterceptorTest.java +++ b/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)); diff --git a/src/test/java/com/auth0/net/client/DefaultHttpClientTest.java b/src/test/java/com/auth0/net/client/DefaultHttpClientTest.java index 0066a581..a9f1e7e7 100644 --- a/src/test/java/com/auth0/net/client/DefaultHttpClientTest.java +++ b/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 params = new HashMap<>(); diff --git a/src/test/resources/mgmt/rate_limit_error.json b/src/test/resources/mgmt/rate_limit_error.json new file mode 100644 index 00000000..27478a62 --- /dev/null +++ b/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" +}