Skip to content

Commit 5929f32

Browse files
authored
Override INVALID_PARAMETER_VALUE on fetching non-existent job/cluster (#257)
## Changes Ports databricks/databricks-sdk-go#864 to the Java SDK. Most services use `RESOURCE_DOES_NOT_EXIST` error code with 404 status code to indicate that a resource doesn't exist. However, for legacy reasons, Jobs and Clusters services use `INVALID_PARAMETER_VALUE` error code with 400 status code instead. This makes tools like Terraform and UCX difficult to maintain, as these services need different error handling logic. However, we can't change these behaviors as customers already depend on the raw HTTP response status & contents. This PR corrects these errors in the SDK itself. SDK users can then do ```java try { BaseJob job = w.jobs().get("123"); } catch (ResourceDoesNotExist e) { ... } ``` just as you would expect from other resources. Updated the README with more information about this as well. ## Tests Added unit tests for error overrides. Added/updated the integration tests for Clusters and Jobs. - [x] `make test` passing - [x] `make fmt` applied - [x] relevant integration tests applied
1 parent b7247b8 commit 5929f32

File tree

12 files changed

+237
-6
lines changed

12 files changed

+237
-6
lines changed

.codegen.json

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,8 @@
1717
"batch": {
1818
".codegen/workspace.java.tmpl": "databricks-sdk-java/src/main/java/com/databricks/sdk/WorkspaceClient.java",
1919
".codegen/account.java.tmpl": "databricks-sdk-java/src/main/java/com/databricks/sdk/AccountClient.java",
20-
".codegen/error-mapper.java.tmpl": "databricks-sdk-java/src/main/java/com/databricks/sdk/core/error/ErrorMapper.java"
20+
".codegen/error-mapper.java.tmpl": "databricks-sdk-java/src/main/java/com/databricks/sdk/core/error/ErrorMapper.java",
21+
".codegen/error-overrides.java.tmpl": "databricks-sdk-java/src/main/java/com/databricks/sdk/core/error/ErrorOverrides.java"
2122
},
2223
"version": {
2324
"pom.xml": "<artifactId>databricks-sdk-parent</artifactId>\n <version>$VERSION</version>",

.codegen/error-overrides.java.tmpl

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
// Code generated from OpenAPI specs by Databricks SDK Generator. DO NOT EDIT.
2+
3+
package com.databricks.sdk.core.error;
4+
5+
import java.util.Arrays;
6+
import java.util.List;
7+
8+
import com.databricks.sdk.support.Generated;
9+
10+
@Generated
11+
class ErrorOverrides {
12+
static final List<ErrorOverride<?>> ALL_OVERRIDES = Arrays.asList(
13+
{{- range $i, $x := .ErrorOverrides }}
14+
{{if not (eq $i 0)}}, {{end}}new ErrorOverride<>(
15+
"{{$x.Name}}",
16+
"{{ replaceAll "\\" "\\\\" $x.PathRegex}}",
17+
"{{$x.Verb}}",
18+
"{{ replaceAll "\\" "\\\\" $x.StatusCodeMatcher}}",
19+
"{{ replaceAll "\\" "\\\\" $x.ErrorCodeMatcher}}",
20+
"{{ replaceAll "\\" "\\\\" $x.MessageMatcher}}",
21+
com.databricks.sdk.core.error.platform.{{$x.OverrideErrorCode.PascalName}}.class)
22+
{{- end}}
23+
);
24+
}

.gitattributes

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
databricks-sdk-java/src/main/java/com/databricks/sdk/AccountClient.java linguist-generated=true
22
databricks-sdk-java/src/main/java/com/databricks/sdk/WorkspaceClient.java linguist-generated=true
33
databricks-sdk-java/src/main/java/com/databricks/sdk/core/error/ErrorMapper.java linguist-generated=true
4+
databricks-sdk-java/src/main/java/com/databricks/sdk/core/error/ErrorOverrides.java linguist-generated=true
45
databricks-sdk-java/src/main/java/com/databricks/sdk/core/error/platform/Aborted.java linguist-generated=true
56
databricks-sdk-java/src/main/java/com/databricks/sdk/core/error/platform/AlreadyExists.java linguist-generated=true
67
databricks-sdk-java/src/main/java/com/databricks/sdk/core/error/platform/BadRequest.java linguist-generated=true

README.md

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ The Databricks SDK for Java includes functionality to accelerate development wit
1414
- [Long-running operations](#long-running-operations)
1515
- [Paginated responses](#paginated-responses)
1616
- [Single-sign-on with OAuth](#single-sign-on-sso-with-oauth)
17+
- [Error handling](#error-handling)
1718
- [Logging](#logging)
1819
- [Interface stability](#interface-stability)
1920
- [Disclaimer](#disclaimer)
@@ -357,6 +358,30 @@ For applications, that do run on developer workstations, Databricks SDK for Java
357358

358359
In order to use OAuth with Databricks SDK for Python, you should use `AccountClient.customAppIntegration().create()` API. Usage of this can be seen in the [Spring Boot example project](/examples/spring-boot-oauth-u2m-demo/src/main/java/com/databricks/sdk/App.java).
359360

361+
## Error Handling
362+
The Databricks SDK for Java provides a robust error-handling mechanism that allows developers to catch and handle API errors. When an error occurs, the SDK will raise an exception that contains information about the error, such as the HTTP status code, error message, and error details. Developers can catch these exceptions and handle them appropriately in their code.
363+
364+
```java
365+
import com.databricks.sdk.WorkspaceClient;
366+
import com.databricks.sdk.core.errors.platform.ResourceDoesNotExist;
367+
import com.databricks.sdk.service.compute.ClusterDetails;
368+
369+
public class ErrorDemo {
370+
public static void main(String[] args) {
371+
WorkspaceClient w = new WorkspaceClient();
372+
try {
373+
ClusterDetails c = w.clusters().get("1234-5678-9012");
374+
} catch (ResourceDoesNotExist e) {
375+
System.out.println("Cluster not found: " + e.getMessage());
376+
}
377+
}
378+
}
379+
```
380+
381+
The SDK handles inconsistencies in error responses amongst the different services, providing a consistent interface for developers to work with. Simply catch the appropriate exception type and handle the error as needed. The errors returned by the Databricks API are defined in [databricks-sdk-java/src/main/java/com/databricks/sdk/core/error/platform](https://github.com/databricks/databricks-sdk-java/tree/main/databricks-sdk-java/src/main/java/com/databricks/sdk/core/error/platform).
382+
383+
384+
360385
## Logging
361386

362387
The Databricks SDK for Java seamlessly integrates with the standard [SLF4J logging framework](https://www.slf4j.org/). This allows developers to easily enable and customize logging for their Databricks Java projects. To enable debug logging in your Databricks java project, you can add the following to your log4j.properties file:

databricks-sdk-java/src/main/java/com/databricks/sdk/core/error/AbstractErrorMapper.java

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,16 @@
11
package com.databricks.sdk.core.error;
22

33
import com.databricks.sdk.core.DatabricksError;
4+
import com.databricks.sdk.core.http.Response;
45
import java.util.HashMap;
56
import java.util.List;
67
import java.util.Map;
8+
import org.slf4j.Logger;
9+
import org.slf4j.LoggerFactory;
710

811
abstract class AbstractErrorMapper {
12+
private static final Logger LOG = LoggerFactory.getLogger(AbstractErrorMapper.class);
13+
914
@FunctionalInterface
1015
protected interface ErrorCodeRule {
1116
DatabricksError create(String message, List<ErrorDetail> details);
@@ -16,7 +21,18 @@ protected interface StatusCodeRule {
1621
DatabricksError create(String errorCode, String message, List<ErrorDetail> details);
1722
}
1823

19-
public DatabricksError apply(int code, ApiErrorBody errorBody) {
24+
public DatabricksError apply(Response resp, ApiErrorBody errorBody) {
25+
for (ErrorOverride<?> override : ErrorOverrides.ALL_OVERRIDES) {
26+
if (override.matches(errorBody, resp)) {
27+
LOG.debug(
28+
"Overriding error with {} (original status code: {}, original error code: {})",
29+
override.getDebugName(),
30+
resp.getStatusCode(),
31+
errorBody.getErrorCode());
32+
return override.makeError(errorBody);
33+
}
34+
}
35+
int code = resp.getStatusCode();
2036
String message = errorBody.getMessage();
2137
String errorCode = errorBody.getErrorCode();
2238
List<ErrorDetail> details = errorBody.getErrorDetails();

databricks-sdk-java/src/main/java/com/databricks/sdk/core/error/ApiErrors.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,7 @@ private static DatabricksError readErrorFromResponse(Response response) {
5252
if (errorBody.getErrorDetails() == null) {
5353
errorBody.setErrorDetails(Collections.emptyList());
5454
}
55-
return ERROR_MAPPER.apply(response.getStatusCode(), errorBody);
55+
return ERROR_MAPPER.apply(response, errorBody);
5656
}
5757

5858
/**
Lines changed: 90 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,90 @@
1+
package com.databricks.sdk.core.error;
2+
3+
import com.databricks.sdk.core.DatabricksError;
4+
import com.databricks.sdk.core.DatabricksException;
5+
import com.databricks.sdk.core.http.Response;
6+
import java.lang.reflect.Constructor;
7+
import java.util.List;
8+
import java.util.regex.Pattern;
9+
10+
public class ErrorOverride<T extends DatabricksError> {
11+
private final String debugName;
12+
private final Pattern pathRegex;
13+
private final String verb;
14+
private final Pattern statusCodeMatcher;
15+
private final Pattern errorCodeMatcher;
16+
private final Pattern messageMatcher;
17+
private final Class<T> customError;
18+
19+
public ErrorOverride(
20+
String debugName,
21+
String pathRegex,
22+
String verb,
23+
String statusCodeMatcher,
24+
String errorCodeMatcher,
25+
String messageMatcher,
26+
Class<T> customError) {
27+
this.debugName = debugName;
28+
this.pathRegex = ErrorOverride.compilePattern(pathRegex);
29+
this.verb = verb;
30+
this.statusCodeMatcher = ErrorOverride.compilePattern(statusCodeMatcher);
31+
this.errorCodeMatcher = ErrorOverride.compilePattern(errorCodeMatcher);
32+
this.messageMatcher = ErrorOverride.compilePattern(messageMatcher);
33+
this.customError = customError;
34+
}
35+
36+
public boolean matches(ApiErrorBody body, Response resp) {
37+
if (!resp.getRequest().getMethod().equals(this.verb)) {
38+
return false;
39+
}
40+
41+
if (this.pathRegex != null
42+
&& !this.pathRegex.matcher(resp.getRequest().getUri().getPath()).matches()) {
43+
return false;
44+
}
45+
String statusCode = Integer.toString(resp.getStatusCode());
46+
if (this.statusCodeMatcher != null && !this.statusCodeMatcher.matcher(statusCode).matches()) {
47+
return false;
48+
}
49+
if (this.errorCodeMatcher != null
50+
&& !this.errorCodeMatcher.matcher(body.getErrorCode()).matches()) {
51+
return false;
52+
}
53+
// Allow matching substring of the error message.
54+
if (this.messageMatcher != null && !this.messageMatcher.matcher(body.getMessage()).find()) {
55+
return false;
56+
}
57+
return true;
58+
}
59+
60+
public String getDebugName() {
61+
return this.debugName;
62+
}
63+
64+
public T makeError(ApiErrorBody body) {
65+
Constructor<?>[] constructors = this.customError.getConstructors();
66+
for (Constructor<?> constructor : constructors) {
67+
Class<?>[] parameterTypes = constructor.getParameterTypes();
68+
// All errors have a 2-argument constructor for the message and the error body.
69+
if (parameterTypes.length == 2
70+
&& parameterTypes[0].equals(String.class)
71+
&& parameterTypes[1].equals(List.class)) {
72+
try {
73+
return (T) constructor.newInstance(body.getMessage(), body.getErrorDetails());
74+
} catch (Exception e) {
75+
throw new DatabricksException(
76+
"Error creating custom error for error type " + this.customError.getName(), e);
77+
}
78+
}
79+
}
80+
throw new DatabricksException(
81+
"No suitable constructor found for error type " + this.customError.getName());
82+
}
83+
84+
private static Pattern compilePattern(String pattern) {
85+
if (pattern == null || pattern.isEmpty()) {
86+
return null;
87+
}
88+
return Pattern.compile(pattern);
89+
}
90+
}

databricks-sdk-java/src/main/java/com/databricks/sdk/core/error/ErrorOverrides.java

Lines changed: 29 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

databricks-sdk-java/src/main/java/com/databricks/sdk/service/sql/ChannelName.java

Lines changed: 1 addition & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

databricks-sdk-java/src/test/java/com/databricks/sdk/core/error/ErrorMapperTest.java

Lines changed: 33 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,8 @@
33
import com.databricks.sdk.VariableSource;
44
import com.databricks.sdk.core.DatabricksError;
55
import com.databricks.sdk.core.error.platform.*;
6+
import com.databricks.sdk.core.http.Request;
7+
import com.databricks.sdk.core.http.Response;
68
import com.fasterxml.jackson.core.JsonProcessingException;
79
import com.fasterxml.jackson.databind.ObjectMapper;
810
import java.util.stream.Stream;
@@ -99,7 +101,37 @@ void applyMapsErrorsCorrectly(Class<?> expectedClass, int statusCode, String err
99101
throws JsonProcessingException {
100102
ErrorMapper mapper = new ErrorMapper();
101103
ApiErrorBody apiErrorBody = new ObjectMapper().readValue(errorBody, ApiErrorBody.class);
102-
DatabricksError error = mapper.apply(statusCode, apiErrorBody);
104+
Request req = new Request("GET", "/a/b/c");
105+
Response resp = new Response(req, statusCode, null, null);
106+
DatabricksError error = mapper.apply(resp, apiErrorBody);
103107
assert error.getClass().equals(expectedClass);
104108
}
109+
110+
static final Stream<Arguments> overrideCases =
111+
Stream.of(
112+
Arguments.of(
113+
ResourceDoesNotExist.class,
114+
"GET",
115+
"https://my.databricks.workspace/api/2.0/clusters/get?cluster_id=123",
116+
400,
117+
"{\"error_code\":\"INVALID_PARAMETER_VALUE\",\"message\":\"Cluster 123 does not exist\"}"),
118+
Arguments.of(
119+
ResourceDoesNotExist.class,
120+
"GET",
121+
"https://my.databricks.workspace/api/2.1/jobs/get?job_id=123",
122+
400,
123+
"{\"error_code\":\"INVALID_PARAMETER_VALUE\",\"message\":\"Job 123 does not exist\"}"));
124+
125+
@ParameterizedTest
126+
@VariableSource("overrideCases")
127+
void applyOverridesErrorsCorrectly(
128+
Class<?> expected, String method, String url, int statusCode, String errorBody)
129+
throws JsonProcessingException {
130+
ErrorMapper mapper = new ErrorMapper();
131+
ApiErrorBody apiErrorBody = new ObjectMapper().readValue(errorBody, ApiErrorBody.class);
132+
Request req = new Request(method, url);
133+
Response resp = new Response(req, statusCode, null, null);
134+
DatabricksError error = mapper.apply(resp, apiErrorBody);
135+
assert error.getClass().equals(expected);
136+
}
105137
}

0 commit comments

Comments
 (0)