Skip to content

Commit b2af201

Browse files
gopalldbclaude
andauthored
Fix SEA result expiry error handling — preserve server error message (#1302)
## Summary When query results expire after the retention period, the SEA server returns HTTP 404 with a descriptive `NOT_FOUND: The result of the statement <id> was not found.` message. The SDK client throws `DatabricksError` (extends `RuntimeException`) which was not caught by `getResultChunks`/`getResultChunksData` — they only caught `IOException`. This caused three problems: 1. **Server error message lost**: `DatabricksError` escaped uncaught, and the `ChunkDownloadTask` finally block created a new exception without chaining the original cause — customer sees `"Download failed for chunk index N"` with no indication of why 2. **Download tasks hang indefinitely**: In `ChunkLinkDownloadService`, the uncaught `RuntimeException` left link futures unresolved, blocking download tasks forever on `future.get()` 3. **Inconsistent with other methods**: `createSession` already catches `DatabricksError` properly — `getResultChunks`/`getResultChunksData` should follow the same pattern ## Changes - **`DatabricksSdkClient.java`**: Add `catch (DatabricksError e)` in `getResultChunks()` and `getResultChunksData()`, wrapping into `DatabricksSQLException` with HTTP status code and server message preserved - **`ChunkLinkDownloadService.java`**: Widen catch from `SQLException` to `Exception` so `DatabricksError` completes link futures exceptionally instead of leaving them unresolved - **`ChunkDownloadTask.java`**: Chain `uncaughtException` as cause in the finally block so the original error propagates to the caller - **`ChunkLinkDownloadServiceTest.java`**: Fix `testHandleExpiredLinks` to mock subsequent chunks needed by batch continuation ## What customers see after this fix - **Before**: `SQLException: "Download failed for chunk index N"` (no server message, potential hang) - **After**: `SQLException: "Error fetching result chunks for statement [id] chunk [N] (HTTP 404): NOT_FOUND: The result of the statement <id> was not found."` (with full cause chain) ## Test plan - [x] Full jdbc-core suite: 3170 tests pass, 0 failures - [x] `ChunkLinkDownloadServiceTest` — 8 tests pass (including fixed `testHandleExpiredLinks`) This pull request was AI-assisted by Isaac. --------- Signed-off-by: Gopal Lal <gopal.lal@databricks.com> Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
1 parent 7592f65 commit b2af201

6 files changed

Lines changed: 99 additions & 3 deletions

File tree

NEXT_CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818
- Fixed primitive types within complex types (ARRAY, MAP, STRUCT) not being correctly parsed when Arrow serialization uses alternate formats: TIMESTAMP/TIMESTAMP_NTZ as epoch microseconds or component arrays, and BINARY as base64-encoded strings.
1919
- Fixed `PARSE_SYNTAX_ERROR` for column names containing special characters (e.g., dots) when `EnableBatchedInserts` is enabled, by re-quoting column names with backticks in reconstructed multi-row INSERT statements.
2020
- Fixed Volume ingestion for SEA mode, which was broken due to statement being closed prematurely.
21+
- Fixed preserving error message during query results expiry and graceful handling of RuntimeException.
2122
- Fixed escaped pattern characters in catalogName for `getSchemas`, as returned catalogName should be unescaped.
2223
- Fixed `getColumnClassName()` returning null for VARIANT columns in SEA mode by adding VARIANT to the type system.
2324
- Fixed `getColumns()` returning `DATA_TYPE=0` (NULL) for GEOMETRY/GEOGRAPHY columns in Thrift mode. Now returns `Types.VARCHAR` (12) when geospatial is disabled and `Types.OTHER` (1111) when enabled, consistent with SEA mode.

src/main/java/com/databricks/jdbc/api/impl/arrow/ChunkDownloadTask.java

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -133,6 +133,7 @@ public Void call() throws DatabricksSQLException, ExecutionException, Interrupte
133133
.completeExceptionally(
134134
new DatabricksSQLException(
135135
"Download failed for chunk index " + chunk.getChunkIndex(),
136+
uncaughtException,
136137
DatabricksDriverErrorCode.CHUNK_DOWNLOAD_ERROR));
137138
}
138139

src/main/java/com/databricks/jdbc/api/impl/arrow/ChunkLinkDownloadService.java

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111
import com.databricks.jdbc.log.JdbcLoggerFactory;
1212
import com.databricks.jdbc.model.core.ChunkLinkFetchResult;
1313
import com.databricks.jdbc.model.core.ExternalLink;
14+
import com.databricks.sdk.core.DatabricksError;
1415
import com.google.common.annotations.VisibleForTesting;
1516
import java.sql.SQLException;
1617
import java.time.Instant;
@@ -286,8 +287,10 @@ private void triggerNextBatchDownload() {
286287
triggerNextBatchDownload();
287288
}
288289
}
289-
} catch (SQLException e) {
290-
// If the download fails, complete exceptionally all pending futures
290+
} catch (SQLException | DatabricksError e) {
291+
// DatabricksError (RuntimeException) is thrown by the SDK client on HTTP
292+
// errors (e.g. 404 when result expires). Without catching it, link futures
293+
// are left unresolved, causing download tasks to block indefinitely.
291294
handleBatchDownloadError(batchStartIndex, e);
292295
}
293296
});
@@ -299,7 +302,7 @@ private void triggerNextBatchDownload() {
299302
* <p>Completes all pending futures exceptionally with the encountered error and resets the
300303
* download progress flag.
301304
*/
302-
private void handleBatchDownloadError(long batchStartIndex, SQLException e) {
305+
private void handleBatchDownloadError(long batchStartIndex, Exception e) {
303306
LOGGER.error(
304307
e,
305308
"Failed to download links for batch starting at {} : {}",

src/main/java/com/databricks/jdbc/dbclient/impl/sqlexec/DatabricksSdkClient.java

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -488,6 +488,13 @@ public ChunkLinkFetchResult getResultChunks(
488488
req.withHeaders(getHeaders("getStatementResultN"));
489489
ResultData resultData = apiClient.execute(req, ResultData.class);
490490
return buildChunkLinkFetchResult(resultData.getExternalLinks());
491+
} catch (DatabricksError e) {
492+
String errorMessage =
493+
String.format(
494+
"Error fetching result chunks for statement [%s] chunk [%d] (HTTP %d): %s",
495+
statementId, chunkIndex, e.getStatusCode(), e.getMessage());
496+
LOGGER.error(errorMessage, e);
497+
throw new DatabricksSQLException(errorMessage, e, DatabricksDriverErrorCode.SDK_CLIENT_ERROR);
491498
} catch (IOException e) {
492499
String errorMessage = "Error while processing the get result chunk request";
493500
LOGGER.error(errorMessage, e);
@@ -541,6 +548,13 @@ public ResultData getResultChunksData(StatementId typedStatementId, long chunkIn
541548
Request req = new Request(Request.GET, path, apiClient.serialize(request));
542549
req.withHeaders(getHeaders("getStatementResultN"));
543550
return apiClient.execute(req, ResultData.class);
551+
} catch (DatabricksError e) {
552+
String errorMessage =
553+
String.format(
554+
"Error fetching result data for statement [%s] chunk [%d] (HTTP %d): %s",
555+
statementId, chunkIndex, e.getStatusCode(), e.getMessage());
556+
LOGGER.error(errorMessage, e);
557+
throw new DatabricksSQLException(errorMessage, e, DatabricksDriverErrorCode.SDK_CLIENT_ERROR);
544558
} catch (IOException e) {
545559
String errorMessage = "Error while processing the get result chunk request";
546560
LOGGER.error(errorMessage, e);

src/test/java/com/databricks/jdbc/api/impl/arrow/ChunkLinkDownloadServiceTest.java

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414
import com.databricks.jdbc.model.core.ChunkLinkFetchResult;
1515
import com.databricks.jdbc.model.core.ExternalLink;
1616
import com.databricks.jdbc.model.telemetry.enums.DatabricksDriverErrorCode;
17+
import com.databricks.sdk.core.DatabricksError;
1718
import java.sql.SQLException;
1819
import java.time.Instant;
1920
import java.time.temporal.ChronoUnit;
@@ -127,6 +128,28 @@ void testGetLinkForChunk_ClientError()
127128
assertEquals(expectedError, exception.getCause());
128129
}
129130

131+
@Test
132+
void testGetLinkForChunk_DatabricksError_propagatesAsExecutionException()
133+
throws SQLException, ExecutionException, InterruptedException {
134+
long chunkIndex = 1L;
135+
// DatabricksError is a RuntimeException thrown by the SDK on HTTP errors (e.g. 404)
136+
DatabricksError runtimeError = new DatabricksError("404", "Results have expired", 404);
137+
when(mockSession.getDatabricksClient()).thenReturn(mockClient);
138+
when(mockClient.getResultChunks(eq(mockStatementId), anyLong(), anyLong()))
139+
.thenThrow(runtimeError);
140+
when(mockChunkMap.get(chunkIndex)).thenReturn(mock(ArrowResultChunk.class));
141+
142+
ChunkLinkDownloadService<ArrowResultChunk> service =
143+
new ChunkLinkDownloadService<>(
144+
mockSession, mockStatementId, TOTAL_CHUNKS, mockChunkMap, NEXT_BATCH_START_INDEX);
145+
146+
CompletableFuture<ExternalLink> future = service.getLinkForChunk(chunkIndex);
147+
148+
ExecutionException exception =
149+
assertThrows(ExecutionException.class, () -> future.get(1, TimeUnit.SECONDS));
150+
assertEquals(runtimeError, exception.getCause());
151+
}
152+
130153
@Test
131154
void testAutoTriggerForSEAClient() throws SQLException, InterruptedException {
132155
when(mockSession.getDatabricksClient()).thenReturn(mockClient);
@@ -167,6 +190,18 @@ void testHandleExpiredLinks()
167190
when(mockChunk.getStatus()).thenReturn(ChunkStatus.PENDING);
168191
when(mockChunkMap.get(chunkIndex)).thenReturn(mockChunk);
169192

193+
// Mock chunks and empty results for subsequent indices so batch continuation
194+
// doesn't fail when looking up row offsets or fetching the next batch.
195+
// Use lenient() since these may or may not be reached depending on timing.
196+
for (long i = 2; i < TOTAL_CHUNKS; i++) {
197+
ArrowResultChunk subsequentChunk = mock(ArrowResultChunk.class);
198+
lenient().when(subsequentChunk.getStartRowOffset()).thenReturn(i * 100L);
199+
lenient().when(mockChunkMap.get(i)).thenReturn(subsequentChunk);
200+
}
201+
lenient()
202+
.when(mockClient.getResultChunks(eq(mockStatementId), eq(2L), anyLong()))
203+
.thenReturn(buildChunkLinkFetchResult(Collections.emptyList()));
204+
170205
ChunkLinkDownloadService<ArrowResultChunk> service =
171206
new ChunkLinkDownloadService<>(
172207
mockSession, mockStatementId, TOTAL_CHUNKS, mockChunkMap, NEXT_BATCH_START_INDEX);

src/test/java/com/databricks/jdbc/dbclient/impl/sqlexec/DatabricksSdkClientTest.java

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1103,4 +1103,46 @@ public void testExecuteStatementWithClosedStatusAndNoParentStatement() throws Ex
11031103
null,
11041104
null));
11051105
}
1106+
1107+
@Test
1108+
public void testGetResultChunks_DatabricksError_throwsSQLException() throws Exception {
1109+
IDatabricksConnectionContext connectionContext =
1110+
DatabricksConnectionContext.parse(JDBC_URL, new Properties());
1111+
DatabricksSdkClient databricksSdkClient =
1112+
new DatabricksSdkClient(connectionContext, statementExecutionService, apiClient);
1113+
1114+
// Simulate a 404 from the server (result expired)
1115+
when(apiClient.execute(any(Request.class), eq(ResultData.class)))
1116+
.thenThrow(new DatabricksError("404", "Results have expired", 404));
1117+
1118+
DatabricksSQLException exception =
1119+
assertThrows(
1120+
DatabricksSQLException.class,
1121+
() -> databricksSdkClient.getResultChunks(STATEMENT_ID, 0, 0));
1122+
1123+
assertTrue(exception.getMessage().contains("HTTP 404"));
1124+
assertTrue(exception.getMessage().contains("Results have expired"));
1125+
assertNotNull(exception.getCause());
1126+
}
1127+
1128+
@Test
1129+
public void testGetResultChunksData_DatabricksError_throwsSQLException() throws Exception {
1130+
IDatabricksConnectionContext connectionContext =
1131+
DatabricksConnectionContext.parse(JDBC_URL, new Properties());
1132+
DatabricksSdkClient databricksSdkClient =
1133+
new DatabricksSdkClient(connectionContext, statementExecutionService, apiClient);
1134+
1135+
// Simulate a 404 from the server (result expired)
1136+
when(apiClient.execute(any(Request.class), eq(ResultData.class)))
1137+
.thenThrow(new DatabricksError("404", "Results have expired", 404));
1138+
1139+
DatabricksSQLException exception =
1140+
assertThrows(
1141+
DatabricksSQLException.class,
1142+
() -> databricksSdkClient.getResultChunksData(STATEMENT_ID, 0));
1143+
1144+
assertTrue(exception.getMessage().contains("HTTP 404"));
1145+
assertTrue(exception.getMessage().contains("Results have expired"));
1146+
assertNotNull(exception.getCause());
1147+
}
11061148
}

0 commit comments

Comments
 (0)