Skip to content

Commit 14e4c63

Browse files
tom-s-powelliamthomaspowellrenaudhartert-db
authored
Capture DatabricksError when retrying API calls (#427)
## What changes are proposed in this pull request? When requests are retried, there is currently no information available to the caller to understand why. On retries no exception is logged and on final retry, the cause is only available if an `IOException` were the original source of failure rather. `DatabricksError` feels like a more useful error to surface as this will capture the error from the server which would have evaluated into a retriable error. Currently the stacktrace appears as follows, which doesn't provide much information: ``` com.databricks.sdk.core.DatabricksException: Request GET /api/2.1/unity-catalog/tables?catalog_name=<REDACTED>&schema_name=<REDACTED> failed after 4 retries \tat com.databricks.sdk.core.ApiClient.executeInner(ApiClient.java:282) \tat com.databricks.sdk.core.ApiClient.getResponse(ApiClient.java:235) \tat com.databricks.sdk.core.ApiClient.execute(ApiClient.java:227) \tat com.databricks.sdk.core.ApiClient.GET(ApiClient.java:148) \tat com.databricks.sdk.service.catalog.TablesImpl.list(TablesImpl.java:47) \tat com.databricks.sdk.support.Paginator.flipNextPage(Paginator.java:58) \tat com.databricks.sdk.support.Paginator.<init>(Paginator.java:51) \tat com.databricks.sdk.service.catalog.TablesAPI.list(TablesAPI.java:102) \tat com.databricks.sdk.service.catalog.TablesAPI.list(TablesAPI.java:89) ``` ## How is this tested? Unit tests added. Signed-off-by: Thomas Powell <tpowell@palantir.com> Co-authored-by: Thomas Powell <tpowell@palantir.com> Co-authored-by: Renaud Hartert <renaud.hartert@databricks.com>
1 parent efe5aa4 commit 14e4c63

File tree

3 files changed

+23
-13
lines changed

3 files changed

+23
-13
lines changed

NEXT_CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,5 +9,6 @@
99
### Documentation
1010

1111
### Internal Changes
12+
* Capture DatabricksError when retrying API calls ([#427](https://github.com/databricks/databricks-sdk-java/pull/427)).
1213

1314
### API Changes

databricks-sdk-java/src/main/java/com/databricks/sdk/core/ApiClient.java

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -247,12 +247,13 @@ private Response executeInner(Request in, String path) {
247247
}
248248
if (attemptNumber == maxAttempts) {
249249
throw new DatabricksException(
250-
String.format("Request %s failed after %d retries", in, maxAttempts), err);
250+
String.format("Request %s failed after %d retries", in, maxAttempts), databricksError);
251251
}
252252

253253
// Retry after a backoff.
254254
long sleepMillis = getBackoffMillis(out, attemptNumber);
255-
LOG.debug(String.format("Retry %s in %dms", in.getRequestLine(), sleepMillis));
255+
LOG.debug(
256+
String.format("Retry %s in %dms", in.getRequestLine(), sleepMillis), databricksError);
256257
try {
257258
timer.sleep(sleepMillis);
258259
} catch (InterruptedException ex) {

databricks-sdk-java/src/test/java/com/databricks/sdk/core/ApiClientTest.java

Lines changed: 19 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111
import com.fasterxml.jackson.annotation.JsonProperty;
1212
import com.fasterxml.jackson.core.JsonProcessingException;
1313
import com.fasterxml.jackson.databind.ObjectMapper;
14+
import com.google.errorprone.annotations.CanIgnoreReturnValue;
1415
import java.io.IOException;
1516
import java.net.MalformedURLException;
1617
import java.net.URL;
@@ -88,12 +89,14 @@ private <T> void runApiClientTest(
8889
runApiClientTest(client, request, clazz, expectedResponse);
8990
}
9091

91-
private void runFailingApiClientTest(
92+
@CanIgnoreReturnValue
93+
private DatabricksException runFailingApiClientTest(
9294
Request request, List<ResponseProvider> responses, Class<?> clazz, String expectedMessage)
9395
throws IOException {
9496
DatabricksException exception =
9597
runFailingApiClientTest(request, responses, clazz, DatabricksException.class);
9698
assertEquals(exception.getMessage(), expectedMessage);
99+
return exception;
97100
}
98101

99102
private <T extends Throwable> T runFailingApiClientTest(
@@ -217,16 +220,21 @@ void retry429() throws IOException {
217220
@Test
218221
void failAfterTooManyRetries() throws IOException {
219222
Request req = getBasicRequest();
220-
runFailingApiClientTest(
221-
req,
222-
Arrays.asList(
223-
getTooManyRequestsResponseWithRetryAfterDateHeader(req),
224-
getTooManyRequestsResponse(req),
225-
getTooManyRequestsResponse(req),
226-
getTooManyRequestsResponse(req),
227-
getSuccessResponse(req)),
228-
MyEndpointResponse.class,
229-
"Request GET /api/my/endpoint failed after 4 retries");
223+
DatabricksException exception =
224+
runFailingApiClientTest(
225+
req,
226+
Arrays.asList(
227+
getTooManyRequestsResponseWithRetryAfterDateHeader(req),
228+
getTooManyRequestsResponse(req),
229+
getTooManyRequestsResponse(req),
230+
getTooManyRequestsResponse(req),
231+
getSuccessResponse(req)),
232+
MyEndpointResponse.class,
233+
"Request GET /api/my/endpoint failed after 4 retries");
234+
assertInstanceOf(DatabricksError.class, exception.getCause());
235+
DatabricksError cause = (DatabricksError) exception.getCause();
236+
assertEquals(cause.getErrorCode(), "TOO_MANY_REQUESTS");
237+
assertEquals(cause.getStatusCode(), 429);
230238
}
231239

232240
@Test

0 commit comments

Comments
 (0)