Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add ErrorInfo to API errors #153

Merged
merged 4 commits into from
Sep 15, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -1,10 +1,13 @@
package com.databricks.sdk.core;

import com.databricks.sdk.core.error.ErrorDetail;
import java.net.ConnectException;
import java.net.SocketException;
import java.net.SocketTimeoutException;
import java.util.Arrays;
import java.util.Collections;
import java.util.List;
import java.util.stream.Collectors;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

Expand All @@ -17,6 +20,7 @@
* unrecoverable way and this exception should be thrown, potentially wrapped in another exception.
*/
public class DatabricksError extends DatabricksException {
private static final String ERROR_INFO_TYPE = "type.googleapis.com/google.rpc.ErrorInfo";
private final Logger LOG = LoggerFactory.getLogger(getClass().getName());

/** Errors returned by Databricks services which are known to be retriable. */
Expand All @@ -40,28 +44,45 @@ public class DatabricksError extends DatabricksException {
private final String errorCode;
private final int statusCode;

private final List<ErrorDetail> details;

public DatabricksError(int statusCode) {
this("", "OK", statusCode, null);
this("", "OK", statusCode, null, Collections.emptyList());
}

public DatabricksError(String errorCode, String message) {
this(errorCode, message, 400, null);
this(errorCode, message, 400, null, Collections.emptyList());
}

public DatabricksError(String errorCode, String message, int statusCode) {
this(errorCode, message, statusCode, null);
this(errorCode, message, statusCode, null, Collections.emptyList());
}

public DatabricksError(String errorCode, int statusCode, Throwable cause) {
this(errorCode, cause.getMessage(), statusCode, cause);
this(errorCode, cause.getMessage(), statusCode, cause, Collections.emptyList());
}

public DatabricksError(
String errorCode, String message, int statusCode, List<ErrorDetail> details) {
this(errorCode, message, statusCode, null, details);
}

private DatabricksError(String errorCode, String message, int statusCode, Throwable cause) {
private DatabricksError(
String errorCode,
String message,
int statusCode,
Throwable cause,
List<ErrorDetail> details) {
super(message, cause);
this.errorCode = errorCode;
this.message = message;
this.cause = cause;
this.statusCode = statusCode;
this.details = details == null ? Collections.emptyList() : details;
}

public List<ErrorDetail> getErrorInfo() {
return this.getDetailsByType(ERROR_INFO_TYPE);
}

public String getErrorCode() {
Expand Down Expand Up @@ -99,6 +120,10 @@ public boolean isRetriable() {
return false;
}

List<ErrorDetail> getDetailsByType(String type) {
return this.details.stream().filter(e -> e.getType().equals(type)).collect(Collectors.toList());
}

private static boolean isCausedBy(Throwable throwable, Class<? extends Throwable> clazz) {
if (throwable == null) {
return false;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

import com.fasterxml.jackson.annotation.JsonIgnoreProperties;
import com.fasterxml.jackson.annotation.JsonProperty;
import java.util.List;

/**
* The union of all JSON error responses from the Databricks APIs, not including HTML responses.
Expand All @@ -20,6 +21,7 @@ public class ApiErrorBody {
private String scimStatus;
private String scimType;
private String api12Error;
private List<ErrorDetail> errorDetails;

public ApiErrorBody() {}

Expand All @@ -29,13 +31,23 @@ public ApiErrorBody(
@JsonProperty("detail") String scimDetail,
@JsonProperty("status") String scimStatus,
@JsonProperty("scimType") String scimType,
@JsonProperty("error") String api12Error) {
@JsonProperty("error") String api12Error,
@JsonProperty("details") List<ErrorDetail> errorDetails) {
this.errorCode = errorCode;
this.message = message;
this.scimDetail = scimDetail;
this.scimStatus = scimStatus;
this.scimType = scimType;
this.api12Error = api12Error;
this.errorDetails = errorDetails;
}

public List<ErrorDetail> getErrorDetails() {
return errorDetails;
}

public void setErrorDetails(List<ErrorDetail> errorDetails) {
this.errorDetails = errorDetails;
}

public String getErrorCode() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
import com.fasterxml.jackson.databind.ObjectMapper;
import java.io.*;
import java.nio.charset.StandardCharsets;
import java.util.Collections;
import java.util.regex.Matcher;
import java.util.regex.Pattern;
import org.apache.commons.io.IOUtils;
Expand Down Expand Up @@ -47,8 +48,14 @@ private static DatabricksError readErrorFromResponse(Response response) {
errorBody.setMessage(message.trim());
errorBody.setErrorCode("SCIM_" + errorBody.getScimStatus());
}
if (errorBody.getErrorDetails() == null) {
errorBody.setErrorDetails(Collections.emptyList());
}
return new DatabricksError(
errorBody.getErrorCode(), errorBody.getMessage(), response.getStatusCode());
errorBody.getErrorCode(),
errorBody.getMessage(),
response.getStatusCode(),
errorBody.getErrorDetails());
}

/**
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
package com.databricks.sdk.core.error;

import com.fasterxml.jackson.annotation.JsonIgnoreProperties;
import com.fasterxml.jackson.annotation.JsonProperty;
import java.util.Collections;
import java.util.Map;

@JsonIgnoreProperties(ignoreUnknown = true)
public class ErrorDetail {

private String type;

private String reason;

private String domain;

private Map<String, String> metadata;

public ErrorDetail() {}

public ErrorDetail(
@JsonProperty("@type") String type,
@JsonProperty("reason") String reason,
@JsonProperty("domain") String domain,
@JsonProperty("metadata") Map<String, String> metadata) {
this.type = type;
this.reason = reason;
this.domain = domain;
this.metadata = Collections.unmodifiableMap(metadata);
}

public String getType() {
return type;
}

public String getReason() {
return reason;
}

public Map<String, String> getMetadata() {
return metadata;
}

public String getDomain() {
return domain;
}

hectorcast-db marked this conversation as resolved.
Show resolved Hide resolved
@Override
public String toString() {
return "ErrorDetails{"
+ "type='"
+ type
+ '\''
+ ", reason='"
+ reason
+ '\''
+ ", domain='"
+ domain
+ '\''
+ ", metadata="
+ metadata
+ '}';
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
import static org.junit.jupiter.api.Assertions.assertThrows;

import com.databricks.sdk.core.error.ApiErrorBody;
import com.databricks.sdk.core.error.ErrorDetail;
import com.databricks.sdk.core.http.Request;
import com.databricks.sdk.core.http.Response;
import com.databricks.sdk.core.utils.FakeTimer;
Expand Down Expand Up @@ -67,14 +68,19 @@ private <T> void runApiClientTest(

private void runFailingApiClientTest(
Request request, List<ResponseProvider> responses, Class<?> clazz, String expectedMessage) {
ApiClient client = getApiClient(request, responses);
DatabricksException exception =
assertThrows(
DatabricksException.class,
() -> client.GET(request.getUri().getPath(), clazz, Collections.emptyMap()));
runFailingApiClientTest(request, responses, clazz, DatabricksException.class);
assertEquals(exception.getMessage(), expectedMessage);
}

private <T extends Throwable> T runFailingApiClientTest(
Request request, List<ResponseProvider> responses, Class<?> clazz, Class<T> exceptionClass) {
ApiClient client = getApiClient(request, responses);
return assertThrows(
exceptionClass,
() -> client.GET(request.getUri().getPath(), clazz, Collections.emptyMap()));
}

private Request getBasicRequest() {
return new Request("GET", "http://my.host/api/my/endpoint");
}
Expand Down Expand Up @@ -173,12 +179,51 @@ void retryDatabricksApi12RetriableError() throws JsonProcessingException {
null,
null,
null,
"Workspace 123 does not have any associated worker environments")),
"Workspace 123 does not have any associated worker environments",
null)),
getSuccessResponse(req)),
MyEndpointResponse.class,
new MyEndpointResponse().setKey("value"));
}

@Test
void errorDetails() throws JsonProcessingException {
Request req = getBasicRequest();

Map<String, String> metadata = new HashMap<>();
metadata.put("etag", "value");
ErrorDetail errorDetails =
new ErrorDetail("type.googleapis.com/google.rpc.ErrorInfo", "reason", "domain", metadata);
ErrorDetail unrelatedDetails =
new ErrorDetail("unrelated", "wrong", "wrongDomain", new HashMap<>());

DatabricksError error =
runFailingApiClientTest(
req,
Arrays.asList(
getTransientError(
req,
401,
new ApiErrorBody(
"ERROR",
null,
null,
null,
null,
null,
Arrays.asList(errorDetails, unrelatedDetails))),
getSuccessResponse(req)),
MyEndpointResponse.class,
DatabricksError.class);
List<ErrorDetail> responseErrors = error.getErrorInfo();
assertEquals(responseErrors.size(), 1);
ErrorDetail responseError = responseErrors.get(0);
assertEquals(errorDetails.getType(), responseError.getType());
assertEquals(errorDetails.getReason(), responseError.getReason());
assertEquals(errorDetails.getMetadata(), responseError.getMetadata());
assertEquals(errorDetails.getDomain(), responseError.getDomain());
}

@Test
void retryDatabricksRetriableError() throws JsonProcessingException {
Request req = getBasicRequest();
Expand All @@ -196,6 +241,7 @@ void retryDatabricksRetriableError() throws JsonProcessingException {
null,
null,
null,
null,
null)),
getSuccessResponse(req)),
MyEndpointResponse.class,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

import com.fasterxml.jackson.core.JsonProcessingException;
import com.fasterxml.jackson.databind.ObjectMapper;
import java.util.*;
import org.junit.jupiter.api.Test;

public class ApiErrorBodyDeserializationSuite {
Expand All @@ -21,13 +22,31 @@ void deserializeErrorResponse() throws JsonProcessingException {
assertEquals(error.getApi12Error(), "theerror");
}

@Test
void deserializeDetailedResponse() throws JsonProcessingException {
ObjectMapper mapper = new ObjectMapper();
String rawResponse =
"{\"error_code\":\"theerrorcode\",\"message\":\"themessage\","
+ "\"details\":["
+ "{\"@type\": \"type.googleapis.com/google.rpc.ErrorInfo\", \"reason\":\"detailreason\", \"domain\":\"detaildomain\",\"metadata\":{\"etag\":\"detailsetag\"}}"
+ "]}";
ApiErrorBody error = mapper.readValue(rawResponse, ApiErrorBody.class);
Map<String, String> metadata = new HashMap<>();
metadata.put("etag", "detailsetag");
ErrorDetail errorDetails = error.getErrorDetails().get(0);
assertEquals(errorDetails.getType(), "type.googleapis.com/google.rpc.ErrorInfo");
assertEquals(errorDetails.getReason(), "detailreason");
assertEquals(errorDetails.getDomain(), "detaildomain");
assertEquals(errorDetails.getMetadata(), metadata);
}

// Test that an ApiErrorBody can be deserialized, even if the response includes unexpected
// parameters.
@Test
void handleUnexpectedFieldsInErrorResponse() throws JsonProcessingException {
ObjectMapper mapper = new ObjectMapper();
String rawResponse =
"{\"error_code\":\"theerrorcode\",\"message\":\"themessage\",\"details\":[\"unexpected\"]}";
"{\"error_code\":\"theerrorcode\",\"message\":\"themessage\",\"unexpectedField\":[\"unexpected\"]}";
ApiErrorBody error = mapper.readValue(rawResponse, ApiErrorBody.class);
assertEquals(error.getErrorCode(), "theerrorcode");
assertEquals(error.getMessage(), "themessage");
Expand Down
Loading