Fix ClickHouseRawResult: surface in-band mid-stream server exceptions on the raw streaming path

[polyglotAI-bot]

Description

Fixes #475.

Stacked on #477 (which fixes #476). #477 repairs the shared in-band detector
(ExceptionTagAwareStream CRLF marker framing) and the native reader path. This PR builds the
raw / custom-FORMAT surface on top of that detector, so its base is the
polyglot/midstream-exception-crlf-marker branch — please merge #477 first. GitHub will
retarget this PR to main automatically once #477 merges; the diff below is purely the raw-path delta.

ExecuteRawResultAsync → ClickHouseRawResult handed the HTTP response body straight to the caller.
When a query fails after the 200 OK is committed and rows are already streaming (e.g. a throwIf
partway through a large FORMAT CSV/JSONEachRow/Arrow/Parquet result), the server appends an
in-band exception block to the body and drops the connection. The raw path never consulted the
X-ClickHouse-Exception-Tag header, so callers saw a System.Net.Http.HttpIOException
("The response ended prematurely") — or, with http_write_exception_in_output_format=1, the raw
__exception__ block spliced into their data — instead of a ClickHouseServerException. The native
ClickHouseDataReader path already handles this; the raw path did not.

Changes

• ExceptionTagAwareStream — add an opt-in throwAtEndOfStream mode (plus true-async ReadAsync
  overrides) that proactively raises the captured ClickHouseServerException when the inner stream ends
  (a 0-byte read) or drops (IOException) with an in-band block in the observed tail. The default
  (false) passive-observer mode used by the native reader is unchanged.
• ClickHouseRawResult — when the X-ClickHouse-Exception-Tag response header is present, wrap the
  body with throwAtEndOfStream: true across all four accessors (ReadAsStreamAsync,
  ReadAsByteArrayAsync, ReadAsStringAsync, CopyToAsync). Both raw entry points
  (ClickHouseClient.ExecuteRawResultAsync, ClickHouseCommand.ExecuteRawResultAsync) funnel through
  the single ClickHouseRawResult constructor, so both are covered. When the header is absent the
  accessors are byte-for-byte unchanged; ReadAsStringAsync preserves the framework's charset/BOM decoding.

Test

• ClickHouseRawResultMidStreamMockTests (no server): all four accessors raise
  ClickHouseServerException on an in-band block; all four return the full body when the tag is present
  but the query succeeded; the untagged path returns the body verbatim (even with __exception__ bytes present).
• ExceptionTagAwareStreamTests: parametrized over {sync Read, ReadByte, async ReadAsync(byte[]),
  async ReadAsync(Memory)} × {clean EOF, dropped connection}; a large-prefix (>4 KiB) case; and a contrast
  case confirming the default passive mode still never throws on its own.
• ClickHouseRawResultMidStreamTests.ExecuteRawResultAsync_MidStreamException_SurfacesServerException
  ([FromVersion(25, 11)]): drives a real committed-then-failed streamed FORMAT CSV query against a live
  server and asserts ExecuteRawResultAsync surfaces the server's error.

Focused suite (mid-stream + exception-tag + raw-result + accept-encoding) is green on net10.0 (78 passed).

Pre-PR validation gate

• Deterministic repro confirmed (live server + mocks)
• Root cause documented above
• Fix targets the root cause (single narrowest layer: the ClickHouseRawResult constructor, covering both entry points)
• Tests fail without the fix, pass with it
• No existing tests weakened
• CHANGELOG.md + RELEASENOTES.md updated
• Convention compliance verified per AGENTS.md

[alex-clickhouse]

I would strongly prefer integration tests over the unit tests we have. Also not entirely sure about the surface, eg the ReadAsStringAsync() and ReadBytes paths...

[codecov]

Codecov Report

❌ Patch coverage is 85.05747% with 13 lines in your changes missing coverage. Please review.

Files with missing lines	Patch %	Lines
...ickHouse.Driver/Formats/ExceptionTagAwareStream.cs	83.01%	8 Missing and 1 partial ⚠️
...ickHouse.Driver/ADO/Readers/ClickHouseRawResult.cs	88.23%	0 Missing and 4 partials ⚠️

📢 Thoughts on this report? Let us know!

[Copilot]

Pull request overview

This PR extends the existing mid-stream exception detection (via X-ClickHouse-Exception-Tag) to the raw/custom-FORMAT result surface (ExecuteRawResultAsync → ClickHouseRawResult) by introducing an opt-in “throw at end-of-stream” mode in ExceptionTagAwareStream, and adds unit/integration coverage to ensure raw accessors surface ClickHouseServerException rather than HttpIOException.

Changes:

• Add throwAtEndOfStream behavior (and async read overrides) to ExceptionTagAwareStream so it can proactively throw a captured ClickHouseServerException on EOF / IOException.
• Wrap ClickHouseRawResult response bodies with ExceptionTagAwareStream(..., throwAtEndOfStream: true) when the exception-tag header is present, across all four accessors.
• Add mock-based and live-server tests for raw-result mid-stream exceptions; update changelog + release notes.

Reviewed changes

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

Show a summary per file

File	Description
ClickHouse.Driver/Formats/ExceptionTagAwareStream.cs	Adds opt-in EOF/IOException-triggered throwing + async read overrides to surface mid-stream server exceptions for direct stream consumers.
ClickHouse.Driver/ADO/Readers/ClickHouseRawResult.cs	Wraps raw response content in ExceptionTagAwareStream when X-ClickHouse-Exception-Tag is present across all accessors.
ClickHouse.Driver.Tests/Formats/ExceptionTagAwareStreamTests.cs	Adds test matrix validating throw-at-EOF mode across sync/async read APIs and termination modes.
ClickHouse.Driver.Tests/ADO/MidStreamExceptionTests.cs	Adds mock and integration tests asserting raw-result accessors surface ClickHouseServerException mid-stream.
CHANGELOG.md	Documents the raw-result mid-stream exception fix.
RELEASENOTES.md	Documents the raw-result mid-stream exception fix.

[polyglotAI-bot]

Correct on the facts, and I've taken the second option you outlined (state the guarantee accurately) rather than adding filtering.

The wrapper observes bytes rather than withholding them, so a caller reading the stream incrementally — or a CopyToAsync destination — can hold part of the result, including the raw __exception__ block, before the exception is raised at end-of-stream. That is inherent to streaming: bytes already handed to the consumer cannot be recalled. Note the two buffered accessors are not affected — ReadAsByteArrayAsync/ReadAsStringAsync materialize the body internally and throw instead of returning it, so no partial or block-bearing body reaches the caller there.

I've corrected the changelog and release note, and added the caveat to the ReadAsStreamAsync/CopyToAsync XML docs so the guarantee is stated where callers will actually see it.

Byte-level filtering (buffer a marker-sized tail, truncate at the opening marker) remains a real option for a stronger guarantee. I've put it to @alex-clickhouse rather than build it speculatively, since he'd flagged this same surface as something he wasn't sure about — leaving this thread open for that decision.

[polyglotAI-bot]

Thanks — both points were well placed, and the second one turned up a real bug.

Integration tests. Replaced the mock-HTTP ClickHouseRawResultMidStreamMockTests with real-server coverage in ClickHouseRawResultMidStreamTests, parametrized over all four accessors (ReadAsStreamAsync, ReadAsByteArrayAsync, ReadAsStringAsync, CopyToAsync):

• ExecuteRawResultAsync_MidStreamException_SurfacesServerException — committed 200 OK, then throwIf mid-stream; asserts ClickHouseServerException carrying the server message and ErrorCode 395.
• ExecuteRawResultAsync_SuccessfulQuery_ReturnsCompleteBody — contrast: a successful query still returns its body unmodified.
• ExecuteRawResultAsync_BufferedAccessorReadTwice_ReturnsSameBody — see below.

I checked these actually have teeth: all four mid-stream cases fail against the unpatched reader with exactly the reported HttpIOException: The response ended prematurely, and pass with the fix.

The only unit tests I kept are the lower-level ExceptionTagAwareStreamTests ones, which pin mechanics a live server won't produce on demand — a dropped-connection IOException termination vs. a clean EOF, and the >4 KiB ring-buffer boundary. Happy to drop those too if you'd rather.

The surface — you were right to poke at it. Exercising ReadAsStringAsync/ReadAsByteArrayAsync for real exposed a regression this PR had introduced. Both wrapped the content stream in a using, and disposing the wrapper closed the response's cached content stream, so a second ReadAsByteArrayAsync()/ReadAsStringAsync() threw ObjectDisposedException where HttpContent buffers and hands the body back again.

That matters more than it first looks: the server sends X-ClickHouse-Exception-Tag on every response — I confirmed on 26.5.1.882 that it is not gated on http_write_exception_in_output_format — so the wrapped path is the default for all raw results on 25.11+, and this would have hit ordinary successful reads, not just the failure case. Fixed by buffering the body and leaving the response-owned stream open (it is still released when the result is disposed), with a regression test that fails without the fix.

So the surface is now:

• ReadAsByteArrayAsync / ReadAsStringAsync — buffered: they throw rather than hand back a truncated body, and repeat reads return the same body.
• ReadAsStreamAsync / CopyToAsync — streaming: the exception surfaces once the end of the response is reached.

On the streaming pair (also Copilot's point): the wrapper observes bytes, it does not withhold them, so a caller parsing incrementally — or a CopyToAsync destination — may already hold part of the result, including the raw __exception__ block, before the throw. That is inherent to streaming; bytes already handed over can't be recalled. I corrected the changelog/release-note wording, which over-claimed here, and spelled it out in the XML docs. If you'd prefer the stream to actually withhold the block (buffer a marker-sized tail and truncate at the opening marker), that's a genuine option — I deliberately didn't build it speculatively on a surface you'd flagged as uncertain. Happy to add it if you want that stronger guarantee.

One boundary worth flagging either way: detection needs the marker in plaintext, so on the AutomaticDecompression = None + explicit AcceptEncoding raw-export path the body is compressed on the wire and the marker won't match. The merged reader path from #476 narrows the same way.

[cursor]

Cursor Bugbot has reviewed your changes using high effort and found 1 potential issue.

^{❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.}

^{Reviewed by Cursor Bugbot for commit 7b89e2b. Configure here.}

[polyglotAI-bot]

Rebase/merge housekeeping: main had advanced (notably #477, the sibling streaming-path mid-stream-exception fix, plus the perf work on this same wrapper), so this branch had gone CONFLICTING. Merged origin/main in — now MERGEABLE again. No approval existed, so none was dismissed.

Conflicts and how they were resolved:

• ClickHouse.Driver/Formats/ExceptionTagAwareStream.cs — both sides changed the constructor and RecordBytes. Kept both features rather than either side:
  • constructor is now (Stream innerStream, string exceptionTag, bool leaveOpen = false, bool throwAtEndOfStream = false). Both existing call sites already use named arguments (ClickHouseDataReader → leaveOpen: true, ClickHouseRawResult → throwAtEndOfStream: true), so neither needed changing.
  • kept main's span-based RecordBytes(ReadOnlySpan<byte>) + byte[] overload, its Read(Span<byte>) override, and its leaveOpen-aware Dispose.
  • kept this PR's throwAtEndOfStream end-of-stream/IOException detection, and also applied it to main's new Read(Span<byte>) so every read API on the wrapper honours the same contract (leaving it out would have let a sync span read on the raw path silently miss an in-band exception).
• CHANGELOG.md / RELEASENOTES.md — both sides appended bug-fix entries; kept both sets, no entry dropped.

Verification after the merge (net10.0, devbox ClickHouse server): build 0 errors; the focused mid-stream / raw-result / ExceptionTagAwareStream suite 69/69 green (including the real-server mid-stream integration cases across all four accessors); ADO + Formats 1058/1058; full suite 9704 passed / 0 failed / 142 pre-existing skips.

No behavioral change was introduced by the merge beyond the union described above.