fix(api): map ClickHouse query failures by class, not HTTP status

[EricAndrechek]

Fixes #403. Fixes #271. Part of #613 (workstream A2). Stacked on #619 (feat/ch-error-classes), which adds the classifier this uses.

What changes

ClickHouse answers a syntax error, a missing grant and an overloaded server alike with HTTP 500. Before this PR:

• /v1/ops/query keyed on that status, so a bad statement came back as 502 (query API maps ClickHouse caller-side errors (e.g. ACCESS_DENIED) to HTTP 502 instead of 4xx #403).
• /v1/query and pipes returned a flat 500, which the SDK retried (bug(api): request-caused query errors return HTTP 500 + retryable:true (should be 4xx, non-retryable) #271).

All the query paths now class the failure with chconn.Classify through one helper, writeCHError (internal/api/ch_errors.go), so they cannot drift apart again:

Class	Status	code	retryable
Rejected (syntax, unknown table/column/identifier, type mismatch, any unlisted code)	400	clickhouse.rejected	false
Rows/bytes limits (158, 307, 396); the role's own time cap (159/160) or memory cap (241) on /v1/query	400	clickhouse.limit_exceeded	false
Denied: ACCESS_DENIED (497)	403	clickhouse.access_denied	false
Denied: anything else (credentials, user, database, a proxy's codeless 401/403)	502	clickhouse.misconfigured	false
Unavailable	503 + Retry-After: 5	clickhouse.unavailable	true
Unknown	500 (native) / 502 (proxy)	clickhouse.unknown	true

Also in this PR:

• Error envelope. The existing {"error": …} envelope (internal/api/errors.go) gains code and retryable on these responses. It is additive: no new envelope, and the code names are namespaced as refactor(errors): typed error kinds for stable machine-readable codes #539 proposes.
• SDK. clients/ts/src/errors.ts takes the server's code and retryable when present, and falls back to HTTP_<status> and "5xx retries" otherwise. There is no clients/go on this base.
• Other paths.
  • The raw-SQL proxy's 64 MiB overflow stays 502 but is now retryable: false, since the same query overflows again.
  • POST /v1/ops/schema/refresh against an unreachable ClickHouse answers 503 + Retry-After instead of 500.
• Role time cap. A capped query now runs with no context deadline, and WaveHouse cancels it two seconds past the cap. Why: clickhouse-go v2.48.0 (context.go:238-241) overwrites max_execution_time with deadline+5s for any deadline over 1s. As it was, a cap overrun came back as a bare DeadlineExceeded, which looks the same as a wait for a pooled connection or a dial timeout, both of which are outages. Now ClickHouse enforces the cap itself and reports TIMEOUT_EXCEEDED. Anything else, including the backstop cancel, is 503. TestQueryErrors_TimeCapReachesClickHouse pins both sides of that driver behaviour against a real ClickHouse.

Why ACCESS_DENIED is a 403, and credential failures a 502

The query paths run as the ClickHouse user in the tenant's settings, not as the caller. So ACCESS_DENIED is WaveHouse's configuration in one sense. It is still a verdict on this statement:

• ClickHouse understood the statement and refused it;
• the same statement is refused every time;
• other statements from the same caller succeed.

That is a 403, and it matches #403's repro exactly: a CREATE USER through /v1/ops/query with a user that lacks the grant. The integration test runs that statement against the test container, and the answer is 403 clickhouse.access_denied. A 5xx would tell clients and monitors that ClickHouse is down, and would invite retries of a request that can never pass. The operator still hears about it, because writeCHError logs every denial at WARN.

Denials that refuse every query rather than one statement are different: a wrong password, an unknown or expired user, DATABASE_ACCESS_DENIED, or a proxy's 401/403. Those are an operator fix the caller cannot act on, so they are 502 clickhouse.misconfigured. They are not retryable, because a retry a few seconds later changes nothing.

Deliberately left for later

• fix(api): timeouts and memory limits on uncapped query paths are retried #620 (filed): on pipes, on /v1/ops/query, and on /v1/query for a role with no cap, a TIMEOUT_EXCEEDED / MEMORY_LIMIT_EXCEEDED / query_timeout expiry is 503 and gets retried. The same codes also come from a busy server, so there I kept the classifier's verdict.
• Memory cap over-attribution: when the role sets max_memory_usage, a server-total MEMORY_LIMIT_EXCEEDED is also answered 400. This is documented in api.md.
• Singleflight: waiters that share one flight each class the shared error by their own role's caps. This comes from the existing role-insensitive flight key (bug(api): singleflight on /v1/query has no clean context-cancellation choice #120 territory), not from this delta.
• Out of scope: refactor(errors): typed error kinds for stable machine-readable codes #539's wider code catalogue (discovery / policy / ingest validation). This PR only adds clickhouse.*.

Tests

• Unit, internal/api/ch_errors_test.go (new): a table of real clickhouse-go errors, run through both /v1/query and pipes:

  • *clickhouse.Exception codes 47/53/60/62/158/159/202/241/497/516;
  • a real refused dial;
  • clickhouse.ErrAcquireConnTimeout;
  • pool-wait and backstop-cancel cases under a time cap;
  • an error with no verdict.

  It also pins that a time-capped query context carries no deadline, and covers the schema-refresh outage.

• Unit, query_test.go: the proxy table covers header codes and body-only codes, a proxy with no code, the query API maps ClickHouse caller-side errors (e.g. ACCESS_DENIED) to HTTP 502 instead of 4xx #403 repro, the credential cases, and a real refused connection.

• Integration, tests/integration/query_errors_test.go (new):

  • Against the shared live ClickHouse:
    • raw SQL SELEC 1 → 400 clickhouse.rejected, not retryable;
    • CREATE USER → 403 clickhouse.access_denied;
    • a structured query on a column dropped behind the registry → 400 (code 47).
  • Against its own container and app: stopping ClickHouse makes both /v1/ops/query and /v1/query answer 503 clickhouse.unavailable, retryable: true, Retry-After: 5.
  • The driver-contract test described under "Role time cap".

• Updated for the new contract:

  • query_limits_test.go: the role caps are 400 limit_exceeded, not 500;
  • app_test.go: a verified token is told apart by the ClickHouse error body, because a closed ClickHouse is now 503;
  • tenant_clickhouse_test.go;
  • e2e tests/e2e/sdk/query.test.ts (role caps → 400) and admin.test.ts (syntax error through wh.sql → 400, clickhouse.rejected, not retried).

• SDK: errors.test.ts and http.test.ts, including a 502 with retryable: false that is not retried.

• make ci: passed at f485505c: unit, integration and e2e, all coverage gates.

Review

The pre-push-reviewer and docs-reviewer (opus) ran in fresh context against the delta vs origin/feat/ch-error-classes.

• Round 1 (01ea6902):
  • Code: iterate, one MUST. A bare DeadlineExceeded under a role time cap was read as the cap, so a pool wait or dial timeout became a non-retryable 400. There was also a stale AGENTS.md line.
  • Docs: iterate, five SHOULDs: configuration.mdx limits, api.md cap wording, access-control naming the three caps, the SDK table missing clickhouse.unknown, and the maxRetries wording.
  • Fixed in 4ff30f31. The reviewer's suggested fix (slack on the deadline) would have been overridden by the driver, so the context now carries no deadline instead.
• Round 2 (4ff30f31): both ship_it.
• Rounds 3–5 (148a160b, 3a314afe, f485505c, test and CHANGELOG only, after make ci caught stale 503/500 expectations in app_test.go and query_limits_test.go):
  • Code: iterate once, because app_test.go:908 could no longer fail. Then ship_it at f485505c, after a full sweep of the test files.
  • Docs reviewer: not re-run for these commits; they touch no docs prose beyond the CHANGELOG file list.

Review markers (#454): in a wt worktree, the SubagentStop hook writes markers into the main checkout's tmp/, keyed to its HEAD. So no tmp/<reviewer>-passed-f485505c… exists for this branch. The verdicts above are the record. No marker was hand-written or skipped.

🤖 Generated with Claude Code

https://claude.ai/code/session_01EJr5tY4WQUy2sc4MbW67vL

[coderabbitai]

Important

Review skipped

Auto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Advanced

Run ID: 86ab3474-95bd-482a-aedc-39f955dd9b1a

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

• 🔍 Trigger review

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[github-actions]

📚 Docs preview is live → https://613f7337-wavehouse-docs.wave-rf.workers.dev

• Commit — ad52e08: Merge remote-tracking branch 'origin/main' into fix/query-error-classes
• Author — @EricAndrechek
• Committed — 2026-09-26 02:40 (UTC-04:00)
• Deployed — 2026-09-26 02:59 EDT

[github-code-quality]

Code Coverage Overview

Languages: Go

Go

The overall line coverage in commit ad52e08 in the fix/query-error-clas... branch remains at 94%, unchanged from commit a1774fb in the main branch.

Show a line coverage summary of the most impacted files.

File	main a1774fb	fix/query-error-clas... ad52e08	+/-
internal/api/query.go	90%	89%	-1%
internal/api/st...ctured_query.go	98%	98%	0%
internal/api/errors.go	100%	100%	0%
internal/api/schema.go	93%	96%	+3%
internal/api/pipes.go	91%	96%	+5%
internal/api/ch_errors.go	0%	100%	+100%

---

_{Updated September 26, 2026 06:59 UTC}