[client-v2] Error handling

[chernser]

Summary

• Adds more information to IOException when reading data in RowBinary readers. Now time difference between error and last next() call is reported. Also row and column is reported .
• Adds reading 401, 403, 404 error body. Previously it was reported as server error with code 0 and no message was copied from body.

Closes #2803
Closes #2764

Checklist

Delete items not relevant to your PR:

• Closes #
• Unit and integration tests covering the common scenarios were added
• A human-readable description of the changes was provided to include in CHANGELOG
• For significant changes, documentation in https://github.com/ClickHouse/clickhouse-docs was updated with further explanations or tutorials

[Copilot]

Pull request overview

Improves client-v2 error handling by enriching binary reader decode exceptions with context (row/column and elapsed time since last next() call) and by surfacing HTTP error response bodies for common non-ClickHouse errors (e.g., 401/403/404), addressing opaque <Unreadable error message> failures (Issue #2803).

Changes:

• Enhance RowBinary reader failures with row/column context and time since last next() call.
• Parse and include HTTP error response bodies for 401/403/404 (including gzip-encoded bodies) instead of reporting code 0 with no message.
• Add integration tests covering 404 base-path handling and truncated/chunked binary decode failures.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.

File	Description
client-v2/src/test/java/com/clickhouse/client/HttpTransportTests.java	Adds integration tests for 404 body propagation and improved binary decode failure reporting.
client-v2/src/main/java/com/clickhouse/client/api/internal/HttpAPIClientHelper.java	Refactors error parsing to include non-ClickHouse HTTP error bodies and adjusts compression wrapping for 404.
client-v2/src/main/java/com/clickhouse/client/api/data_formats/internal/AbstractBinaryFormatReader.java	Adds row/column + elapsed-time context to binary reader exceptions.
clickhouse-data/src/main/java/com/clickhouse/data/ClickHouseColumn.java	Adds helper to format column index + name for improved error messages.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

DataTransferException is imported but never used in this file; Java treats unused imports as a compilation error. Remove the unused import (or use it if intended).

Suggested change

import com.clickhouse.client.api.DataTypeUtils;

[Copilot]

Charset and ArrayList are imported but not used anywhere in this class; unused imports will fail compilation. Please remove them (or use them if they were meant for parsing the response charset).

Suggested change

	import java.nio.charset.Charset;
	import java.nio.charset.StandardCharsets;
	import java.security.NoSuchAlgorithmException;
	import java.util.ArrayList;
	import java.nio.charset.StandardCharsets;
	import java.security.NoSuchAlgorithmException;

[Copilot]

body.read(...) can return -1 for an empty response body; passing a negative length into new String(buffer, 0, msgLen, ...) throws and will log a warning even for normal empty-body cases. Handle msgLen <= 0 explicitly (treat as no body) before constructing the string.

Suggested change

	msg = new String(buffer, 0, msgLen, StandardCharsets.UTF_8);
	if (msgLen > 0) {
	msg = new String(buffer, 0, msgLen, StandardCharsets.UTF_8);
	}

[Copilot]

Row numbering in the new error message is currently 0-based (row starts at -1 and is incremented before reading), which will report the first row as row 0 while column indexes are 1-based. Consider reporting row + 1 (or incrementing only after a row is successfully read) to make the message consistent and less confusing.

Suggested change

	+ " row " + row + " (time since last next call " + timeSinceLastNext() + ")";
	+ " row " + (row + 1) + " (time since last next call " + timeSinceLastNext() + ")";

[sonarqubecloud]

Quality Gate failed

Failed conditions
0.0% Coverage on New Code (required ≥ 80%)

See analysis details on SonarQube Cloud

[Copilot]

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

readNotClickHouseError() will fail to surface proxy/non-ClickHouse error bodies when compressServerResponse is enabled, because the entity may be wrapped in LZ4Entity and body.read() can throw ClientException (invalid LZ4 magic). Unlike readClickHouseError(), this method doesn't unwrap ClickHouseLZ4InputStream on invalid magic, so the error body is lost and the message falls back to "unknown server error". Consider adding the same unwrapping logic here (or avoiding LZ4 wrapping for non-ClickHouse errors in a more general way than hard-coding specific status codes).

[Copilot]

In the readError() exception handler, the constructed message is missing the "Code: " prefix used by the normal path, which can lead to inconsistent ServerException#getMessage() formatting for unreadable errors. Consider aligning the catch-path message format with readClickHouseError().

Suggested change

	String msg = String.format(ERROR_CODE_PREFIX_PATTERN, serverCode) + " <Unreadable error message> (transport error: " + httpResponse.getCode() + ")";
	String msg = "Code: " + String.format(ERROR_CODE_PREFIX_PATTERN, serverCode) + " <Unreadable error message> (transport error: " + httpResponse.getCode() + ")";

[Copilot]

assertBinaryReadFailureContainsColumnName() unconditionally calls thrown.printStackTrace(), which will spam test output even when the test passes and makes CI logs noisy. Consider removing it or only printing the stack trace when an assertion fails.

Suggested change

	thrown.printStackTrace();
	ClientException clientException = findCause(thrown, ClientException.class);
	Assert.assertNotNull(clientException,
	"Expected ClientException in cause chain, but was: " + thrown);
	Assert.assertTrue(containsMessageInCauseChain(thrown, "Reading column "),
	"Expected column information in failure message chain, but was: " + thrown);
	try {
	ClientException clientException = findCause(thrown, ClientException.class);
	Assert.assertNotNull(clientException,
	"Expected ClientException in cause chain, but was: " + thrown);
	Assert.assertTrue(containsMessageInCauseChain(thrown, "Reading column "),
	"Expected column information in failure message chain, but was: " + thrown);
	} catch (AssertionError e) {
	// Print the full cause chain only when the assertion fails to avoid noisy logs on success.
	thrown.printStackTrace();
	throw e;
	}

[Copilot]

readNotClickHouseError() uses the result of InputStream.read(...) as a length without handling -1 (EOF) or 0. When the response body is empty this can throw and gets logged as a warning, and it also treats an empty/blank body as a message instead of falling back to the default. Consider handling httpEntity == null, and treating msgLen <= 0 or blank content as "unknown server error" without throwing/logging.

Suggested change

	byte[] buffer = new byte[ERROR_BODY_BUFFER_SIZE];
	String msg = null;
	try {
	InputStream body = httpEntity.getContent();
	int msgLen = body.read(buffer, 0, buffer.length - 2);
	msg = new String(buffer, 0, msgLen, StandardCharsets.UTF_8);
	} catch (Exception e) {
	LOG.warn("Failed to read error message (queryId = " + queryId + ")", e);
	String msg = null;
	if (httpEntity != null) {
	byte[] buffer = new byte[ERROR_BODY_BUFFER_SIZE];
	try {
	InputStream body = httpEntity.getContent();
	if (body != null) {
	int msgLen = body.read(buffer, 0, buffer.length - 2);
	if (msgLen > 0) {
	String rawMsg = new String(buffer, 0, msgLen, StandardCharsets.UTF_8).trim();
	if (!rawMsg.isEmpty()) {
	msg = rawMsg;
	}
	}
	}
	} catch (Exception e) {
	LOG.warn("Failed to read error message (queryId = " + queryId + ")", e);
	}