Skip to content

TCP Q1/Q2: public exception hierarchy and named server error codes - #590

Draft
alex-clickhouse wants to merge 2 commits into
tcp/epic-p-observabilityfrom
tcp/epic-q1-exceptions
Draft

TCP Q1/Q2: public exception hierarchy and named server error codes#590
alex-clickhouse wants to merge 2 commits into
tcp/epic-p-observabilityfrom
tcp/epic-q1-exceptions

Conversation

@alex-clickhouse

Copy link
Copy Markdown
Collaborator

Stacked on #589 (tcp/epic-p-observability). Review only the last commit.

Closes Q1 (exception type hierarchy) and Q2 (server error to typed .NET mapping).

The problem

Neither of the two types that carry the driver's semantics could be caught by name — both were internal sealed. Everything else escaping a public method was an undifferentiated BCL type with no common base, so the only way to distinguish a server rejection from a broken socket was GetType().Name.

The shape

An abstract ClickHouseTcpException : DbException with private protected constructors, so the hierarchy is closed at three leaves:

Type Means IsTransient
ClickHouseTcpServerException the server reported an error by error code
ClickHouseTcpProtocolException the bytes did not match the protocol false
ClickHouseTcpTransportException the connection failed true

Argument validation, disposal and cancellation keep their framework types — those are the caller's mistakes, not the database's.

DbException was chosen over plain Exception for IsTransient and ErrorCode, which retry frameworks probe for, and because the shipped HTTP ClickHouseServerException already derives from it.

Q2: codes, not subclasses

A 26.7 server names about 660 error codes, so subclassing is not on. ClickHouseErrorCode names the 54 worth branching on; anything else reads as Unknown with RawCode intact. Every value was read off a live 26.7.1 server rather than transcribed.

catch (ClickHouseTcpServerException e) when (e.Code == ClickHouseErrorCode.UnknownTable)
catch (ClickHouseTcpServerException e) when (e.IsTransient)

IsTransient covers load and contention, and deliberately excludes MemoryLimitExceeded and TooSlow: both repeat for the same query at the same size, so retrying unchanged just fails again.

Wrapping, and what is not wrapped

The transport exceptions are never constructed by this code — there is no throw new IOException anywhere in the project — so all 338 throw sites were never the work. They enter through four calls into the socket: the two reads in ReadBuffer, the write in ClickHouseBinaryWriter, and connect plus the TLS handshake.

ObjectDisposedException is not wrapped. A socket or stream only raises it because this process disposed it — the pool aborting a connection, a session disposing under a live read, the caller disposing the client. Calling that a transient transport failure invites a retry that cannot help, and it would have masked TlsParameters' refusal to fall back from pinned roots to the machine trust store.

For the same reason TlsParameters now builds its handshake options separately from handshaking, and the connect runs that first. A ConfigureTls hook that fails loading a client certificate off disk throws IOException, and run inside the connect that read as a network error worth retrying.

InvalidDataException is folded into the protocol exception, including the frames raised by the shared CompressionFrame, which cannot name a TCP type and so is translated at the frame reader.

Tests

3351 pass, 5 skipped, against a real 26.7.1 server.

New integration coverage asserts four real server errors map to their named code (60/62/46/81, each verified against the server before being written down), a max_execution_time breach proving IsTransient end to end, a wrong password reaching AuthenticationFailed, and connect-refused producing a transport exception that names the endpoint and keeps the SocketException. Unit tests cover only what a live server cannot produce on demand: an unnamed code, each arm of the transient table, and the two disposal paths.

Two things caught during the change, both worth noting because they were caught by existing machinery rather than by reading:

  • TcpActivity.cs built the OTel db.response.status_code tag from .Code, which silently became the enum — the tag would have started emitting AuthenticationFailed instead of 516. An existing test failed on it.
  • Nine assertions compared .Code against an int. NUnit2021/NUnit2041 failed the build rather than letting them mis-compare at runtime.

Not in scope

No PublicAPI.Tcp.txt and no XML doc generation — the project has neither today, and both belong with R1/R4, which own the surface. The <exception> tags added here are therefore source-only for now.

No changelog fragment, consistent with the rest of this stack.

Copilot AI 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.

Pull request overview

Adds a public TCP exception hierarchy, named server error codes, transport wrapping, and protocol-failure classification.

Changes:

  • Introduces typed server, protocol, and transport exceptions.
  • Maps selected ClickHouse error codes and transient status.
  • Updates diagnostics and test coverage for the new behavior.

Reviewed changes

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

Show a summary per file
File Description
ClickHouse.Driver.Tcp/Types/Codecs/StringColumnCodec.cs Uses the public protocol exception.
ClickHouse.Driver.Tcp/Types/Codecs/NestedColumnCodec.cs Updates offset-validation exceptions.
ClickHouse.Driver.Tcp/Types/Codecs/MapColumnCodec.cs Updates map protocol exceptions.
ClickHouse.Driver.Tcp/Types/Codecs/LowCardinalityColumnCodec.cs Updates wire-validation exceptions.
ClickHouse.Driver.Tcp/Types/Codecs/JsonStringColumnCodec.cs Updates serialization-version failure.
ClickHouse.Driver.Tcp/Types/Codecs/DynamicColumnCodec.cs Updates Dynamic protocol failures.
ClickHouse.Driver.Tcp/Types/Codecs/ArrayColumnCodec.cs Updates array offset failures.
ClickHouse.Driver.Tcp/Protocol/TransportFailure.cs Centralizes transport classification and wrapping.
ClickHouse.Driver.Tcp/Protocol/TlsParameters.cs Separates TLS option construction from negotiation.
ClickHouse.Driver.Tcp/Protocol/ReadBuffer.cs Wraps read and EOF failures.
ClickHouse.Driver.Tcp/Protocol/Handshake.cs Uses typed handshake exceptions.
ClickHouse.Driver.Tcp/Protocol/ClickHouseTcpConnection.cs Propagates the new hierarchy through operations.
ClickHouse.Driver.Tcp/Protocol/ClickHouseProtocolException.cs Removes the old internal exception.
ClickHouse.Driver.Tcp/Protocol/ClickHouseBinaryWriter.cs Wraps write failures.
ClickHouse.Driver.Tcp/Protocol/ClickHouseBinaryReader.cs Classifies malformed wire values.
ClickHouse.Driver.Tcp/Format/BlockReader.cs Updates block protocol failures.
ClickHouse.Driver.Tcp/Exceptions/ClickHouseTcpTransportException.cs Adds the transport exception leaf.
ClickHouse.Driver.Tcp/Exceptions/ClickHouseTcpServerException.cs Publishes server errors and code mapping.
ClickHouse.Driver.Tcp/Exceptions/ClickHouseTcpProtocolException.cs Adds the protocol exception leaf.
ClickHouse.Driver.Tcp/Exceptions/ClickHouseTcpException.cs Adds the shared DbException base.
ClickHouse.Driver.Tcp/Exceptions/ClickHouseErrorCode.cs Defines named server error codes.
ClickHouse.Driver.Tcp/Diagnostic/TcpActivity.cs Preserves raw error codes in telemetry.
ClickHouse.Driver.Tcp/Compression/CompressedFrameReader.cs Translates malformed compression frames.
ClickHouse.Driver.Tcp/Client/MetadataCallbackBridge.cs Updates metadata protocol failures.
ClickHouse.Driver.Tcp.Tests/Types/QBitColumnCodecTests.cs Updates truncated-read expectations.
ClickHouse.Driver.Tcp.Tests/Types/NestedColumnCodecTests.cs Updates protocol assertions.
ClickHouse.Driver.Tcp.Tests/Types/MapColumnCodecTests.cs Updates protocol assertions.
ClickHouse.Driver.Tcp.Tests/Types/LowCardinalityColumnCodecTests.cs Updates protocol assertions.
ClickHouse.Driver.Tcp.Tests/Types/JsonStringColumnCodecTests.cs Updates protocol assertions.
ClickHouse.Driver.Tcp.Tests/Types/DynamicColumnCodecTests.cs Updates version-failure assertion.
ClickHouse.Driver.Tcp.Tests/Types/ArrayColumnCodecTests.cs Updates protocol assertions.
ClickHouse.Driver.Tcp.Tests/Protocol/TlsParametersTests.cs Covers separated TLS configuration.
ClickHouse.Driver.Tcp.Tests/Protocol/ReadBufferTests.cs Covers wrapping and disposal behavior.
ClickHouse.Driver.Tcp.Tests/Protocol/HandshakeTests.cs Updates handshake exception assertions.
ClickHouse.Driver.Tcp.Tests/Protocol/CompressedSendFailureTests.cs Expects transport exceptions on send failure.
ClickHouse.Driver.Tcp.Tests/Protocol/ClickHouseTcpConnectionTests.cs Updates ping and handshake failures.
ClickHouse.Driver.Tcp.Tests/Protocol/ClickHouseTcpConnectionQueryTests.cs Updates query failure assertions.
ClickHouse.Driver.Tcp.Tests/Protocol/ClickHouseTcpConnectionInsertTests.cs Updates insert failure assertions.
ClickHouse.Driver.Tcp.Tests/Protocol/ClickHouseBinaryReaderWriterTests.cs Tests typed reader failures.
ClickHouse.Driver.Tcp.Tests/Integration/ClickHouseTcpTracingIntegrationTests.cs Updates tracing error types.
ClickHouse.Driver.Tcp.Tests/Integration/ClickHouseTcpSessionIntegrationTests.cs Updates session error assertions.
ClickHouse.Driver.Tcp.Tests/Integration/ClickHouseTcpParameterIntegrationTests.cs Updates parameter error assertions.
ClickHouse.Driver.Tcp.Tests/Integration/ClickHouseTcpLoggingIntegrationTests.cs Updates logged exception types.
ClickHouse.Driver.Tcp.Tests/Integration/ClickHouseTcpExceptionIntegrationTests.cs Adds end-to-end exception mapping tests.
ClickHouse.Driver.Tcp.Tests/Integration/ClickHouseTcpConnectionQueryIntegrationTests.cs Verifies typed query errors.
ClickHouse.Driver.Tcp.Tests/Integration/ClickHouseTcpConnectionIntegrationTests.cs Verifies authentication and transport errors.
ClickHouse.Driver.Tcp.Tests/Integration/ClickHouseTcpConnectionInsertIntegrationTests.cs Verifies typed insert errors.
ClickHouse.Driver.Tcp.Tests/Integration/ClickHouseTcpClientIntegrationTests.cs Updates client error assertions.
ClickHouse.Driver.Tcp.Tests/Format/BlockInfoTests.cs Updates block-info exception assertion.
ClickHouse.Driver.Tcp.Tests/Exceptions/ClickHouseTcpServerExceptionTests.cs Tests code and transient mapping.
ClickHouse.Driver.Tcp.Tests/Diagnostic/TcpActivityTests.cs Verifies raw telemetry status codes.
ClickHouse.Driver.Tcp.Tests/Compression/CompressedFrameStreamingTests.cs Updates compression protocol assertions.
ClickHouse.Driver.Tcp.Tests/Client/MetadataCallbackBridgeTests.cs Updates metadata exception assertions.

💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread ClickHouse.Driver.Tcp/Protocol/ClickHouseBinaryWriter.cs Outdated
Comment thread ClickHouse.Driver.Tcp/Exceptions/ClickHouseTcpException.cs
Comment thread ClickHouse.Driver.Tcp/Exceptions/ClickHouseTcpException.cs
@codecov

codecov Bot commented Aug 26, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@alex-clickhouse
alex-clickhouse force-pushed the tcp/epic-q1-exceptions branch 2 times, most recently from 202eb3a to afc96db Compare August 26, 2026 15:40
@alex-clickhouse
alex-clickhouse force-pushed the tcp/epic-q1-exceptions branch 3 times, most recently from 6f1328b to caecf6f Compare August 28, 2026 09:03
@alex-clickhouse
alex-clickhouse force-pushed the tcp/epic-q1-exceptions branch 2 times, most recently from 46f149c to bba9dd5 Compare August 28, 2026 11:25
alex-clickhouse and others added 2 commits August 28, 2026 18:28
…odes

Nothing a caller could catch described what went wrong. The two types that
carried the driver's own semantics were internal, so the only way to tell a
server rejection from a broken socket was to compare GetType().Name; every
other failure arrived as an undifferentiated BCL type with no common base.

Three sealed types under an abstract ClickHouseTcpException : DbException,
whose constructors are private protected so the set stays closed at three:

  ClickHouseTcpServerException     the server reported an error
  ClickHouseTcpProtocolException   the bytes did not match the protocol
  ClickHouseTcpTransportException  the connection failed

Argument validation, disposal and cancellation keep their framework types.
Those are the caller's own mistakes, not the database's.

The server exception gains RawCode and a ClickHouseErrorCode naming the 54
codes worth branching on, of the roughly 660 a 26.7 server defines; an
unnamed code reads as Unknown with RawCode intact. IsTransient covers load
and contention, and deliberately excludes MemoryLimitExceeded and TooSlow,
which repeat for the same query. HResult is set so the inherited
DbException.ErrorCode carries the same number the HTTP client reports.

The transport types are never constructed here, so they are wrapped where
they enter: the two reads in ReadBuffer, the write in ClickHouseBinaryWriter,
and connect plus the TLS handshake. ObjectDisposedException is not among
them. A socket only raises it because this process disposed it -- the pool
aborting, a session disposing under a live read -- so calling that a
transient transport failure would invite a retry that cannot help.

For the same reason TlsParameters now builds its handshake options
separately from handshaking, and the connect runs that first. A
ConfigureTls hook that fails loading a client certificate off disk throws
IOException, and inside the connect that read as a network error to retry.

InvalidDataException is folded into the protocol exception, including the
frames raised by the shared CompressionFrame, which cannot name a TCP type.

Co-Authored-By: Claude <noreply@anthropic.com>
… read-path catch contract

The compressed frame writer and reader each drive a second writer/read buffer over an internal
adapter stream. Neither is the socket, so translating a failure there as a transport failure
relabels the layer's own errors: a caller-supplied IClickHouseCompressor throwing IOException from
Encode was reported as a transient network failure. Both take a flag, false at the adapter sites.
The genuine socket I/O below them runs through the raw reader and writer, which translate.

Four wire-payload checks reported a framework type the caller cannot catch as
ClickHouseTcpException: the Variant discriminators mode and out-of-range discriminator, the Dynamic
out-of-range discriminator, and a server protocol revision below the supported minimum. The block
reader and the Dynamic state prefix also resolve a codec from a type name read off the wire, so a
type this client cannot read escaped as FormatException or NotSupportedException; both translate,
as the insert path already did. Type resolution keeps those types where it is reached from a
Create(TypeNode) factory.

Co-Authored-By: Claude <noreply@anthropic.com>
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.

2 participants