Skip to content

fix(client-v2,jdbc-v2): handle tagged errors in response streams - #3097

Open
jjj-n wants to merge 6 commits into
ClickHouse:mainfrom
jjj-n:codex/issue-3077-stream-timeout
Open

jjj-n wants to merge 6 commits into
ClickHouse:mainfrom
jjj-n:codex/issue-3077-stream-timeout

Conversation

@jjj-n

@jjj-n jjj-n commented Sep 3, 2026

Copy link
Copy Markdown
Contributor

Summary

ClickHouse can append a tagged exception frame to an HTTP 200 response after streaming result data. This change detects and validates that frame so callers receive the server error when they reach it.

  • Validate the frame against X-ClickHouse-Exception-Tag, its UTF-8 message byte length, and its end marker. Preserve ordinary payload bytes and mismatched markers.
  • Keep normal reads demand-driven so a small response does not release its pooled connection prematurely.
  • Report complete server errors even when the server terminates chunked HTTP framing without a final zero-length chunk. Abort the affected connection after a complete frame.
  • Deliver already-decoded binary rows before reporting a prefetch error on the next advance. Preserve available LZ4 block data before reading another block.
  • Map streamed server error 159 to JDBC SQLTimeoutException / HYT00, retaining vendor code and cause. Retain the existing error-60 mapping to 42S02.
  • Document the behavior in CHANGELOG.md and docs/features.md.

Closes #2702
Closes #3077

Compatibility

No existing public method signatures, configuration defaults, or successful result formats are changed by this PR. HEADER_EXCEPTION_TAG and SQL_STATE_TIMEOUT are additive constants. Tagged server failures intentionally become exceptions instead of result data. The branch incorporates upstream main through ae682fcbd; its socket-buffer documentation and defaults are retained.

Validation

On the merge with current main (2026-09-18), Maven reactor verify completed successfully with JDK 17.0.19, Maven 3.9.16, UTC, and integration tests skipped:

Module Tests Failures / errors Skipped
clickhouse-data 1,673 0 113
clickhouse-client 133 0 0
client-v2 701 0 0
jdbc-v2 1,896 0 0
Total 4,403 0 113

Command: mvn -t <local-jdk17-toolchains.xml> -pl jdbc-v2 -am -Duser.timezone=UTC -DskipITs=true -Dmaven.javadoc.skip=true verify.

The live-server integration tests could not be rerun on this merge because local Docker Desktop failed to start while initializing its Inference manager. No full integration-suite pass is claimed. GitHub CI still requires its own successful run.

The focused coverage includes split exception markers, mismatched tags, incomplete and invalid frames, UTF-8 lengths, compressed responses, interrupted HTTP chunk framing, last-row delivery, and JDBC error mapping.

On 2026-09-11, both originally reported integration failures (StatementTest#testConnectionExhaustion and StatementTest#testUpdateQueryWithResultSet) passed against ClickHouse 26.3.33.24 after the fixes. This historical result does not substitute for validation on the current merge.

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Stale Bugbot comment from a previous run.

@chernser chernser left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are two tests failing:

[INFO] 
[ERROR] Failures: 
[ERROR]   StatementTest.testConnectionExhaustion:653 Exception expected
[ERROR]   StatementTest.testUpdateQueryWithResultSet:1333 Connection seems closed when should not expected [true] but found [false]
[INFO] 
[ERROR] Tests run: 469, Failures: 2, Errors: 0, Skipped: 0
[INFO] 


I've tested locally with the command:

~/tmp/clickhouse-java codex/issue-3077-stream-timeout$  mvn -DclickhouseVersion=26.3 clean verify

Limit normal tagged-response reads to the caller's requested length so parsing result metadata does not release pooled HTTP connections prematurely. Preserve exception-marker lookahead and add small-read regression coverage.
Preserve tagged-response streaming documentation alongside upstream query format settings. Verified client-v2 and jdbc-v2 unit tests and both connection-pool regression tests against ClickHouse 26.3.

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes using high effort and found 2 potential issues.

Fix All in Cursor

Reviewed by Cursor Bugbot for commit 19dca3a. Configure here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[jdbc-v2] Handle query timeout in the middle of stream [client-v2] Handle new error tag from ClickHouse

2 participants