From fccd8a2872b925c73f02495c71488e61ce0096c1 Mon Sep 17 00:00:00 2001
From: Jim Anderson <jimranderson@gmail.com>
Date: Thu, 2 Nov 2023 14:50:13 -0500
Subject: [PATCH 1/4] propagate error message on rate limit exception
---
.../java/com/auth0/client/auth/AuthAPI.java | 2 +-
.../auth0/exception/RateLimitException.java | 9 +++++++++
src/main/java/com/auth0/net/BaseRequest.java | 10 +++++++++-
.../com/auth0/net/RateLimitInterceptor.java | 18 +++++++++---------
.../auth0/net/client/DefaultHttpClient.java | 4 +---
src/test/java/com/auth0/client/MockServer.java | 12 +++++++++++-
.../java/com/auth0/net/BaseRequestTest.java | 12 ++++++------
.../com/auth0/net/MultipartRequestTest.java | 4 ++--
.../auth0/net/RateLimitInterceptorTest.java | 2 --
src/test/resources/mgmt/rate_limit_error.json | 6 ++++++
10 files changed, 54 insertions(+), 25 deletions(-)
create mode 100644 src/test/resources/mgmt/rate_limit_error.json
diff --git a/src/main/java/com/auth0/client/auth/AuthAPI.java b/src/main/java/com/auth0/client/auth/AuthAPI.java
index 9a94076e..5b35b3c8 100644
--- a/src/main/java/com/auth0/client/auth/AuthAPI.java
+++ b/src/main/java/com/auth0/client/auth/AuthAPI.java
@@ -1359,7 +1359,7 @@ public Builder withHttpClient(Auth0HttpClient httpClient) {
*/
public AuthAPI build() {
return new AuthAPI(domain, clientId, clientSecret, clientAssertionSigner,
- Objects.nonNull(httpClient) ? httpClient : DefaultHttpClient.newBuilder().build());
+ Objects.nonNull(httpClient) ? httpClient : DefaultHttpClient.newBuilder().withMaxRetries(0).build());
}
}
}
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.
* <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;
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<String, Object> 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..3a4e73eb 100644
--- a/src/main/java/com/auth0/net/client/DefaultHttpClient.java
+++ b/src/main/java/com/auth0/net/client/DefaultHttpClient.java
@@ -138,9 +138,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<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;
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<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);
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/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"
+}
From 6d01c8875a3b471a1fbfdc242b0bba36f6e997cd Mon Sep 17 00:00:00 2001
From: Jim Anderson <jimranderson@gmail.com>
Date: Fri, 3 Nov 2023 11:24:26 -0500
Subject: [PATCH 2/4] ensure async response is closed
---
.../auth0/net/client/DefaultHttpClient.java | 2 +
.../net/client/DefaultHttpClientTest.java | 55 +++++++++++++++++++
2 files changed, 57 insertions(+)
diff --git a/src/main/java/com/auth0/net/client/DefaultHttpClient.java b/src/main/java/com/auth0/net/client/DefaultHttpClient.java
index 3a4e73eb..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();
}
}
});
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<String, Object> params = new HashMap<>();
From 9111008f199b11fd3466db208991c9a57f8ea01b Mon Sep 17 00:00:00 2001
From: Jim Anderson <jimranderson@gmail.com>
Date: Fri, 3 Nov 2023 11:25:41 -0500
Subject: [PATCH 3/4] revert authapi retry config change
---
src/main/java/com/auth0/client/auth/AuthAPI.java | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/main/java/com/auth0/client/auth/AuthAPI.java b/src/main/java/com/auth0/client/auth/AuthAPI.java
index 5b35b3c8..9a94076e 100644
--- a/src/main/java/com/auth0/client/auth/AuthAPI.java
+++ b/src/main/java/com/auth0/client/auth/AuthAPI.java
@@ -1359,7 +1359,7 @@ public Builder withHttpClient(Auth0HttpClient httpClient) {
*/
public AuthAPI build() {
return new AuthAPI(domain, clientId, clientSecret, clientAssertionSigner,
- Objects.nonNull(httpClient) ? httpClient : DefaultHttpClient.newBuilder().withMaxRetries(0).build());
+ Objects.nonNull(httpClient) ? httpClient : DefaultHttpClient.newBuilder().build());
}
}
}
From a0134432fe70037220598f677eec08a23a294a31 Mon Sep 17 00:00:00 2001
From: Jim Anderson <jimranderson@gmail.com>
Date: Mon, 6 Nov 2023 19:38:35 -0600
Subject: [PATCH 4/4] Release 2.8.0
---
CHANGELOG.md | 6 ++++++
README.md | 4 ++--
2 files changed, 8 insertions(+), 2 deletions(-)
diff --git a/CHANGELOG.md b/CHANGELOG.md
index cf8fbb3e..aece5d83 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -1,5 +1,11 @@
# Change Log
+## [2.8.0](https://github.com/auth0/auth0-java/tree/2.8.0) (2023-11-07)
+[Full Changelog](https://github.com/auth0/auth0-java/compare/2.7.0...2.8.0)
+
+**Fixed**
+- Propagate error messages on rate limit exceptions [\#579](https://github.com/auth0/auth0-java/pull/579) ([jimmyjames](https://github.com/jimmyjames))
+
## [2.7.0](https://github.com/auth0/auth0-java/tree/2.7.0) (2023-10-31)
[Full Changelog](https://github.com/auth0/auth0-java/compare/2.6.1...2.7.0)
diff --git a/README.md b/README.md
index 4da400ed..6789a3bd 100644
--- a/README.md
+++ b/README.md
@@ -34,14 +34,14 @@ Add the dependency via Maven:
<dependency>
<groupId>com.auth0</groupId>
<artifactId>auth0</artifactId>
- <version>2.7.0</version>
+ <version>2.8.0</version>
</dependency>
```
or Gradle:
```gradle
-implementation 'com.auth0:auth0:2.7.0'
+implementation 'com.auth0:auth0:2.8.0'
```
### Configure the SDK