GH-863: [JDBC] Suppress benign exceptions from gRPC layer on ArrowFlightSqlClientHandler#close (#910)

## What's Changed

When using the Flight SQL JDBC driver with connection pooling and a
catalog parameter, ArrowFlightSqlClientHandler.close() performs a
CloseSession RPC that can fail during gRPC channel shutdown.

These transient failures (UNAVAILABLE or INTERNAL with "Connection
closed after GOAWAY") cause noisy errors in pooling frameworks like
Apache Commons DBCP.

With this PR these exceptions will instead be suppressed and logged,
following the procedure that was used for
[ARROW-17785](https://issues.apache.org/jira/browse/ARROW-17785)

### Are these changes tested?

Yes

Closes #863

---------

Co-authored-by: Vando Pereira <vando@dremio.com>

Author: ennuite

flight/flight-sql-jdbc-core/src/main/java/org/apache/arrow/driver/jdbc/client/ArrowFlightSqlClientHandler.java

@@ -262,15 +262,85 @@ public FlightInfo getInfo(final String query) {
   @Override
   public void close() throws SQLException {
     if (catalog.isPresent()) {
-      sqlClient.closeSession(new CloseSessionRequest(), getOptions());
+      try {
+        sqlClient.closeSession(new CloseSessionRequest(), getOptions());
+      } catch (FlightRuntimeException fre) {
+        handleBenignCloseException(
+            fre, "Failed to close Flight SQL session.", "closing Flight SQL session");
+      }
     }
     try {
       AutoCloseables.close(sqlClient);
+    } catch (FlightRuntimeException fre) {
+      handleBenignCloseException(
+          fre, "Failed to clean up client resources.", "closing Flight SQL client");
     } catch (final Exception e) {
       throw new SQLException("Failed to clean up client resources.", e);
     }
   }
 
+  /**
+   * Handles FlightRuntimeException during close operations, suppressing benign gRPC shutdown errors
+   * while re-throwing genuine failures.
+   *
+   * @param fre the FlightRuntimeException to handle
+   * @param sqlErrorMessage the SQLException message to use for genuine failures
+   * @param operationDescription description of the operation for logging
+   * @throws SQLException if the exception represents a genuine failure
+   */
+  private void handleBenignCloseException(
+      FlightRuntimeException fre, String sqlErrorMessage, String operationDescription)
+      throws SQLException {
+    if (isBenignCloseException(fre)) {
+      logSuppressedCloseException(fre, operationDescription);
+    } else {
+      throw new SQLException(sqlErrorMessage, fre);
+    }
+  }
+
+  /**
+   * Handles FlightRuntimeException during close operations, suppressing benign gRPC shutdown errors
+   * while re-throwing genuine failures as FlightRuntimeException.
+   *
+   * @param fre the FlightRuntimeException to handle
+   * @param operationDescription description of the operation for logging
+   * @throws FlightRuntimeException if the exception represents a genuine failure
+   */
+  private void handleBenignCloseException(FlightRuntimeException fre, String operationDescription)
+      throws FlightRuntimeException {
+    if (isBenignCloseException(fre)) {
+      logSuppressedCloseException(fre, operationDescription);
+    } else {
+      throw fre;
+    }
+  }
+
+  /**
+   * Determines if a FlightRuntimeException represents a benign close operation error that should be
+   * suppressed.
+   *
+   * @param fre the FlightRuntimeException to check
+   * @return true if the exception should be suppressed, false otherwise
+   */
+  private boolean isBenignCloseException(FlightRuntimeException fre) {
+    return fre.status().code().equals(FlightStatusCode.UNAVAILABLE)
+        || (fre.status().code().equals(FlightStatusCode.INTERNAL)
+            && fre.getMessage() != null
+            && fre.getMessage().contains("Connection closed after GOAWAY"));
+  }
+
+  /**
+   * Logs a suppressed close exception with appropriate level based on debug settings.
+   *
+   * @param fre the FlightRuntimeException being suppressed
+   * @param operationDescription description of the operation for logging
+   */
+  private void logSuppressedCloseException(
+      FlightRuntimeException fre, String operationDescription) {
+    // ARROW-17785 and GH-863: suppress exceptions caused by flaky gRPC layer during shutdown
+    LOGGER.debug("Suppressed error {}", operationDescription, fre);
+  }
+
   /** A prepared statement handler. */
   public interface PreparedStatement extends AutoCloseable {
     /**
@@ -386,14 +456,7 @@ public void close() {
         try {
           preparedStatement.close(getOptions());
         } catch (FlightRuntimeException fre) {
-          // ARROW-17785: suppress exceptions caused by flaky gRPC layer
-          if (fre.status().code().equals(FlightStatusCode.UNAVAILABLE)
-              || (fre.status().code().equals(FlightStatusCode.INTERNAL)
-                  && fre.getMessage().contains("Connection closed after GOAWAY"))) {
-            LOGGER.warn("Supressed error closing PreparedStatement", fre);
-            return;
-          }
-          throw fre;
+          handleBenignCloseException(fre, "closing PreparedStatement");
         }
       }
     };

flight/flight-sql-jdbc-core/src/test/java/org/apache/arrow/driver/jdbc/client/ArrowFlightSqlClientHandlerTest.java

@@ -0,0 +1,88 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *    http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.arrow.driver.jdbc.client;
+
+import static org.junit.jupiter.api.Assertions.assertDoesNotThrow;
+import static org.junit.jupiter.api.Assertions.assertThrows;
+import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.Mockito.doThrow;
+import static org.mockito.Mockito.mock;
+
+import java.sql.SQLException;
+import java.util.ArrayList;
+import java.util.Collection;
+import java.util.Optional;
+import org.apache.arrow.flight.CallOption;
+import org.apache.arrow.flight.CallStatus;
+import org.apache.arrow.flight.CloseSessionRequest;
+import org.apache.arrow.flight.FlightStatusCode;
+import org.apache.arrow.flight.sql.FlightSqlClient;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.MethodSource;
+
+public class ArrowFlightSqlClientHandlerTest {
+
+  @ParameterizedTest
+  @MethodSource
+  public void testCloseHandlesFlightRuntimeException(
+      boolean throwFromCloseSession, CallStatus callStatus, boolean shouldSuppress)
+      throws Exception {
+    FlightSqlClient sqlClient = mock(FlightSqlClient.class);
+    String cacheKey = "cacheKey";
+    Optional<String> catalog =
+        throwFromCloseSession ? Optional.of("test_catalog") : Optional.empty();
+    final Collection<CallOption> credentialOptions = new ArrayList<>();
+    ArrowFlightSqlClientHandler.Builder builder = new ArrowFlightSqlClientHandler.Builder();
+
+    if (throwFromCloseSession) {
+      doThrow(callStatus.toRuntimeException())
+          .when(sqlClient)
+          .closeSession(any(CloseSessionRequest.class), any(CallOption[].class));
+    } else {
+      doThrow(callStatus.toRuntimeException()).when(sqlClient).close();
+    }
+
+    ArrowFlightSqlClientHandler sqlClientHandler =
+        new ArrowFlightSqlClientHandler(
+            cacheKey, sqlClient, builder, credentialOptions, catalog, null);
+
+    if (shouldSuppress) {
+      assertDoesNotThrow(sqlClientHandler::close);
+    } else {
+      assertThrows(SQLException.class, sqlClientHandler::close);
+    }
+  }
+
+  private static Object[] testCloseHandlesFlightRuntimeException() {
+    CallStatus benignInternalError =
+        new CallStatus(FlightStatusCode.INTERNAL, null, "Connection closed after GOAWAY", null);
+    CallStatus notBenignInternalError =
+        new CallStatus(FlightStatusCode.INTERNAL, null, "Not a benign internal error", null);
+    CallStatus unavailableError = new CallStatus(FlightStatusCode.UNAVAILABLE, null, null, null);
+    CallStatus unknownError = new CallStatus(FlightStatusCode.UNKNOWN, null, null, null);
+    return new Object[] {
+      new Object[] {true, benignInternalError, true},
+      new Object[] {false, benignInternalError, true},
+      new Object[] {true, notBenignInternalError, false},
+      new Object[] {false, notBenignInternalError, false},
+      new Object[] {true, unavailableError, true},
+      new Object[] {false, unavailableError, true},
+      new Object[] {true, unknownError, false},
+      new Object[] {false, unknownError, false},
+    };
+  }
+}