Skip to content

Commit cbd40f3

Browse files
Fix error type for non-JSON error responses
When the server returns a plain-text error (e.g. "Invalid Token" with HTTP 403), parseUnknownError set the error code to "UNKNOWN", which matched ErrorMapper's errorCodeMapping and short-circuited the status code mapping. This caused all non-JSON errors to produce Unknown instead of the correct typed exception (PermissionDenied, Unauthenticated, etc.). The root cause was that parseUnknownError assumed response.getStatus() returned "403 Forbidden" but it only returns the reason phrase ("Forbidden"). The split-on-space logic always fell into the default "UNKNOWN" branch. The fix removes the error code derivation entirely, leaving errorCode null so AbstractErrorMapper falls through to the status code mapping. The error message now uses the raw response body instead of appending Jackson parse exception details. Co-authored-by: Isaac
1 parent 4d195ac commit cbd40f3

2 files changed

Lines changed: 68 additions & 13 deletions

File tree

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

Lines changed: 3 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -116,27 +116,17 @@ private static Optional<ApiErrorBody> parseApiError(Response response) {
116116
try {
117117
return Optional.of(MAPPER.readValue(body, ApiErrorBody.class));
118118
} catch (IOException e) {
119-
return Optional.of(parseUnknownError(response, body, e));
119+
return Optional.of(parseUnknownError(body));
120120
}
121121
}
122122

123-
private static ApiErrorBody parseUnknownError(Response response, String body, IOException err) {
123+
private static ApiErrorBody parseUnknownError(String body) {
124124
ApiErrorBody errorBody = new ApiErrorBody();
125-
String[] statusParts = response.getStatus().split(" ", 2);
126-
if (statusParts.length < 2) {
127-
errorBody.setErrorCode("UNKNOWN");
128-
} else {
129-
String errorCode = statusParts[1].replaceAll("^[ .]+|[ .]+$", "");
130-
errorBody.setErrorCode(errorCode.replaceAll(" ", "_").toUpperCase());
131-
}
132-
133125
Matcher messageMatcher = HTML_ERROR_REGEX.matcher(body);
134126
if (messageMatcher.find()) {
135127
errorBody.setMessage(messageMatcher.group(1).replaceAll("^[ .]+|[ .]+$", ""));
136128
} else {
137-
errorBody.setMessage(
138-
String.format(
139-
"Response from server (%s) %s: %s", response.getStatus(), body, err.getMessage()));
129+
errorBody.setMessage(body);
140130
}
141131
return errorBody;
142132
}
Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,65 @@
1+
package com.databricks.sdk.core.error;
2+
3+
import static org.junit.jupiter.api.Assertions.*;
4+
5+
import com.databricks.sdk.core.DatabricksError;
6+
import com.databricks.sdk.core.error.platform.*;
7+
import com.databricks.sdk.core.http.Request;
8+
import com.databricks.sdk.core.http.Response;
9+
import java.util.Collections;
10+
import org.junit.jupiter.api.Test;
11+
12+
class PlainTextErrorTest {
13+
14+
@Test
15+
void plainTextForbiddenReturnsPermissionDenied() {
16+
DatabricksError error = getError(403, "Forbidden", "Invalid Token");
17+
assertInstanceOf(PermissionDenied.class, error);
18+
assertEquals("Invalid Token", error.getMessage());
19+
}
20+
21+
@Test
22+
void plainTextUnauthorizedReturnsUnauthenticated() {
23+
DatabricksError error = getError(401, "Unauthorized", "Bad credentials");
24+
assertInstanceOf(Unauthenticated.class, error);
25+
assertEquals("Bad credentials", error.getMessage());
26+
}
27+
28+
@Test
29+
void plainTextNotFoundReturnsNotFound() {
30+
DatabricksError error = getError(404, "Not Found", "no such endpoint");
31+
assertInstanceOf(NotFound.class, error);
32+
assertEquals("no such endpoint", error.getMessage());
33+
}
34+
35+
@Test
36+
void htmlErrorExtractsPreContent() {
37+
String html = "<html><body><pre>some error message</pre></body></html>";
38+
DatabricksError error = getError(403, "Forbidden", html);
39+
assertInstanceOf(PermissionDenied.class, error);
40+
assertEquals("some error message", error.getMessage());
41+
}
42+
43+
@Test
44+
void emptyBodyFallsBackToStatusCode() {
45+
Request request = new Request("GET", "https://example.com/api/2.0/clusters/get");
46+
Response response = new Response(request, 403, "Forbidden", Collections.emptyMap(), "");
47+
DatabricksError error = ApiErrors.getDatabricksError(response);
48+
assertInstanceOf(PermissionDenied.class, error);
49+
}
50+
51+
@Test
52+
void nullBodyFallsBackToStatusCode() {
53+
Request request = new Request("GET", "https://example.com/api/2.0/clusters/get");
54+
Response response =
55+
new Response(request, 403, "Forbidden", Collections.emptyMap(), (String) null);
56+
DatabricksError error = ApiErrors.getDatabricksError(response);
57+
assertInstanceOf(PermissionDenied.class, error);
58+
}
59+
60+
private static DatabricksError getError(int statusCode, String status, String body) {
61+
Request request = new Request("GET", "https://example.com/api/2.0/clusters/get");
62+
Response response = new Response(request, statusCode, status, Collections.emptyMap(), body);
63+
return ApiErrors.getDatabricksError(response);
64+
}
65+
}

0 commit comments

Comments
 (0)