Override INVALID_PARAMETER_VALUE on fetching non-existent job/cluster (…

…databricks#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

.codegen.json

@@ -17,7 +17,8 @@
   "batch": {
     ".codegen/workspace.java.tmpl": "databricks-sdk-java/src/main/java/com/databricks/sdk/WorkspaceClient.java",
     ".codegen/account.java.tmpl": "databricks-sdk-java/src/main/java/com/databricks/sdk/AccountClient.java",
-    ".codegen/error-mapper.java.tmpl": "databricks-sdk-java/src/main/java/com/databricks/sdk/core/error/ErrorMapper.java"
+    ".codegen/error-mapper.java.tmpl": "databricks-sdk-java/src/main/java/com/databricks/sdk/core/error/ErrorMapper.java",
+    ".codegen/error-overrides.java.tmpl": "databricks-sdk-java/src/main/java/com/databricks/sdk/core/error/ErrorOverrides.java"
   },
   "version": {
     "pom.xml": "<artifactId>databricks-sdk-parent</artifactId>\n  <version>$VERSION</version>",

.codegen/error-overrides.java.tmpl

@@ -0,0 +1,24 @@
+// Code generated from OpenAPI specs by Databricks SDK Generator. DO NOT EDIT.
+
+package com.databricks.sdk.core.error;
+
+import java.util.Arrays;
+import java.util.List;
+
+import com.databricks.sdk.support.Generated;
+
+@Generated
+class ErrorOverrides {
+  static final List<ErrorOverride<?>> ALL_OVERRIDES = Arrays.asList(
+{{- range $i, $x := .ErrorOverrides }}
+    {{if not (eq $i 0)}}, {{end}}new ErrorOverride<>(
+        "{{$x.Name}}",
+        "{{ replaceAll "\\" "\\\\" $x.PathRegex}}",
+        "{{$x.Verb}}",
+        "{{ replaceAll "\\" "\\\\" $x.StatusCodeMatcher}}",
+        "{{ replaceAll "\\" "\\\\" $x.ErrorCodeMatcher}}",
+        "{{ replaceAll "\\" "\\\\" $x.MessageMatcher}}",
+        com.databricks.sdk.core.error.platform.{{$x.OverrideErrorCode.PascalName}}.class)
+{{- end}}
+  );
+}

.gitattributes

@@ -1,6 +1,7 @@
 databricks-sdk-java/src/main/java/com/databricks/sdk/AccountClient.java linguist-generated=true
 databricks-sdk-java/src/main/java/com/databricks/sdk/WorkspaceClient.java linguist-generated=true
 databricks-sdk-java/src/main/java/com/databricks/sdk/core/error/ErrorMapper.java linguist-generated=true
+databricks-sdk-java/src/main/java/com/databricks/sdk/core/error/ErrorOverrides.java linguist-generated=true
 databricks-sdk-java/src/main/java/com/databricks/sdk/core/error/platform/Aborted.java linguist-generated=true
 databricks-sdk-java/src/main/java/com/databricks/sdk/core/error/platform/AlreadyExists.java linguist-generated=true
 databricks-sdk-java/src/main/java/com/databricks/sdk/core/error/platform/BadRequest.java linguist-generated=true

README.md

@@ -14,6 +14,7 @@ The Databricks SDK for Java includes functionality to accelerate development wit
 - [Long-running operations](#long-running-operations)
 - [Paginated responses](#paginated-responses)
 - [Single-sign-on with OAuth](#single-sign-on-sso-with-oauth)
+- [Error handling](#error-handling)
 - [Logging](#logging)
 - [Interface stability](#interface-stability)
 - [Disclaimer](#disclaimer)
@@ -357,6 +358,30 @@ For applications, that do run on developer workstations, Databricks SDK for Java
 
 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).
 
+## Error Handling
+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.
+
+```java
+import com.databricks.sdk.WorkspaceClient;
+import com.databricks.sdk.core.errors.platform.ResourceDoesNotExist;
+import com.databricks.sdk.service.compute.ClusterDetails;
+
+public class ErrorDemo {
+    public static void main(String[] args) {
+        WorkspaceClient w = new WorkspaceClient();
+        try {
+            ClusterDetails c = w.clusters().get("1234-5678-9012");
+        } catch (ResourceDoesNotExist e) {
+          System.out.println("Cluster not found: " + e.getMessage());
+        }
+    }
+}
+```
+
+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).
+
+
+
 ## Logging
 
 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

@@ -1,11 +1,16 @@
 package com.databricks.sdk.core.error;
 
 import com.databricks.sdk.core.DatabricksError;
+import com.databricks.sdk.core.http.Response;
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 
 abstract class AbstractErrorMapper {
+  private static final Logger LOG = LoggerFactory.getLogger(AbstractErrorMapper.class);
+
   @FunctionalInterface
   protected interface ErrorCodeRule {
     DatabricksError create(String message, List<ErrorDetail> details);
@@ -16,7 +21,18 @@ protected interface StatusCodeRule {
     DatabricksError create(String errorCode, String message, List<ErrorDetail> details);
   }
 
-  public DatabricksError apply(int code, ApiErrorBody errorBody) {
+  public DatabricksError apply(Response resp, ApiErrorBody errorBody) {
+    for (ErrorOverride<?> override : ErrorOverrides.ALL_OVERRIDES) {
+      if (override.matches(errorBody, resp)) {
+        LOG.debug(
+            "Overriding error with {} (original status code: {}, original error code: {})",
+            override.getDebugName(),
+            resp.getStatusCode(),
+            errorBody.getErrorCode());
+        return override.makeError(errorBody);
+      }
+    }
+    int code = resp.getStatusCode();
     String message = errorBody.getMessage();
     String errorCode = errorBody.getErrorCode();
     List<ErrorDetail> details = errorBody.getErrorDetails();

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

@@ -52,7 +52,7 @@ private static DatabricksError readErrorFromResponse(Response response) {
     if (errorBody.getErrorDetails() == null) {
       errorBody.setErrorDetails(Collections.emptyList());
     }
-    return ERROR_MAPPER.apply(response.getStatusCode(), errorBody);
+    return ERROR_MAPPER.apply(response, errorBody);
   }
 
   /**

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

@@ -0,0 +1,90 @@
+package com.databricks.sdk.core.error;
+
+import com.databricks.sdk.core.DatabricksError;
+import com.databricks.sdk.core.DatabricksException;
+import com.databricks.sdk.core.http.Response;
+import java.lang.reflect.Constructor;
+import java.util.List;
+import java.util.regex.Pattern;
+
+public class ErrorOverride<T extends DatabricksError> {
+  private final String debugName;
+  private final Pattern pathRegex;
+  private final String verb;
+  private final Pattern statusCodeMatcher;
+  private final Pattern errorCodeMatcher;
+  private final Pattern messageMatcher;
+  private final Class<T> customError;
+
+  public ErrorOverride(
+      String debugName,
+      String pathRegex,
+      String verb,
+      String statusCodeMatcher,
+      String errorCodeMatcher,
+      String messageMatcher,
+      Class<T> customError) {
+    this.debugName = debugName;
+    this.pathRegex = ErrorOverride.compilePattern(pathRegex);
+    this.verb = verb;
+    this.statusCodeMatcher = ErrorOverride.compilePattern(statusCodeMatcher);
+    this.errorCodeMatcher = ErrorOverride.compilePattern(errorCodeMatcher);
+    this.messageMatcher = ErrorOverride.compilePattern(messageMatcher);
+    this.customError = customError;
+  }
+
+  public boolean matches(ApiErrorBody body, Response resp) {
+    if (!resp.getRequest().getMethod().equals(this.verb)) {
+      return false;
+    }
+
+    if (this.pathRegex != null
+        && !this.pathRegex.matcher(resp.getRequest().getUri().getPath()).matches()) {
+      return false;
+    }
+    String statusCode = Integer.toString(resp.getStatusCode());
+    if (this.statusCodeMatcher != null && !this.statusCodeMatcher.matcher(statusCode).matches()) {
+      return false;
+    }
+    if (this.errorCodeMatcher != null
+        && !this.errorCodeMatcher.matcher(body.getErrorCode()).matches()) {
+      return false;
+    }
+    // Allow matching substring of the error message.
+    if (this.messageMatcher != null && !this.messageMatcher.matcher(body.getMessage()).find()) {
+      return false;
+    }
+    return true;
+  }
+
+  public String getDebugName() {
+    return this.debugName;
+  }
+
+  public T makeError(ApiErrorBody body) {
+    Constructor<?>[] constructors = this.customError.getConstructors();
+    for (Constructor<?> constructor : constructors) {
+      Class<?>[] parameterTypes = constructor.getParameterTypes();
+      // All errors have a 2-argument constructor for the message and the error body.
+      if (parameterTypes.length == 2
+          && parameterTypes[0].equals(String.class)
+          && parameterTypes[1].equals(List.class)) {
+        try {
+          return (T) constructor.newInstance(body.getMessage(), body.getErrorDetails());
+        } catch (Exception e) {
+          throw new DatabricksException(
+              "Error creating custom error for error type " + this.customError.getName(), e);
+        }
+      }
+    }
+    throw new DatabricksException(
+        "No suitable constructor found for error type " + this.customError.getName());
+  }
+
+  private static Pattern compilePattern(String pattern) {
+    if (pattern == null || pattern.isEmpty()) {
+      return null;
+    }
+    return Pattern.compile(pattern);
+  }
+}

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

@@ -3,6 +3,8 @@
 import com.databricks.sdk.VariableSource;
 import com.databricks.sdk.core.DatabricksError;
 import com.databricks.sdk.core.error.platform.*;
+import com.databricks.sdk.core.http.Request;
+import com.databricks.sdk.core.http.Response;
 import com.fasterxml.jackson.core.JsonProcessingException;
 import com.fasterxml.jackson.databind.ObjectMapper;
 import java.util.stream.Stream;
@@ -99,7 +101,37 @@ void applyMapsErrorsCorrectly(Class<?> expectedClass, int statusCode, String err
       throws JsonProcessingException {
     ErrorMapper mapper = new ErrorMapper();
     ApiErrorBody apiErrorBody = new ObjectMapper().readValue(errorBody, ApiErrorBody.class);
-    DatabricksError error = mapper.apply(statusCode, apiErrorBody);
+    Request req = new Request("GET", "/a/b/c");
+    Response resp = new Response(req, statusCode, null, null);
+    DatabricksError error = mapper.apply(resp, apiErrorBody);
     assert error.getClass().equals(expectedClass);
   }
+
+  static final Stream<Arguments> overrideCases =
+      Stream.of(
+          Arguments.of(
+              ResourceDoesNotExist.class,
+              "GET",
+              "https://my.databricks.workspace/api/2.0/clusters/get?cluster_id=123",
+              400,
+              "{\"error_code\":\"INVALID_PARAMETER_VALUE\",\"message\":\"Cluster 123 does not exist\"}"),
+          Arguments.of(
+              ResourceDoesNotExist.class,
+              "GET",
+              "https://my.databricks.workspace/api/2.1/jobs/get?job_id=123",
+              400,
+              "{\"error_code\":\"INVALID_PARAMETER_VALUE\",\"message\":\"Job 123 does not exist\"}"));
+
+  @ParameterizedTest
+  @VariableSource("overrideCases")
+  void applyOverridesErrorsCorrectly(
+      Class<?> expected, String method, String url, int statusCode, String errorBody)
+      throws JsonProcessingException {
+    ErrorMapper mapper = new ErrorMapper();
+    ApiErrorBody apiErrorBody = new ObjectMapper().readValue(errorBody, ApiErrorBody.class);
+    Request req = new Request(method, url);
+    Response resp = new Response(req, statusCode, null, null);
+    DatabricksError error = mapper.apply(resp, apiErrorBody);
+    assert error.getClass().equals(expected);
+  }
 }

databricks-sdk-java/src/test/java/com/databricks/sdk/integration/ClustersIT.java

@@ -3,7 +3,7 @@
 import static org.junit.jupiter.api.Assertions.*;
 
 import com.databricks.sdk.WorkspaceClient;
-import com.databricks.sdk.core.error.platform.InvalidParameterValue;
+import com.databricks.sdk.core.error.platform.ResourceDoesNotExist;
 import com.databricks.sdk.integration.framework.CollectionUtils;
 import com.databricks.sdk.integration.framework.EnvContext;
 import com.databricks.sdk.integration.framework.EnvOrSkip;
@@ -52,7 +52,7 @@ void latestRuntime(WorkspaceClient w) {
   @Test
   void clusterDoesNotExist(WorkspaceClient w) {
     assertThrowsExactly(
-        InvalidParameterValue.class,
+        ResourceDoesNotExist.class,
         () -> {
           w.clusters().get("does-not-exist");
         });

databricks-sdk-java/src/test/java/com/databricks/sdk/integration/JobsIT.java

@@ -1,6 +1,9 @@
 package com.databricks.sdk.integration;
 
+import static org.junit.jupiter.api.Assertions.*;
+
 import com.databricks.sdk.WorkspaceClient;
+import com.databricks.sdk.core.error.platform.ResourceDoesNotExist;
 import com.databricks.sdk.integration.framework.CollectionUtils;
 import com.databricks.sdk.integration.framework.EnvContext;
 import com.databricks.sdk.integration.framework.EnvTest;
@@ -20,4 +23,13 @@ void listsJobs(WorkspaceClient w) {
 
     CollectionUtils.assertUnique(all);
   }
+
+  @Test
+  void getNonExistingJob(WorkspaceClient w) {
+    assertThrowsExactly(
+        ResourceDoesNotExist.class,
+        () -> {
+          w.jobs().get(123456789);
+        });
+  }
 }