ClickHouse / ClickHouse/clickhouse-go
HTTP transport ignores X-ClickHouse-Exception-Code; HTTP 200 with error body in exec/insert path treated as success
- Dominant language
- Go
- Stars
- 3.3k
- Forks
- 684
- Avg merge
- 2d 3h
- Merged PRs (30d)
- 14
Description
## Description
ClickHouse server can respond with HTTP `200 OK` while reporting a query error via the `X-ClickHouse-Exception-Code` response header (and an error message in the body). This happens, for example:
- When the server has already begun streaming response headers (chunked transfer) before encountering the error mid-stream — by then the status line has been flushed and cannot be changed.
- When a distributed DDL (e.g. `ON CLUSTER`) triggers a remote-shard error that is surfaced through a Native-format result block rather than as a `__exception__` body marker. See the previously closed user report #1398 (`Sorting key contains nullable columns` on `ON CLUSTER`: `HTTP/1.1 200 OK` with the error encoded inside the Native body, no `__exception__` marker; the only documented workaround was a server-side `SET http_wait_end_of_query=1`, not a client fix).
This driver's HTTP transport never inspects `X-ClickHouse-Exception-Code`. A grep across the repo for that header returns zero hits.
In `conn_http.go::httpConnect.executeRequest` (around lines 705–724) the only success/failure check is the HTTP status code:
```go
func (h *httpConnect) executeRequest(req *http.Request) (*http.Response, error) {
if h.client == nil {
return nil, sqldriver.ErrBadConn
}
resp, err := h.client.Do(req)
if err != nil {
return nil, err
}
if resp.StatusCode != http.StatusOK {
defer discardAndClose(resp.Body)
msgBytes, err := h.readRawResponse(resp)
if err != nil {
return nil, fmt.Errorf(\"[HTTP %d] failed to read response: %w\", resp.StatusCode, err)
}
return nil, fmt.Errorf(\"[HTTP %d] response body: \\\"%s\\\"\", resp.StatusCode, string(msgBytes))
}
return resp, nil
}
```
`readData` (lines 422–470) does scan the response body for a `__exception__` marker, which catches *some* mid-stream exceptions, but:
1. It only fires on the read path that decodes a `proto.Block`. The exec path is `conn_http_exec.go::httpConnect.exec`:
```go
func (h *httpConnect) exec(ctx context.Context, query string, args ...any) error {
options := queryOptions(ctx)
query, err := bindQueryOrAppendParameters(true, &options, query, h.handshake.Timezone, args...)
if err != nil {
return err
}
res, err := h.sendQuery(ctx, query, &options, nil) //nolint:bodyclose // false positive
if err != nil {
return err
}
defer discardAndClose(res.Body)
return nil
}
```
The body is discarded; on HTTP 200 `Exec`/DDL/raw insert flush returns `nil` regardless of `X-ClickHouse-Exception-Code` or body content.
2. It does not handle responses whose body is a valid Native block that *encodes* error rows (the #1398 case): the block decodes successfully, no `__exception__` marker is present, and the driver returns no error.
3. It never consults `X-ClickHouse-Exception-Code`, which the server sets unconditionally when an exception occurred — that header would be the most reliable signal regardless of body format or status code.
### Net effect
`Exec` / `INSERT` calls that the server reports as failed via `X-ClickHouse-Exception-Code` + body-encoded error can be observed by the application as `nil` error, risking silent data loss or silent schema-change failure. This is the same class of bug reported in #1398, which was closed via a server-side workaround (`SET http_wait_end_of_query=1`) rather than a client fix; that workaround is not the driver default and is not documented in the driver.
## ClickHouse server version
Code analysis only; not verified against a running server (no server reachable in this environment). The behavior is described against the current `main` of this repository and is consistent with server v23.* through v25.* per the source bug.
## Reproduction
Minimal Go test using a stub HTTP server that mimics ClickHouse's documented behavior of returning `200 OK` with `X-ClickHouse-Exception-Code` and an error body (e.g. the chunked-streaming case where status was already flushed). This is the shape recommended for `conn_http_test.go`:
```go
package clickhouse
import (
\"context\"
\"net/http\"
\"net/http/httptest\"
\"strings\"
\"testing\"
\"github.com/ClickHouse/clickhouse-go/v2\"
)
func TestHTTPExecReturnsErrorOnExceptionCodeHeader(t *testing.T) {
srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
// Simulate the server having already started streaming the body before
// detecting an error mid-stream. The status line was already 200 OK;
// the error is surfaced via X-ClickHouse-Exception-Code + body text.
w.Header().Set(\"X-ClickHouse-Exception-Code\", \"44\")
w.Header().Set(\"X-ClickHouse-Server-Display-Name\", \"stub\")
w.WriteHeader(http.StatusOK)
_, _ = w.Write([]byte(\"Code: 44. DB::Exception: Sorting key contains nullable columns\"))
}))
defer srv.Close()
conn, err := clickhouse.Open(&clickhouse.Options{
Addr: []string{strings.TrimPrefix(srv.URL, \"http://\")},
Protocol: clickhouse.HTTP,
})
if err != nil {
t.Fatalf(\"open: %v\", err)
}
defer conn.Close()
err = conn.Exec(context.Background(),
\"CREATE TABLE t (a Nullable(String)) ENGINE = MergeTree PRIMARY KEY a\")
if err == nil {
t.Fatal(\"expected error when server returned X-ClickHouse-Exception-Code, got nil\")
}
}
```
**Expected:** `Exec` returns a non-nil error carrying ClickHouse exception code `44`.
**Actual:** `Exec` returns `nil`. The `X-ClickHouse-Exception-Code` header is never inspected; `executeRequest` returns the response as success because `StatusCode == 200`, and `httpConnect.exec` then calls `discardAndClose(res.Body)` and returns `nil`.
The original user environment that produced this (`ON CLUSTER` DDL with `Sorting key contains nullable columns` on remote shards, returning 200 + Native-encoded error rows) is described in #1398.
## Suggested fix
In `conn_http.go::httpConnect.executeRequest`, after `h.client.Do(req)`:
1. Inspect the response for `X-ClickHouse-Exception-Code` (and the newer `X-ClickHouse-Exception-Tag`). If the header is present and non-empty, treat the response as an error regardless of HTTP status: read the body, surface it as a `*Exception` with the parsed code.
2. Optionally, default `http_wait_end_of_query=1` on the `Exec`/`Ping` paths where streaming is not needed, so the server can emit a proper non-200 status when an error is detected before the body is flushed.
3. Add a regression test in `conn_http_test.go` that exercises a stub HTTP server returning `200 OK` + `X-ClickHouse-Exception-Code: 44` + a non-`__exception__` body, asserting that `Exec` returns a non-nil error.
## Related
- Closed manifestation in this repo: #1398 (`Error in server, but 200 response`) — closed via server-side `SET http_wait_end_of_query=1`, not a client fix.
- Related ambiguity: #1468 (`why ExceptionWhileProcessing didn't return err`).
- Source bug in the Rust client: ClickHouse/clickhouse-rs#255
- Central tracking issue: ClickHouse/integrations-ai-playground#147
Contributor guide
Research direction
Start in conn_http.go::httpConnect.executeRequest and follow the response flow into conn_http_exec.go::httpConnect.exec, where successful HTTP 200 bodies are discarded. Read the existing response and exception handling, then add the regression case to conn_http_test.go using the described stub response. Done means Exec returns a non-nil error containing exception code 44 for HTTP 200 responses with the exception header.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- go
- Domain
- backend-api-design, database
- Issue type
- Bug
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Quiet
- Clarity
- Clearly specified
- Newbie friendliness
- 64/100