Improve error messages for transient HTTP failures during Thrift polling (#1426)

## Summary
Fixes unclear error messages when the cluster is temporarily unavailable
(e.g. HTTP 502 Bad Gateway) during `GetOperationStatus` polling.
Previously, errors showed `error: [null]` with no way for callers to
identify them as transient/retryable.

- **Enrich null error messages**: When the server returns `ERROR_STATUS`
with null `errorMessage`, the driver now includes `errorCode`,
`errorDetailsJson`, and `infoMessages` from `TStatus` instead of showing
`error: [null]`
- **Catch transient HTTP failures**: `TException` from
`GetOperationStatus` in the polling loop (typically from 502/503 after
retry exhaustion) is now caught and wrapped with a descriptive message
explaining this is a transient communication failure
- **Retryable SQL state**: Uses SQL state `08S01` (communication link
failure) so callers can programmatically identify these as retryable
errors

### Before
```
Operation status check failed with status code: [ERROR_STATUS] for statement [46a475dc-...], error: [null]
```
No SQL state, no way to identify as transient.

### After
```
Lost connection to server while polling statement [46a475dc-...]. This is typically a transient error (e.g. HTTP 502 Bad Gateway) indicating the cluster was temporarily unavailable. Cause: Retry failure. HTTP response code: 502, Error Message: Bad Gateway
```
SQL state: `08S01` (communication link failure).

Fixes #1165

## Test plan
- [x] `DatabricksThriftAccessorTest` — 50 tests pass
- [x] Updated `testExecute_setsStatementIdEvenIfStatusRequestFails` to
verify `08S01` SQL state and descriptive message

This pull request was AI-assisted by Isaac.

---------

Signed-off-by: Gopal Lal <gopal.lal@databricks.com>
Signed-off-by: Vikrant Puppala <vikrant.puppala@databricks.com>
Signed-off-by: Sreekanth Vadigi <sreekanth.vadigi@databricks.com>
Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
Co-authored-by: Vikrant Puppala <vikrant.puppala@databricks.com>
Co-authored-by: Sreekanth <sreekanth.vadigi@databricks.com>
Co-authored-by: Samikshya Chand <148681192+samikshya-db@users.noreply.github.com>

Author: 5 people

NEXT_CHANGELOG.md

@@ -18,7 +18,7 @@
 - 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.
 - 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.
 - Fixed Volume ingestion for SEA mode, which was broken due to statement being closed prematurely.
-- Fixed preserving error message during query results expiry and graceful handling of RuntimeException.
+- Fixed unclear `error: [null]` messages during transient HTTP failures (e.g. 502 Bad Gateway) in Thrift polling. Error messages now include server error details and use SQL state `08S01` (communication link failure) so callers can identify retryable errors. Also fixed `DatabricksError` (RuntimeException) from SDK client being unhandled in CloudFetch download paths.
 - Fixed escaped pattern characters in catalogName for `getSchemas`, as returned catalogName should be unescaped.
 - Fixed `getColumnClassName()` returning null for VARIANT columns in SEA mode by adding VARIANT to the type system.
 - 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/common/DatabricksJdbcConstants.java

@@ -113,6 +113,9 @@ public final class DatabricksJdbcConstants {
   /** Standard SQL state for operation cancelled (SQLSTATE HY008). */
   public static final String OPERATION_CANCELLED_SQLSTATE = "HY008";
 
+  /** Standard SQL state for communication link failure (SQLSTATE 08S01). */
+  public static final String COMMUNICATION_LINK_FAILURE_SQLSTATE = "08S01";
+
   public static final int TEMPORARY_REDIRECT_STATUS_CODE = 307;
   public static final String REDACTED_TOKEN = "****";
   public static final String QUERY_TAGS = "query_tags";

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

@@ -41,6 +41,7 @@
 import com.databricks.sdk.core.ApiClient;
 import com.databricks.sdk.core.DatabricksConfig;
 import com.databricks.sdk.core.DatabricksError;
+import com.databricks.sdk.core.DatabricksException;
 import com.databricks.sdk.core.http.Request;
 import com.databricks.sdk.service.sql.*;
 import com.google.common.annotations.VisibleForTesting;
@@ -488,11 +489,11 @@ public ChunkLinkFetchResult getResultChunks(
       req.withHeaders(getHeaders("getStatementResultN"));
       ResultData resultData = apiClient.execute(req, ResultData.class);
       return buildChunkLinkFetchResult(resultData.getExternalLinks());
-    } catch (DatabricksError e) {
+    } catch (DatabricksException e) {
       String errorMessage =
           String.format(
-              "Error fetching result chunks for statement [%s] chunk [%d] (HTTP %d): %s",
-              statementId, chunkIndex, e.getStatusCode(), e.getMessage());
+              "Error fetching result chunks for statement [%s] chunk [%d]: %s",
+              statementId, chunkIndex, e.getMessage());
       LOGGER.error(errorMessage, e);
       throw new DatabricksSQLException(errorMessage, e, DatabricksDriverErrorCode.SDK_CLIENT_ERROR);
     } catch (IOException e) {
@@ -548,11 +549,11 @@ public ResultData getResultChunksData(StatementId typedStatementId, long chunkIn
       Request req = new Request(Request.GET, path, apiClient.serialize(request));
       req.withHeaders(getHeaders("getStatementResultN"));
       return apiClient.execute(req, ResultData.class);
-    } catch (DatabricksError e) {
+    } catch (DatabricksException e) {
       String errorMessage =
           String.format(
-              "Error fetching result data for statement [%s] chunk [%d] (HTTP %d): %s",
-              statementId, chunkIndex, e.getStatusCode(), e.getMessage());
+              "Error fetching result data for statement [%s] chunk [%d]: %s",
+              statementId, chunkIndex, e.getMessage());
       LOGGER.error(errorMessage, e);
       throw new DatabricksSQLException(errorMessage, e, DatabricksDriverErrorCode.SDK_CLIENT_ERROR);
     } catch (IOException e) {

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

@@ -1,5 +1,6 @@
 package com.databricks.jdbc.dbclient.impl.thrift;
 
+import static com.databricks.jdbc.common.DatabricksJdbcConstants.COMMUNICATION_LINK_FAILURE_SQLSTATE;
 import static com.databricks.jdbc.common.DatabricksJdbcConstants.OPERATION_CANCELLED_SQLSTATE;
 import static com.databricks.jdbc.common.DatabricksJdbcConstants.QUERY_EXECUTION_TIMEOUT_SQLSTATE;
 import static com.databricks.jdbc.common.EnvironmentVariables.*;
@@ -34,6 +35,7 @@
 import org.apache.thrift.TException;
 import org.apache.thrift.TFieldIdEnum;
 import org.apache.thrift.protocol.TBinaryProtocol;
+import org.apache.thrift.transport.TTransportException;
 
 final class DatabricksThriftAccessor {
 
@@ -329,8 +331,13 @@ private TGetOperationStatusResp pollTillOperationFinished(
       // Check for timeout before continuing
       timeoutHandler.checkTimeout();
 
-      // Polling for operation status
-      statusResp = getOperationStatus(statusReq, statementId);
+      // TTransportException means a transport-level failure (e.g. HTTP 502 Bad Gateway)
+      // after retries were exhausted. Other TException subtypes propagate unchanged.
+      try {
+        statusResp = getOperationStatus(statusReq, statementId);
+      } catch (TTransportException e) {
+        throw buildTransportFailureException(statementId.toSQLExecStatementId(), e);
+      }
       checkOperationStatusForErrors(statusResp, statementId.toSQLExecStatementId());
       // Save some time if sleep isn't required by breaking.
       if (!shouldContinuePolling(statusResp)) {
@@ -752,7 +759,11 @@ TFetchResultsResp fetchMetadataResults(TResp response, String contextDescription
             DatabricksDriverErrorCode.OPERATION_TIMEOUT_ERROR);
     while (shouldContinuePolling(statusResp)) {
       metadataTimeoutHandler.checkTimeout();
-      statusResp = getThriftClient().GetOperationStatus(statusReq);
+      try {
+        statusResp = getThriftClient().GetOperationStatus(statusReq);
+      } catch (TTransportException e) {
+        throw buildTransportFailureException(statementId, e);
+      }
       checkOperationStatusForErrors(statusResp, statementId);
       if (!shouldContinuePolling(statusResp)) {
         break;
@@ -822,11 +833,12 @@ private void checkOperationStatusForErrors(TGetOperationStatusResp statusResp, S
     // and the operation handle becomes invalid. Without this check, the polling loop would
     // continue indefinitely since operationState may not be set in the response.
     if (statusResp.isSetStatus() && isErrorStatusCode(statusResp.getStatus())) {
+      String serverError = enrichErrorMessage(statusResp.getStatus());
       String errorMsg =
           String.format(
               "Operation status check failed with status code: [%s] for statement [%s], "
                   + "error: [%s]",
-              statusResp.getStatus().getStatusCode(), statementId, statusResp.getErrorMessage());
+              statusResp.getStatus().getStatusCode(), statementId, serverError);
       LOGGER.error(errorMsg);
       throw new DatabricksSQLException(
           errorMsg, statusResp.isSetSqlState() ? statusResp.getSqlState() : null);
@@ -838,10 +850,11 @@ private void checkOperationStatusForErrors(TGetOperationStatusResp statusResp, S
     }
 
     if (statusResp.isSetOperationState() && isErrorOperationState(statusResp.getOperationState())) {
+      String serverError = enrichErrorMessage(statusResp.getStatus());
       String errorMsg =
           String.format(
               "Operation failed with error: [%s] for statement [%s], with response [%s]",
-              statusResp.getErrorMessage(), statementId, statusResp);
+              serverError, statementId, statusResp);
       LOGGER.error(errorMsg);
 
       String sqlState = statusResp.getSqlState();
@@ -855,6 +868,52 @@ private void checkOperationStatusForErrors(TGetOperationStatusResp statusResp, S
     }
   }
 
+  /**
+   * Enriches a null or empty error message from TStatus by including errorCode, errorDetailsJson,
+   * and infoMessages. Returns the original errorMessage if it is already present.
+   */
+  private String enrichErrorMessage(TStatus status) {
+    if (status == null) {
+      return "no error details from server";
+    }
+    String errorMessage = status.getErrorMessage();
+    if (errorMessage != null && !errorMessage.isEmpty()) {
+      return errorMessage;
+    }
+    StringBuilder detail = new StringBuilder();
+    if (status.isSetErrorCode()) {
+      detail.append("errorCode=").append(status.getErrorCode());
+    }
+    if (status.isSetErrorDetailsJson()
+        && status.getErrorDetailsJson() != null
+        && !status.getErrorDetailsJson().isEmpty()) {
+      if (detail.length() > 0) detail.append(", ");
+      detail.append("details=").append(status.getErrorDetailsJson());
+    }
+    if (status.isSetInfoMessages() && status.getInfoMessages() != null) {
+      if (detail.length() > 0) detail.append(", ");
+      detail.append("infoMessages=").append(status.getInfoMessages());
+    }
+    return detail.length() > 0 ? detail.toString() : "no error details from server";
+  }
+
+  /**
+   * Builds a DatabricksSQLException for transport-level failures (e.g. HTTP 502 Bad Gateway) during
+   * polling. Uses SQL state 08S01 (communication link failure) so callers can identify retryable
+   * errors.
+   */
+  private DatabricksSQLException buildTransportFailureException(
+      String statementId, TTransportException e) {
+    String errorMsg =
+        String.format(
+            "Lost connection to server while polling statement [%s] (%s). "
+                + "This is typically a transient error (e.g. HTTP 502 Bad Gateway) "
+                + "indicating the cluster was temporarily unavailable. Cause: %s",
+            statementId, e.getClass().getSimpleName(), e.getMessage());
+    LOGGER.error(errorMsg, e);
+    return new DatabricksSQLException(errorMsg, e, COMMUNICATION_LINK_FAILURE_SQLSTATE);
+  }
+
   private boolean shouldContinuePolling(TGetOperationStatusResp statusResp) {
     return statusResp == null
         || !statusResp.isSetOperationState()

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

@@ -1120,7 +1120,6 @@ public void testGetResultChunks_DatabricksError_throwsSQLException() throws Exce
             DatabricksSQLException.class,
             () -> databricksSdkClient.getResultChunks(STATEMENT_ID, 0, 0));
 
-    assertTrue(exception.getMessage().contains("HTTP 404"));
     assertTrue(exception.getMessage().contains("Results have expired"));
     assertNotNull(exception.getCause());
   }
@@ -1141,7 +1140,6 @@ public void testGetResultChunksData_DatabricksError_throwsSQLException() throws
             DatabricksSQLException.class,
             () -> databricksSdkClient.getResultChunksData(STATEMENT_ID, 0));
 
-    assertTrue(exception.getMessage().contains("HTTP 404"));
     assertTrue(exception.getMessage().contains("Results have expired"));
     assertNotNull(exception.getCause());
   }

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

@@ -26,6 +26,7 @@
 import java.sql.Statement;
 import java.util.ArrayList;
 import org.apache.thrift.TException;
+import org.apache.thrift.transport.TTransportException;
 import org.junit.jupiter.api.AfterEach;
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
@@ -472,6 +473,38 @@ void testGetStatementResult_cancelled_throwsWithHY008() throws Exception {
     assertTrue(exception.getMessage().contains("was cancelled"));
   }
 
+  @Test
+  void testPollingPath_errorStatusWithNullMessage_includesErrorCode() throws Exception {
+    setup(false);
+    TExecuteStatementReq request = new TExecuteStatementReq();
+    TExecuteStatementResp executeResp =
+        new TExecuteStatementResp()
+            .setOperationHandle(tOperationHandle)
+            .setStatus(new TStatus().setStatusCode(TStatusCode.SUCCESS_STATUS));
+    when(thriftClient.ExecuteStatement(request)).thenReturn(executeResp);
+
+    // Server returns ERROR_STATUS with null errorMessage but populated errorCode
+    TStatus errorStatus = new TStatus().setStatusCode(TStatusCode.ERROR_STATUS).setErrorCode(502);
+    TGetOperationStatusResp errorResp =
+        new TGetOperationStatusResp()
+            .setStatus(errorStatus)
+            .setOperationState(TOperationState.RUNNING_STATE);
+    when(thriftClient.GetOperationStatus(operationStatusReq)).thenReturn(errorResp);
+
+    Statement statement = mock(Statement.class);
+    when(parentStatement.getStatement()).thenReturn(statement);
+    when(statement.getQueryTimeout()).thenReturn(0);
+
+    DatabricksSQLException exception =
+        assertThrows(
+            DatabricksSQLException.class,
+            () -> accessor.execute(request, parentStatement, session, StatementType.SQL));
+
+    // Verify the enriched message includes errorCode instead of "error: [null]"
+    assertTrue(exception.getMessage().contains("errorCode=502"));
+    assertFalse(exception.getMessage().contains("error: [null]"));
+  }
+
   @Test
   void testListPrimaryKeys() throws TException, SQLException, DatabricksValidationException {
     setup(false);
@@ -778,7 +811,7 @@ void testExecute_setsStatementIdEvenIfStatusRequestFails()
     // Make execute statement succeed but get operation status fail
     when(thriftClient.ExecuteStatement(request)).thenReturn(tExecuteStatementResp);
     when(thriftClient.GetOperationStatus(any(TGetOperationStatusReq.class)))
-        .thenThrow(new TException("Failed to get status"));
+        .thenThrow(new TTransportException("Retry failure. HTTP response code: 502"));
     Statement statement = mock(Statement.class);
     when(parentStatement.getStatement()).thenReturn(statement);
     when(statement.getQueryTimeout()).thenReturn(0);
@@ -789,12 +822,15 @@ void testExecute_setsStatementIdEvenIfStatusRequestFails()
     try {
       accessor.execute(request, parentStatement, session, StatementType.SQL);
       fail("Expected exception due to GetOperationStatus failure");
-    } catch (DatabricksHttpException e) {
+    } catch (DatabricksSQLException e) {
       // Verify that statement ID was set on parent statement despite the failure
       verify(parentStatement).setStatementId(eq(expectedStatementId));
 
-      // Verify the error was from GetOperationStatus
-      assertTrue(e.getMessage().contains("Failed to get status"));
+      // Verify the error indicates a transient communication failure
+      assertTrue(e.getMessage().contains("Lost connection to server while polling"));
+      assertTrue(e.getMessage().contains("TTransportException"));
+      assertTrue(e.getMessage().contains("502"));
+      assertEquals("08S01", e.getSQLState());
     }
   }