fixed handling error when successful http status by error code header present

Author: chernser

clickhouse-http-client/src/main/java/com/clickhouse/client/http/ApacheHttpConnectionImpl.java

@@ -194,11 +194,11 @@ private void setHeaders(HttpRequest request, Map<String, String> headers) {
     }
 
     private void checkResponse(ClickHouseConfig config, CloseableHttpResponse response) throws IOException {
-        if (response.getCode() == HttpURLConnection.HTTP_OK) {
+        final Header errorCode = response.getFirstHeader(ClickHouseHttpProto.HEADER_EXCEPTION_CODE);
+        if (response.getCode() == HttpURLConnection.HTTP_OK && errorCode == null) {
             return;
         }
 
-        final Header errorCode = response.getFirstHeader(ClickHouseHttpProto.HEADER_EXCEPTION_CODE);
         final Header serverName = response.getFirstHeader(ClickHouseHttpProto.HEADER_SRV_DISPLAY_NAME);
         if (response.getEntity() == null) {
             throw new ConnectException(

client-v2/src/main/java/com/clickhouse/client/api/Client.java

@@ -1,6 +1,7 @@
 package com.clickhouse.client.api;
 
 import com.clickhouse.client.ClickHouseClient;
+import com.clickhouse.client.ClickHouseException;
 import com.clickhouse.client.ClickHouseNode;
 import com.clickhouse.client.ClickHouseRequest;
 import com.clickhouse.client.ClickHouseResponse;
@@ -80,6 +81,7 @@
 import java.util.TimeZone;
 import java.util.UUID;
 import java.util.concurrent.CompletableFuture;
+import java.util.concurrent.CompletionException;
 import java.util.concurrent.ConcurrentHashMap;
 import java.util.concurrent.ExecutionException;
 import java.util.concurrent.ExecutorService;
@@ -1455,6 +1457,11 @@ public CompletableFuture<InsertResponse> insert(String tableName,
                     return response;
                 } catch (ExecutionException e) {
                     throw  new ClientException("Failed to get insert response", e.getCause());
+                } catch (CompletionException e) {
+                    if (e.getCause() instanceof ClickHouseException) {
+                        throw new ServerException(((ClickHouseException)e.getCause()).getErrorCode(), e.getCause().getMessage().trim());
+                    }
+                    throw new ClientException("Failed to get query response", e.getCause());
                 } catch (InterruptedException | TimeoutException e) {
                     throw  new ClientException("Operation has likely timed out.", e);
                 }
@@ -1575,7 +1582,7 @@ public CompletableFuture<QueryResponse> query(String sqlQuery, Map<String, Objec
                         } else {
                             throw lastException;
                         }
-                    } catch (ClientException e) {
+                    } catch (ClientException | ServerException e) {
                         throw e;
                     } catch (Exception e) {
                         throw new ClientException("Query request failed", e);
@@ -1609,6 +1616,11 @@ public CompletableFuture<QueryResponse> query(String sqlQuery, Map<String, Objec
                     return new QueryResponse(clickHouseResponse, format, clientStats, finalSettings);
                 } catch (ClientException e) {
                     throw e;
+                } catch (CompletionException e) {
+                    if (e.getCause() instanceof ClickHouseException) {
+                        throw new ServerException(((ClickHouseException)e.getCause()).getErrorCode(), e.getCause().getMessage().trim());
+                    }
+                    throw new ClientException("Failed to get query response", e.getCause());
                 } catch (Exception e) {
                     throw new ClientException("Failed to get query response", e);
                 }

client-v2/src/main/java/com/clickhouse/client/api/internal/HttpAPIClientHelper.java

@@ -348,13 +348,14 @@ public ClassicHttpResponse executeRequest(ClickHouseNode server, Map<String, Obj
 
         try {
             ClassicHttpResponse httpResponse = httpClient.executeOpen(null, req, context);
+            httpResponse.getEntity().getTrailers().get();
             httpResponse.setEntity(wrapEntity(httpResponse.getEntity(), true));
             if (httpResponse.getCode() == HttpStatus.SC_PROXY_AUTHENTICATION_REQUIRED) {
                 throw new ClientMisconfigurationException("Proxy authentication required. Please check your proxy settings.");
             } else if (httpResponse.getCode() == HttpStatus.SC_BAD_GATEWAY) {
                 httpResponse.close();
                 throw new ClientException("Server returned '502 Bad gateway'. Check network and proxy settings.");
-            } else if (httpResponse.getCode() >= HttpStatus.SC_BAD_REQUEST) {
+            } else if (httpResponse.getCode() >= HttpStatus.SC_BAD_REQUEST || httpResponse.containsHeader(ClickHouseHttpProto.HEADER_EXCEPTION_CODE)) {
                 try {
                     throw readError(httpResponse);
                 } finally {

client-v2/src/test/java/com/clickhouse/client/HttpTransportTests.java

@@ -25,6 +25,7 @@
 import org.apache.hc.core5.http.ConnectionRequestTimeoutException;
 import org.apache.hc.core5.http.HttpStatus;
 import org.apache.hc.core5.net.URIBuilder;
+import org.eclipse.jetty.server.Server;
 import org.testcontainers.utility.ThrowingFunction;
 import org.testng.Assert;
 import org.testng.annotations.DataProvider;
@@ -338,6 +339,43 @@ public static Object[] testServerErrorHandlingDataProvider() {
         return new Object[] { ClickHouseFormat.JSON, ClickHouseFormat.TabSeparated, ClickHouseFormat.RowBinary };
     }
 
+
+    @Test(groups = { "integration" })
+    public void testErrorWithSuccessfulResponse() {
+        WireMockServer mockServer = new WireMockServer( WireMockConfiguration
+                .options().port(9090).notifier(new ConsoleNotifier(false)));
+        mockServer.start();
+
+        try (Client client = new Client.Builder().addEndpoint(Protocol.HTTP, "localhost", mockServer.port(), false)
+                .setUsername("default")
+                .setPassword("")
+                .compressServerResponse(false)
+                .useNewImplementation(true)
+                .build()) {
+            mockServer.addStubMapping(WireMock.post(WireMock.anyUrl())
+                    .willReturn(WireMock.aResponse()
+                            .withStatus(HttpStatus.SC_OK)
+                            .withChunkedDribbleDelay(2, 200)
+                            .withHeader("X-ClickHouse-Exception-Code", "241")
+                            .withHeader("X-ClickHouse-Summary",
+                                    "{ \"read_bytes\": \"10\", \"read_rows\": \"1\"}")
+                            .withBody("Code: 241. DB::Exception: Memory limit (for query) exceeded: would use 97.21 MiB"))
+                    .build());
+
+            try (QueryResponse response = client.query("SELECT 1").get(1, TimeUnit.SECONDS)) {
+                Assert.fail("Expected exception");
+            } catch (ServerException e) {
+                e.printStackTrace();
+                Assert.assertEquals(e.getMessage(), "Code: 241. DB::Exception: Memory limit (for query) exceeded: would use 97.21 MiB");
+            } catch (Exception e) {
+                e.printStackTrace();
+                Assert.fail("Unexpected exception", e);
+            }
+        } finally {
+            mockServer.stop();
+        }
+    }
+
     @Test(groups = { "integration" })
     public void testAdditionalHeaders() {
         WireMockServer mockServer = new WireMockServer( WireMockConfiguration