Skip to content

Fix false positive error detection when JSON data contains ClickHouse exception text#261

Open
jradtilbrook wants to merge 1 commit into
smi2:masterfrom
jradtilbrook:fix/false-positive-error-detection-in-json-data
Open

Fix false positive error detection when JSON data contains ClickHouse exception text#261
jradtilbrook wants to merge 1 commit into
smi2:masterfrom
jradtilbrook:fix/false-positive-error-detection-in-json-data

Conversation

@jradtilbrook
Copy link
Copy Markdown

@jradtilbrook jradtilbrook commented May 8, 2026

Summary

  • When querying tables that store ClickHouse exceptions as data (e.g. error tracking systems), hasErrorClickhouse() can falsely flag valid responses as errors if the exception text appears in the last 4096 bytes of the response body.
  • On PHP 8.3+, uses json_validate() as a confirmation step when the tail-regex matches: if the response is structurally valid JSON, the query succeeded and the match is just data.
  • Zero change in behaviour on PHP < 8.3. Zero overhead on the common path (regex doesn't match).

Problem

The > 4096 byte branch in hasErrorClickhouse() checks the last 4096 bytes of a JSON response against CLICKHOUSE_ERROR_REGEX. This exists to detect mid-stream errors (where ClickHouse appends error text to an already-streaming HTTP 200 response, breaking the JSON structure).

However, if the response data legitimately contains ClickHouse exception strings (e.g. an error tracking system storing exceptions in a ClickHouse table), and that text lands in the tail of the response, it triggers a false positive.

Solution

When the regex matches in the tail, call json_validate($body) (PHP 8.3+) before concluding there's an error:

  • Valid JSON → query succeeded, the match is just data → return false
  • Invalid JSON → the response structure is broken by a real mid-stream error → return true

json_validate() is O(1) memory (no decoded structure allocated) and only runs in the case where the regex already matched.

@jradtilbrook
Copy link
Copy Markdown
Author

Related to #234

@jradtilbrook jradtilbrook marked this pull request as ready for review May 8, 2026 07:31
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.

1 participant