Skip to content

Commit 1410b8a

Browse files
chernserchernser
committed
Fixed reading errors for different scenarios.
1 parent 807fb14 commit 1410b8a

File tree

3 files changed

+145
-43
lines changed

3 files changed

+145
-43
lines changed

‎client-v2/pom.xml‎

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -66,12 +66,6 @@
6666
<artifactId>jackson-core</artifactId>
6767
<version>2.17.2</version>
6868
</dependency>
69-
<!-- https://mvnrepository.com/artifact/org.apache.commons/commons-text -->
70-
<dependency>
71-
<groupId>org.apache.commons</groupId>
72-
<artifactId>commons-text</artifactId>
73-
<version>1.12.0</version>
74-
</dependency>
7569

7670
<!-- Test Dependencies -->
7771
<dependency>

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

Lines changed: 51 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,6 @@
1616
import com.clickhouse.client.config.ClickHouseDefaults;
1717
import com.clickhouse.client.http.ClickHouseHttpProto;
1818
import com.clickhouse.client.http.config.ClickHouseHttpOption;
19-
import org.apache.commons.text.StringEscapeUtils;
2019
import org.apache.hc.client5.http.ConnectTimeoutException;
2120
import org.apache.hc.client5.http.classic.methods.HttpPost;
2221
import org.apache.hc.client5.http.config.ConnectionConfig;
@@ -288,31 +287,50 @@ public Exception readError(ClassicHttpResponse httpResponse) {
288287

289288
byte [] buffer = new byte[ERROR_BODY_BUFFER_SIZE];
290289
byte [] lookUpStr = String.format(ERROR_CODE_PREFIX_PATTERN, serverCode).getBytes(StandardCharsets.UTF_8);
290+
StringBuilder msgBuilder = new StringBuilder();
291+
boolean found = false;
291292
while (true) {
292293
int rBytes = body.read(buffer);
293294
if (rBytes == -1) {
294295
break;
295296
} else {
296-
for (int i = 0; i < rBytes; i++) {
297-
if (buffer[i] == lookUpStr[0]) {
298-
boolean found = true;
299-
for (int j = 1; j < Math.min(rBytes - i, lookUpStr.length); j++) {
300-
if (buffer[i + j] != lookUpStr[j]) {
301-
found = false;
302-
break;
303-
}
304-
}
305-
if (found) {
306-
String msg = StringEscapeUtils.unescapeJson(new String(buffer, i, rBytes - i, StandardCharsets.UTF_8));
307-
return new ServerException(serverCode, msg.replaceAll("\n", " "));
297+
System.out.println(new String(buffer, 0, rBytes, StandardCharsets.UTF_8));
298+
}
299+
300+
for (int i = 0; i < rBytes; i++) {
301+
if (buffer[i] == lookUpStr[0]) {
302+
found = true;
303+
for (int j = 1; j < Math.min(rBytes - i, lookUpStr.length); j++) {
304+
if (buffer[i + j] != lookUpStr[j]) {
305+
found = false;
306+
break;
308307
}
309308
}
309+
if (found) {
310+
msgBuilder.append(new String(buffer, i, rBytes - i, StandardCharsets.UTF_8));
311+
break;
312+
}
310313
}
311314
}
315+
316+
if (found) {
317+
break;
318+
}
312319
}
313320

314-
return new ServerException(serverCode, String.format(ERROR_CODE_PREFIX_PATTERN, serverCode) + " <Unreadable error message>");
321+
while (true) {
322+
int rBytes = body.read(buffer);
323+
if (rBytes == -1) {
324+
break;
325+
}
326+
msgBuilder.append(new String(buffer, 0, rBytes, StandardCharsets.UTF_8));
327+
}
328+
329+
String msg = msgBuilder.toString().replaceAll("\\s+", " ").replaceAll("\\\\n", " ")
330+
.replaceAll("\\\\/", "/");
331+
return new ServerException(serverCode, msg);
315332
} catch (Exception e) {
333+
LOG.error("Failed to read error message", e);
316334
return new ServerException(serverCode, String.format(ERROR_CODE_PREFIX_PATTERN, serverCode) + " <Unreadable error message>");
317335
}
318336
}
@@ -500,26 +518,28 @@ private void addQueryParams(URIBuilder req, Map<String, String> chConfig, Map<St
500518
}
501519

502520
private HttpEntity wrapEntity(HttpEntity httpEntity, int httpStatus, boolean isResponse) {
503-
504-
switch (httpStatus) {
505-
case HttpStatus.SC_OK:
506-
case HttpStatus.SC_CREATED:
507-
case HttpStatus.SC_ACCEPTED:
508-
case HttpStatus.SC_NO_CONTENT:
509-
case HttpStatus.SC_PARTIAL_CONTENT:
510-
case HttpStatus.SC_RESET_CONTENT:
511-
case HttpStatus.SC_NOT_MODIFIED:
512-
case HttpStatus.SC_BAD_REQUEST:
513-
boolean serverCompression = chConfiguration.getOrDefault(ClickHouseClientOption.COMPRESS.getKey(), "false").equalsIgnoreCase("true");
514-
boolean clientCompression = chConfiguration.getOrDefault(ClickHouseClientOption.DECOMPRESS.getKey(), "false").equalsIgnoreCase("true");
515-
boolean useHttpCompression = chConfiguration.getOrDefault("client.use_http_compression", "false").equalsIgnoreCase("true");
516-
if (serverCompression || clientCompression) {
521+
boolean serverCompression = MapUtils.getFlag(chConfiguration, ClickHouseClientOption.COMPRESS.getKey(), false);
522+
boolean clientCompression = MapUtils.getFlag(chConfiguration, ClickHouseClientOption.DECOMPRESS.getKey(), false);
523+
524+
if (serverCompression || clientCompression) {
525+
// Server doesn't compress certain errors like 403
526+
switch (httpStatus) {
527+
case HttpStatus.SC_OK:
528+
case HttpStatus.SC_CREATED:
529+
case HttpStatus.SC_ACCEPTED:
530+
case HttpStatus.SC_NO_CONTENT:
531+
case HttpStatus.SC_PARTIAL_CONTENT:
532+
case HttpStatus.SC_RESET_CONTENT:
533+
case HttpStatus.SC_NOT_MODIFIED:
534+
case HttpStatus.SC_BAD_REQUEST:
535+
case HttpStatus.SC_INTERNAL_SERVER_ERROR:
536+
boolean useHttpCompression = MapUtils.getFlag(chConfiguration, "client.use_http_compression", false);
517537
return new LZ4Entity(httpEntity, useHttpCompression, serverCompression, clientCompression,
518538
MapUtils.getInt(chConfiguration, "compression.lz4.uncompressed_buffer_size"), isResponse);
519-
}
520-
default:
521-
return httpEntity;
539+
}
522540
}
541+
542+
return httpEntity;
523543
}
524544

525545
public static int getHeaderInt(Header header, int defaultValue) {

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

Lines changed: 94 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,7 @@
3838
import java.time.temporal.ChronoUnit;
3939
import java.util.Arrays;
4040
import java.util.Base64;
41+
import java.util.EnumSet;
4142
import java.util.List;
4243
import java.util.Random;
4344
import java.util.concurrent.CompletableFuture;
@@ -308,13 +309,14 @@ public static Object[][] noResponseFailureProvider() {
308309
}
309310

310311
@Test(groups = { "integration" }, dataProvider = "testServerErrorHandlingDataProvider")
311-
public void testServerErrorHandling(ClickHouseFormat format) {
312+
public void testServerErrorHandling(ClickHouseFormat format, boolean serverCompression, boolean useHttpCompression) {
312313
ClickHouseNode server = getServer(ClickHouseProtocol.HTTP);
313314
try (Client client = new Client.Builder()
314315
.addEndpoint(server.getBaseUri())
315316
.setUsername("default")
316317
.setPassword("")
317-
.compressServerResponse(false)
318+
.compressServerResponse(serverCompression)
319+
.useHttpCompression(useHttpCompression)
318320
.build()) {
319321

320322
QuerySettings querySettings = new QuerySettings().setFormat(format);
@@ -344,14 +346,45 @@ public void testServerErrorHandling(ClickHouseFormat format) {
344346
e.printStackTrace();
345347
Assert.fail(e.getMessage(), e);
346348
}
349+
350+
try (Client client = new Client.Builder()
351+
.addEndpoint(server.getBaseUri())
352+
.setUsername("non-existing-user")
353+
.setPassword("nothing")
354+
.compressServerResponse(serverCompression)
355+
.useHttpCompression(useHttpCompression)
356+
.build()) {
357+
358+
try (QueryResponse response = client.query("SELECT 1").get(1, TimeUnit.SECONDS)) {
359+
Assert.fail("Expected exception");
360+
} catch (ServerException e) {
361+
e.printStackTrace();
362+
Assert.assertEquals(e.getCode(), 516);
363+
Assert.assertTrue(e.getMessage().startsWith("Code: 516. DB::Exception: non-existing-user: Authentication failed: password is incorrect, or there is no user with such name. (AUTHENTICATION_FAILED)"),
364+
e.getMessage());
365+
} catch (Exception e) {
366+
e.printStackTrace();
367+
Assert.fail("Unexpected exception", e);
368+
}
369+
}
347370
}
348371

349372
@DataProvider(name = "testServerErrorHandlingDataProvider")
350-
public static Object[] testServerErrorHandlingDataProvider() {
351-
return new Object[] { ClickHouseFormat.JSON, ClickHouseFormat.TabSeparated, ClickHouseFormat.RowBinary, ClickHouseFormat.TSKV,
352-
ClickHouseFormat.JSONEachRow};
353-
}
373+
public static Object[][] testServerErrorHandlingDataProvider() {
374+
EnumSet<ClickHouseFormat> formats = EnumSet.of(ClickHouseFormat.CSV, ClickHouseFormat.TSV,
375+
ClickHouseFormat.JSON, ClickHouseFormat.JSONCompact);
376+
377+
Object[][] result = new Object[formats.size() * 3][];
378+
379+
int i = 0;
380+
for (ClickHouseFormat format : formats) {
381+
result[i++] = new Object[]{format, false, false};
382+
result[i++] = new Object[]{format, true, false};
383+
result[i++] = new Object[]{format, true, true};
384+
}
354385

386+
return result;
387+
}
355388

356389
@Test(groups = { "integration" })
357390
public void testErrorWithSuccessfulResponse() {
@@ -389,6 +422,61 @@ public void testErrorWithSuccessfulResponse() {
389422
}
390423
}
391424

425+
@Test(groups = { "integration" }, dataProvider = "testServerErrorsUncompressedDataProvider")
426+
public void testServerErrorsUncompressed(int code, String message, String expectedMessage) {
427+
WireMockServer mockServer = new WireMockServer( WireMockConfiguration
428+
.options().port(9090).notifier(new ConsoleNotifier(false)));
429+
mockServer.start();
430+
431+
mockServer.addStubMapping(WireMock.post(WireMock.anyUrl())
432+
.willReturn(WireMock.aResponse()
433+
.withStatus(HttpStatus.SC_OK)
434+
.withChunkedDribbleDelay(2, 200)
435+
.withHeader("X-ClickHouse-Exception-Code", String.valueOf(code))
436+
.withHeader("X-ClickHouse-Summary",
437+
"{ \"read_bytes\": \"10\", \"read_rows\": \"1\"}")
438+
.withBody(message))
439+
.build());
440+
441+
try (Client client = new Client.Builder().addEndpoint(Protocol.HTTP, "localhost", mockServer.port(), false)
442+
.setUsername("default")
443+
.setPassword("")
444+
.compressServerResponse(false)
445+
.build()) {
446+
447+
try (QueryResponse response = client.query("SELECT 1").get(1, TimeUnit.SECONDS)) {
448+
Assert.fail("Expected exception");
449+
} catch (ServerException e) {
450+
e.printStackTrace();
451+
Assert.assertEquals(e.getCode(), code);
452+
Assert.assertEquals(e.getMessage(), expectedMessage);
453+
} catch (Exception e) {
454+
e.printStackTrace();
455+
Assert.fail("Unexpected exception", e);
456+
}
457+
} finally {
458+
mockServer.stop();
459+
}
460+
}
461+
462+
@DataProvider(name = "testServerErrorsUncompressedDataProvider")
463+
public static Object[][] testServerErrorsUncompressedDataProvider() {
464+
return new Object[][] {
465+
{ 241, "Code: 241. DB::Exception: Memory limit (for query) exceeded: would use 97.21 MiB",
466+
"Code: 241. DB::Exception: Memory limit (for query) exceeded: would use 97.21 MiB"},
467+
{900, "Code: 900. DB::Exception: \uD83D\uDCBE Floppy disk is full",
468+
"Code: 900. DB::Exception: \uD83D\uDCBE Floppy disk is full"},
469+
{901, "Code: 901. DB::Exception: I write, erase, rewrite\n" +
470+
"Erase again, and then\n" +
471+
"A poppy blooms\n" +
472+
" (by Katsushika Hokusai)",
473+
"Code: 901. DB::Exception: I write, erase, rewrite " +
474+
"Erase again, and then " +
475+
"A poppy blooms" +
476+
" (by Katsushika Hokusai)"}
477+
};
478+
}
479+
392480
@Test(groups = { "integration" })
393481
public void testAdditionalHeaders() {
394482
WireMockServer mockServer = new WireMockServer( WireMockConfiguration

0 commit comments

Comments
 (0)