Skip to content

Commit 75d13a2

Browse files
authored
Fix cancel error handling: vendor code, telemetry level, SEA async path (#1425)
## Summary Follow-up to #1291 addressing review feedback: - **F2**: `getErrorCode()` now returns `DatabricksDriverErrorCode.EXECUTE_STATEMENT_CANCELLED.ordinal()` instead of 0. The 3-arg `DatabricksSQLException` constructor was silently discarding the error code. - **F3**: Cancellation exceptions use `silentExceptions=true` so BI-tool cancellations (Tableau, Looker) don't emit ERROR-level telemetry. Cancellations are logged at TRACE level instead. - **F4**: SEA `getStatementResult()` now detects CANCELED state and throws `HY008` — previously the async cancel path returned a `DatabricksResultSet` wrapping null result data. - **F5**: Added polling-path test (RUNNING → CANCELED during `pollTillOperationFinished`) for Thrift, which had zero coverage. ## Test plan - [x] `DatabricksSdkClientTest#testHandleFailedExecution_CancelledState_ThrowsWithHY008` — asserts HY008, message, and error code - [x] `DatabricksSdkClientTest#testHandleFailedExecution_FailedState_ThrowsWithoutHY008` — unchanged - [x] `DatabricksSdkClientTest#testGetStatementResult_CancelledState_ThrowsWithHY008` — **new** (F4) - [x] `DatabricksThriftAccessorTest#testGetStatementResult_cancelled_throwsWithHY008` — now asserts error code - [x] `DatabricksThriftAccessorTest#testPollingPath_cancelledDuringExecution_throwsWithHY008` — **new** (F5) NO_CHANGELOG=true This pull request was AI-assisted by Isaac. --------- Signed-off-by: Gopal Lal <gopal.lal@databricks.com>
1 parent 269174e commit 75d13a2

6 files changed

Lines changed: 154 additions & 49 deletions

File tree

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

Lines changed: 17 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -427,6 +427,19 @@ public DatabricksResultSet getStatementResult(
427427
LOGGER.error(errorMessage, e);
428428
throw new DatabricksSQLException(errorMessage, e, DatabricksDriverErrorCode.SDK_CLIENT_ERROR);
429429
}
430+
431+
// Detect cancellation before constructing ResultSet (result data is null when cancelled)
432+
if (response.getStatus() != null
433+
&& response.getStatus().getState() == StatementState.CANCELED) {
434+
String cancelMessage = String.format("Statement [%s] was cancelled", statementId);
435+
LOGGER.info(cancelMessage);
436+
throw new DatabricksSQLException(
437+
cancelMessage,
438+
OPERATION_CANCELLED_SQLSTATE,
439+
DatabricksDriverErrorCode.EXECUTE_STATEMENT_CANCELLED,
440+
true);
441+
}
442+
430443
return new DatabricksResultSet(
431444
response.getStatus(),
432445
typedStatementId,
@@ -726,14 +739,16 @@ void handleFailedExecution(
726739
StatementState statementState = response.getStatus().getState();
727740
ServiceError error = response.getStatus().getError();
728741

729-
// Distinguish cancellation from failure
742+
// Distinguish cancellation from failure — silentExceptions=true so cancellations
743+
// (common in BI tools like Tableau/Looker) don't emit ERROR-level telemetry
730744
if (statementState == StatementState.CANCELED) {
731745
String cancelMessage = String.format("Statement [%s] was cancelled", statementId);
732746
LOGGER.info(cancelMessage);
733747
throw new DatabricksSQLException(
734748
cancelMessage,
735749
OPERATION_CANCELLED_SQLSTATE,
736-
DatabricksDriverErrorCode.EXECUTE_STATEMENT_CANCELLED);
750+
DatabricksDriverErrorCode.EXECUTE_STATEMENT_CANCELLED,
751+
true);
737752
}
738753

739754
String errorMessage =

src/main/java/com/databricks/jdbc/dbclient/impl/thrift/DatabricksThriftAccessor.java

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -934,8 +934,13 @@ private <T extends TBase<T, F>, F extends TFieldIdEnum> boolean hasResultDataInD
934934
private DatabricksSQLException cancelledStatementException(String statementId) {
935935
String msg = String.format("Statement [%s] was cancelled", statementId);
936936
LOGGER.info(msg);
937+
// silentExceptions=true: cancellations are common in BI tools and should not
938+
// emit ERROR-level telemetry
937939
return new DatabricksSQLException(
938-
msg, OPERATION_CANCELLED_SQLSTATE, DatabricksDriverErrorCode.EXECUTE_STATEMENT_CANCELLED);
940+
msg,
941+
OPERATION_CANCELLED_SQLSTATE,
942+
DatabricksDriverErrorCode.EXECUTE_STATEMENT_CANCELLED,
943+
true);
939944
}
940945

941946
private boolean isErrorStatusCode(TStatus status) {

src/main/java/com/databricks/jdbc/exception/DatabricksSQLException.java

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -51,8 +51,16 @@ public DatabricksSQLException(String reason, String sqlState, boolean silentExce
5151

5252
public DatabricksSQLException(
5353
String reason, String sqlState, DatabricksDriverErrorCode internalError) {
54-
super(reason, sqlState);
55-
logTelemetryEvent(sqlState, reason, false);
54+
this(reason, sqlState, internalError, false);
55+
}
56+
57+
public DatabricksSQLException(
58+
String reason,
59+
String sqlState,
60+
DatabricksDriverErrorCode internalError,
61+
boolean silentExceptions) {
62+
super(reason, sqlState, internalError.getCode());
63+
logTelemetryEvent(sqlState, reason, silentExceptions);
5664
}
5765

5866
public DatabricksSQLException(String reason, String sqlState, int vendorCode) {
Lines changed: 54 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -1,48 +1,58 @@
11
package com.databricks.jdbc.model.telemetry.enums;
22

33
public enum DatabricksDriverErrorCode {
4-
CONNECTION_ERROR,
5-
CHUNK_DOWNLOAD_ERROR,
6-
EXECUTE_STATEMENT_FAILED,
7-
EXECUTE_STATEMENT_CANCELLED,
8-
RESULT_SET_ERROR,
9-
UNSUPPORTED_OPERATION,
10-
NOT_IMPLEMENTED_OPERATION,
11-
STATEMENT_EXECUTION_TIMEOUT,
12-
STATEMENT_CLOSED,
13-
CONNECTION_CLOSED,
14-
LOGGING_INITIALISATION_ERROR,
15-
RESULT_SET_CLOSED,
16-
VOLUME_OPERATION_PARSING_ERROR,
17-
THREAD_INTERRUPTED_ERROR,
18-
INPUT_VALIDATION_ERROR,
19-
COMPLEX_DATA_TYPE_STRUCT_CONVERSION_ERROR,
20-
COMPLEX_DATA_TYPE_ARRAY_CONVERSION_ERROR,
21-
COMPLEX_DATA_TYPE_MAP_CONVERSION_ERROR,
22-
INVALID_STATE,
23-
INLINE_CHUNK_PARSING_ERROR,
24-
DECOMPRESSION_ERROR,
25-
VOLUME_OPERATION_URL_GENERATION_ERROR,
26-
VOLUME_OPERATION_LOCAL_FILE_EXISTS_ERROR,
27-
VOLUME_OPERATION_EXCEPTION,
28-
VOLUME_OPERATION_PUT_OPERATION_EXCEPTION,
29-
VOLUME_OPERATION_DELETE_OPERATION_EXCEPTION,
30-
VOLUME_OPERATION_FILE_DOWNLOAD_ERROR,
31-
VOLUME_OPERATION_INVALID_STATE,
32-
SESSION_ID_PARSING_EXCEPTION,
33-
BATCH_EXECUTE_EXCEPTION,
34-
TEMPORARY_REDIRECT_EXCEPTION,
35-
JSON_PARSING_ERROR,
36-
CATALOG_OR_SCHEMA_FETCH_ERROR,
37-
INTEGRATION_TEST_ERROR,
38-
SDK_CLIENT_ERROR,
39-
OPERATION_TIMEOUT_ERROR,
40-
SSL_HANDSHAKE_ERROR,
41-
AUTH_ERROR,
42-
INVALID_CHUNK_STATE_TRANSITION,
43-
CHUNK_READY_ERROR,
44-
TRANSACTION_SET_AUTOCOMMIT_ERROR,
45-
TRANSACTION_COMMIT_ERROR,
46-
TRANSACTION_ROLLBACK_ERROR,
47-
RATE_LIMIT_EXCEEDED
4+
CONNECTION_ERROR(1001),
5+
CHUNK_DOWNLOAD_ERROR(1002),
6+
EXECUTE_STATEMENT_FAILED(1003),
7+
EXECUTE_STATEMENT_CANCELLED(1008),
8+
RESULT_SET_ERROR(1004),
9+
UNSUPPORTED_OPERATION(1005),
10+
NOT_IMPLEMENTED_OPERATION(1006),
11+
STATEMENT_EXECUTION_TIMEOUT(1007),
12+
STATEMENT_CLOSED(1009),
13+
CONNECTION_CLOSED(1010),
14+
LOGGING_INITIALISATION_ERROR(1011),
15+
RESULT_SET_CLOSED(1012),
16+
VOLUME_OPERATION_PARSING_ERROR(1013),
17+
THREAD_INTERRUPTED_ERROR(1014),
18+
INPUT_VALIDATION_ERROR(1015),
19+
COMPLEX_DATA_TYPE_STRUCT_CONVERSION_ERROR(1016),
20+
COMPLEX_DATA_TYPE_ARRAY_CONVERSION_ERROR(1017),
21+
COMPLEX_DATA_TYPE_MAP_CONVERSION_ERROR(1018),
22+
INVALID_STATE(1019),
23+
INLINE_CHUNK_PARSING_ERROR(1020),
24+
DECOMPRESSION_ERROR(1021),
25+
VOLUME_OPERATION_URL_GENERATION_ERROR(1022),
26+
VOLUME_OPERATION_LOCAL_FILE_EXISTS_ERROR(1023),
27+
VOLUME_OPERATION_EXCEPTION(1024),
28+
VOLUME_OPERATION_PUT_OPERATION_EXCEPTION(1025),
29+
VOLUME_OPERATION_DELETE_OPERATION_EXCEPTION(1026),
30+
VOLUME_OPERATION_FILE_DOWNLOAD_ERROR(1027),
31+
VOLUME_OPERATION_INVALID_STATE(1028),
32+
SESSION_ID_PARSING_EXCEPTION(1029),
33+
BATCH_EXECUTE_EXCEPTION(1030),
34+
TEMPORARY_REDIRECT_EXCEPTION(1031),
35+
JSON_PARSING_ERROR(1032),
36+
CATALOG_OR_SCHEMA_FETCH_ERROR(1033),
37+
INTEGRATION_TEST_ERROR(1034),
38+
SDK_CLIENT_ERROR(1035),
39+
OPERATION_TIMEOUT_ERROR(1036),
40+
SSL_HANDSHAKE_ERROR(1037),
41+
AUTH_ERROR(1038),
42+
INVALID_CHUNK_STATE_TRANSITION(1039),
43+
CHUNK_READY_ERROR(1040),
44+
TRANSACTION_SET_AUTOCOMMIT_ERROR(1041),
45+
TRANSACTION_COMMIT_ERROR(1042),
46+
TRANSACTION_ROLLBACK_ERROR(1043),
47+
RATE_LIMIT_EXCEEDED(1044);
48+
49+
private final int code;
50+
51+
DatabricksDriverErrorCode(int code) {
52+
this.code = code;
53+
}
54+
55+
public int getCode() {
56+
return code;
57+
}
4858
}

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

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -312,6 +312,7 @@ public void testHandleFailedExecution_CancelledState_ThrowsWithHY008() throws Ex
312312

313313
assertEquals("HY008", exception.getSQLState());
314314
assertTrue(exception.getMessage().contains("was cancelled"));
315+
assertEquals(1008, exception.getErrorCode()); // EXECUTE_STATEMENT_CANCELLED stable code
315316
}
316317

317318
@Test
@@ -338,6 +339,34 @@ public void testHandleFailedExecution_FailedState_ThrowsWithoutHY008() throws Ex
338339
assertTrue(exception.getMessage().contains("execution failed"));
339340
}
340341

342+
@Test
343+
public void testGetStatementResult_CancelledState_ThrowsWithHY008() throws Exception {
344+
IDatabricksConnectionContext connectionContext =
345+
DatabricksConnectionContext.parse(JDBC_URL, new Properties());
346+
DatabricksSdkClient databricksSdkClient =
347+
new DatabricksSdkClient(connectionContext, statementExecutionService, apiClient);
348+
349+
// Server returns CANCELED with null result data
350+
StatementStatus cancelledStatus = new StatementStatus().setState(StatementState.CANCELED);
351+
GetStatementResponse cancelledResponse = new GetStatementResponse();
352+
cancelledResponse.setStatus(cancelledStatus);
353+
cancelledResponse.setStatementId(STATEMENT_ID.toSQLExecStatementId());
354+
355+
when(apiClient.execute(any(Request.class), eq(GetStatementResponse.class)))
356+
.thenReturn(cancelledResponse);
357+
358+
DatabricksSQLException exception =
359+
assertThrows(
360+
DatabricksSQLException.class,
361+
() ->
362+
databricksSdkClient.getStatementResult(
363+
STATEMENT_ID, mock(DatabricksSession.class), null));
364+
365+
assertEquals("HY008", exception.getSQLState());
366+
assertTrue(exception.getMessage().contains("was cancelled"));
367+
assertEquals(1008, exception.getErrorCode()); // EXECUTE_STATEMENT_CANCELLED stable code
368+
}
369+
341370
@Test
342371
public void testDisposition_arrowAndCloudFetchEnabled_usesExternalLinks() throws Exception {
343372
setupClientMocks(true, false);

src/test/java/com/databricks/jdbc/dbclient/impl/thrift/DatabricksThriftAccessorTest.java

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -471,6 +471,44 @@ void testGetStatementResult_cancelled_throwsWithHY008() throws Exception {
471471

472472
assertEquals("HY008", exception.getSQLState());
473473
assertTrue(exception.getMessage().contains("was cancelled"));
474+
assertEquals(1008, exception.getErrorCode()); // EXECUTE_STATEMENT_CANCELLED stable code
475+
}
476+
477+
@Test
478+
void testPollingPath_cancelledDuringExecution_throwsWithHY008() throws Exception {
479+
setup(false);
480+
TExecuteStatementReq request = new TExecuteStatementReq();
481+
TExecuteStatementResp executeResp =
482+
new TExecuteStatementResp()
483+
.setOperationHandle(tOperationHandle)
484+
.setStatus(new TStatus().setStatusCode(TStatusCode.SUCCESS_STATUS));
485+
when(thriftClient.ExecuteStatement(request)).thenReturn(executeResp);
486+
487+
// First poll returns RUNNING, second returns CANCELED (simulates cancel during execution)
488+
TGetOperationStatusResp runningResp =
489+
new TGetOperationStatusResp()
490+
.setStatus(new TStatus().setStatusCode(TStatusCode.STILL_EXECUTING_STATUS))
491+
.setOperationState(TOperationState.RUNNING_STATE);
492+
TGetOperationStatusResp cancelledResp =
493+
new TGetOperationStatusResp()
494+
.setStatus(new TStatus().setStatusCode(TStatusCode.SUCCESS_STATUS))
495+
.setOperationState(TOperationState.CANCELED_STATE);
496+
when(thriftClient.GetOperationStatus(operationStatusReq))
497+
.thenReturn(runningResp)
498+
.thenReturn(cancelledResp);
499+
500+
Statement statement = mock(Statement.class);
501+
when(parentStatement.getStatement()).thenReturn(statement);
502+
when(statement.getQueryTimeout()).thenReturn(0);
503+
504+
DatabricksSQLException exception =
505+
assertThrows(
506+
DatabricksSQLException.class,
507+
() -> accessor.execute(request, parentStatement, session, StatementType.SQL));
508+
509+
assertEquals("HY008", exception.getSQLState());
510+
assertTrue(exception.getMessage().contains("was cancelled"));
511+
assertEquals(1008, exception.getErrorCode()); // EXECUTE_STATEMENT_CANCELLED stable code
474512
}
475513

476514
@Test

0 commit comments

Comments
 (0)