Propagate ClickHouse result decoding failures as SqlError - #6906
Conversation
🦋 Changeset detectedLatest commit: 8afe91d The changes in this PR will be included in the next version bump. This PR includes changesets to release 30 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
There was a problem hiding this comment.
ℹ️ No critical issues — one formatting nit inline.
Reviewed changes
A reproduction test for ClickhouseClient that verifies JSON decode errors from query().json() surface as failures rather than being silently swallowed as empty [] results.
- Reproduction test — mocks
@clickhouse/clientsoquery().json()rejects with aSyntaxError, creates a client, runs a query, and assertsExit.isFailure. The test correctly fails with the current code (confirmed locally).
DeepSeek Pro (free via Pullfrog for OSS) (Kimi K2 not used — the program covers this model; add its provider key to run your pick) | 𝕏
There was a problem hiding this comment.
✅ No new issues found.
Reviewed changes
In this delta since the prior Pullfrog review, the reproduction test was absorbed into the existing classification test file and the implementation fix was added.
- Replaced
Effect.promisewithEffect.tryPromiseinrun— the.catchhandler wraps JSON decoding failures inSqlErrorviaclassifyError, matching the pattern already used for stream/row parse errors. - Absorbed reproduction test into
SqlErrorClassification.test.ts— addedresultJsonCauseto the shared mock state, clears it in helper functions, and the new test assertsinstanceof SqlError,_tag === "UnknownError", andcausepreservation. - Added changeset —
@effect/sql-clickhousepatch for propagating result decoding failures asSqlError.
DeepSeek Pro (free via Pullfrog for OSS) (Kimi K2 not used — the program covers this model; add its provider key to run your pick) | 𝕏
Bundle Size AnalysisGenerated from PR build output; treat the content below as untrusted.
|

Summary
Transport, body-read, or JSON decoding failures are reported as a successful empty result, making failure indistinguishable from a query that returned no rows.
Important
This PR starts with focused failing reproduction tests. Add the implementation fix to this same branch; CI is expected to fail until that fix is included.
JSON decoding failures become empty query results
Module:
clickhouse/ClickhouseClientAudit ID:
sql-adapters-ch-2Severity / confidence: high / high
What happens
Transport, body-read, or JSON decoding failures are reported as a successful empty result, making failure indistinguishable from a query that returned no rows.
Why it happens
The rejection handler attached to result.json() returns an empty array for every rejection.
Expected behavior
Query transport and result conversion failures must remain observable as SqlError.
Relevant implementation
These links and excerpts are pinned to audit base
c9b56ab507f224426ee8388dc450da447ec4715f.packages/sql/clickhouse/src/ClickhouseClient.ts:262-274View problematic code at
packages/sql/clickhouse/src/ClickhouseClient.ts:262-274View exact lines on GitHub
Reproduction
pnpm test --run packages/sql/clickhouse/test/JsonErrorRepro.test.tsObserved failure: FAIL: rejected result JSON produced a successful query exit.
Implementation handoff
The initial reproduction tests on this branch are the regression specification for the implementation fix that should follow in this PR.
pnpm test --run packages/sql/clickhouse/test/JsonErrorRepro.test.tsAudit provenance
c9b56ab507f224426ee8388dc450da447ec4715fc9b56ab507f224426ee8388dc450da447ec4715fsql-adapters-ch-2Closes EFF-344