Skip to content

fix: detect exceptions written as a JSON row - #493

Open
cristianchiriac wants to merge 1 commit into
ClickHouse:mainfrom
cristianchiriac:fix-json-row-exception
Open

cristianchiriac wants to merge 1 commit into
ClickHouse:mainfrom
cristianchiriac:fix-json-row-exception

Conversation

@cristianchiriac

Copy link
Copy Markdown

Summary

Fixes #492.

On servers that don't send X-ClickHouse-Exception-Tag (before 25.11, e.g. the 25.8 LTS line), an error raised after streaming has started is written into the output format. For JSON formats like JSONEachRow the last row is {"exception": "Code: 395. DB::Exception: ... (version 25.8.33.6 (official build))"}. extract_exception() only looked for the raw-text form, which ends with )) + newline, so this row ends up in the caller's data and fetch_bytes(...) finishes without an error.

extract_exception() now also checks chunks ending with )"} + newline: extract_exception_json() takes the last line, requires it to be {"exception": <JSON string>}, decodes the string with serde_json (so escaped quotes in the message come out right), and returns Error::BadResponse when it looks like a ClickHouse exception (Code:, DB::, Exception:), the same check as the raw-text path. Other rows are left alone.

Changelog text: Fixed detection of mid-stream exceptions written as a JSON row ({"exception": ...}) by servers without exception tagging.

Checklist

  • Unit and integration tests covering the common scenarios were added: it_extracts_exception_json_row covers the row from the issue, a message with escaped quotes, and a data row with an exception key that isn't a ClickHouse exception. It fails without the change.
  • A human-readable description of the changes was provided so that we can include it in CHANGELOG later

cargo test --lib: 112 passed. cargo fmt --check is clean; cargo clippy --lib --tests only shows the two existing redundant reference in format! warnings in other files.

Servers before 25.11 (no X-ClickHouse-Exception-Tag) write an error that
happens mid-stream into the output format. For JSON formats such as
JSONEachRow that is a final {"exception": "Code: ..."} row, which ends
with )"} and a newline rather than )) and a newline, so the response was
returned as data without an error.

Fixes ClickHouse#492
@CLAassistant

Copy link
Copy Markdown

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

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.

fetch_bytes("JSONEachRow"): in-band {"exception": ...} row from pre-25.11 servers is returned as data, no error

2 participants