Detect streamed ClickHouse exceptions#132
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9700fca6a2
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| @@ -517,6 +585,11 @@ export function createExecutorWithClient( | |||
| ...(settings ? { clickhouse_settings: settings } : {}), | |||
| }) | |||
| const rows = await result.json<T>() | |||
There was a problem hiding this comment.
In query() (and the same pattern in queryJson()), the code calls result.json() before assertStreamedQuerySucceeded(). With send_progress_in_http_headers=1, ClickHouse can return HTTP 200, set X-ClickHouse-Exception-Code, and then append a plain-text exception block to the body; in that case JSON decoding may throw first and header-based detection is never reached, so the new streamed-exception handling is bypassed. Move the header assertion ahead of body parsing so these failures are consistently surfaced as ClickHouseStreamedException.
Useful? React with 👍 / 👎.
ClickHouse can return HTTP 200, set x-clickhouse-exception-code, then append a plain-text exception block after partial JSON rows when send_progress_in_http_headers=1. Decoding the body first would throw on the trailing non-JSON content before the header check ran, bypassing ClickHouseStreamedException. Assert headers first in query() and queryJson(), with a regression test that fails .json() to prove the header check runs first. Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
1 participant
Summary
Verification