From c46d706ed8477b4c4d165ad59c824fffd7d6c6aa Mon Sep 17 00:00:00 2001 From: "posthog[bot]" <206114724+posthog[bot]@users.noreply.github.com> Date: Sun, 30 Aug 2026 00:44:03 +0000 Subject: [PATCH 1/3] fix(feature-flags): tolerate transient clickhouse connection errors in enrichment task The find_flags_with_enriched_analytics task made unretried ClickHouse round-trips. A dropped socket at connect time or mid-read raised ConnectionResetError or EOFError, which the task reported through capture_exception, minting a new error-tracking issue on each blip while the run silently skipped. Add CH_TRANSIENT_CONNECTION_ERRORS (ConnectionResetError, EOFError) and handle it in the task wrapper the same way CHQueryErrorUnknownTable is handled: log a warning and skip the run. The wrapper covers both round-trips - the materialized-column registry lookup and the main analytics query - so the next 12-hourly run recovers. Generated-By: PostHog Desktop Task-Id: eb9b671d-d850-47ce-b10b-9a58cda0fa0d --- posthog/errors.py | 6 ++++++ posthog/tasks/tasks.py | 7 ++++++- posthog/test/test_feature_flag_analytics.py | 13 +++++++++++-- 3 files changed, 23 insertions(+), 3 deletions(-) diff --git a/posthog/errors.py b/posthog/errors.py index efa9747f42a6..2d44f126bd71 100644 --- a/posthog/errors.py +++ b/posthog/errors.py @@ -1052,3 +1052,9 @@ class CHQueryErrorUnknownTable(ExposedCHQueryError): ClickHouseAtCapacity, ClickHouseClusterMemoryLimitExceeded, ) + +# Transient ClickHouse connection failures: the socket resets, or the server drops the connection +# during the handshake or while a result streams back. These are raised below the ServerException +# layer, so wrap_clickhouse_query_error passes them through unchanged and CH_TRANSIENT_ERRORS does +# not cover them. The connection self-heals, so callers that retry or skip on these lose nothing. +CH_TRANSIENT_CONNECTION_ERRORS = (ConnectionResetError, EOFError) diff --git a/posthog/tasks/tasks.py b/posthog/tasks/tasks.py index 0ccdf4f5ca7e..7c7bba8aab17 100644 --- a/posthog/tasks/tasks.py +++ b/posthog/tasks/tasks.py @@ -23,7 +23,7 @@ from posthog.clickhouse.client.limit import ConcurrencyLimitExceeded, limit_concurrency from posthog.clickhouse.query_tagging import Feature, Product, get_query_tags, tag_queries from posthog.cloud_utils import is_cloud -from posthog.errors import CH_TRANSIENT_ERRORS, CHQueryErrorUnknownTable +from posthog.errors import CH_TRANSIENT_CONNECTION_ERRORS, CH_TRANSIENT_ERRORS, CHQueryErrorUnknownTable from posthog.exceptions import ClickHouseAtCapacity from posthog.exceptions_capture import capture_exception from posthog.metrics import pushed_metrics_registry @@ -1038,6 +1038,11 @@ def find_flags_with_enriched_analytics() -> None: # Expected on self-hosted instances with an incomplete ClickHouse schema (e.g. missing # migrations) - not worth capturing as an exception, just skip this run. logger.warning("Find flags with enriched analytics skipped, table missing", error=e) + except CH_TRANSIENT_CONNECTION_ERRORS as e: + # A ClickHouse connection dropped at connect time or mid-read. This covers both round-trips + # in the task: the materialized-column registry lookup and the main analytics query. The + # next 12-hourly run recovers, so skip this one instead of minting an error-tracking issue. + logger.warning("Find flags with enriched analytics skipped, transient connection error", error=e) except Exception as e: logger.exception("Find flags with enriched analytics failed", error=e) capture_exception( diff --git a/posthog/test/test_feature_flag_analytics.py b/posthog/test/test_feature_flag_analytics.py index 0a93462b94d4..4e1594c6205c 100644 --- a/posthog/test/test_feature_flag_analytics.py +++ b/posthog/test/test_feature_flag_analytics.py @@ -17,6 +17,8 @@ from django.core.cache import cache +from parameterized import parameterized + from posthog import redis from posthog.constants import FlagRequestType from posthog.errors import CHQueryErrorUnknownTable @@ -1091,9 +1093,16 @@ def test_logs_and_captures_on_failure_without_reraising(self, mock_find_flags: M mock_capture.assert_called_once() + @parameterized.expand( + [ + ("unknown_table", CHQueryErrorUnknownTable("Table default.events doesn't exist", code=60)), + ("connection_reset", ConnectionResetError(104, "Connection reset by peer")), + ("eof", EOFError("Unexpected EOF while reading bytes")), + ] + ) @patch("products.feature_flags.backend.flag_analytics.find_flags_with_enriched_analytics") - def test_unknown_table_error_is_not_captured(self, mock_find_flags: MagicMock) -> None: - mock_find_flags.side_effect = CHQueryErrorUnknownTable("Table default.events doesn't exist", code=60) + def test_benign_error_is_not_captured(self, _name: str, error: Exception, mock_find_flags: MagicMock) -> None: + mock_find_flags.side_effect = error with patch("posthog.tasks.tasks.capture_exception") as mock_capture: find_flags_with_enriched_analytics_task() From ed004b6c483e609e0e6d319b7e80cab573c09eb8 Mon Sep 17 00:00:00 2001 From: "posthog[bot]" <206114724+posthog[bot]@users.noreply.github.com> Date: Sun, 30 Aug 2026 01:49:31 +0000 Subject: [PATCH 2/3] fix(feature-flags): cover connect-time clickhouse network errors as transient The clickhouse_driver converts a connect-time socket error or timeout into NetworkError or SocketTimeoutError, not into the raw ConnectionResetError. The transient tuple only listed ConnectionResetError and EOFError, so a connect-time reset or a handshake drop still reached capture_exception and minted an error-tracking issue. Add NetworkError and SocketTimeoutError to CH_TRANSIENT_CONNECTION_ERRORS, next to the raw read-path types, matching the existing RETRIABLE_CH_ERRORS set in report_agent. Extend the parameterized task test with cases for both. Generated-By: PostHog Desktop Task-Id: e271d2b3-7987-43dc-828c-9ee478e31e90 --- posthog/errors.py | 14 ++++++++------ posthog/test/test_feature_flag_analytics.py | 3 +++ 2 files changed, 11 insertions(+), 6 deletions(-) diff --git a/posthog/errors.py b/posthog/errors.py index 2d44f126bd71..e9e89cbb2bdc 100644 --- a/posthog/errors.py +++ b/posthog/errors.py @@ -3,7 +3,7 @@ from enum import StrEnum from typing import Optional -from clickhouse_driver.errors import ServerException +from clickhouse_driver.errors import NetworkError, ServerException, SocketTimeoutError from posthog.hogql.errors import ExposedHogQLError @@ -1053,8 +1053,10 @@ class CHQueryErrorUnknownTable(ExposedCHQueryError): ClickHouseClusterMemoryLimitExceeded, ) -# Transient ClickHouse connection failures: the socket resets, or the server drops the connection -# during the handshake or while a result streams back. These are raised below the ServerException -# layer, so wrap_clickhouse_query_error passes them through unchanged and CH_TRANSIENT_ERRORS does -# not cover them. The connection self-heals, so callers that retry or skip on these lose nothing. -CH_TRANSIENT_CONNECTION_ERRORS = (ConnectionResetError, EOFError) +# Transient ClickHouse connection failures. The driver raises these below the ServerException layer, +# so wrap_clickhouse_query_error passes them through unchanged and CH_TRANSIENT_ERRORS does not cover +# them. The driver converts a connect-time socket error or timeout - including a reset during the +# handshake - into NetworkError or SocketTimeoutError. A reset or EOF while a result streams back +# arrives as the raw ConnectionResetError or EOFError. The connection self-heals, so callers that +# retry or skip on these lose nothing. +CH_TRANSIENT_CONNECTION_ERRORS = (ConnectionResetError, EOFError, NetworkError, SocketTimeoutError) diff --git a/posthog/test/test_feature_flag_analytics.py b/posthog/test/test_feature_flag_analytics.py index 4e1594c6205c..7c77ee4b71a5 100644 --- a/posthog/test/test_feature_flag_analytics.py +++ b/posthog/test/test_feature_flag_analytics.py @@ -17,6 +17,7 @@ from django.core.cache import cache +from clickhouse_driver.errors import NetworkError, SocketTimeoutError from parameterized import parameterized from posthog import redis @@ -1098,6 +1099,8 @@ def test_logs_and_captures_on_failure_without_reraising(self, mock_find_flags: M ("unknown_table", CHQueryErrorUnknownTable("Table default.events doesn't exist", code=60)), ("connection_reset", ConnectionResetError(104, "Connection reset by peer")), ("eof", EOFError("Unexpected EOF while reading bytes")), + ("network_error", NetworkError("Connection refused (localhost:9000)")), + ("socket_timeout", SocketTimeoutError("Socket timeout while connecting (localhost:9000)")), ] ) @patch("products.feature_flags.backend.flag_analytics.find_flags_with_enriched_analytics") From 7e8a6746347c363a44950b93dfc1010bc1444478 Mon Sep 17 00:00:00 2001 From: "posthog[bot]" <206114724+posthog[bot]@users.noreply.github.com> Date: Sun, 30 Aug 2026 01:51:45 +0000 Subject: [PATCH 3/3] docs(feature-flags): correct transient-error coverage comment on flags task The handler comment claimed it covers both round-trips, including the materialized-column registry lookup. The registry lookup goes through get_enabled_materialized_columns, which is cached with background_refresh=True. A warm-but-stale entry refreshes on a background thread, so a transient error there runs outside this handler and the SDK thread hook reports it separately. The task still returns the stale value. Rewrite the comment to describe what the handler covers inline and to note the background-refresh path it cannot reach. No behavior change. Generated-By: PostHog Desktop Task-Id: e271d2b3-7987-43dc-828c-9ee478e31e90 --- posthog/tasks/tasks.py | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/posthog/tasks/tasks.py b/posthog/tasks/tasks.py index 7c7bba8aab17..78860b6a170b 100644 --- a/posthog/tasks/tasks.py +++ b/posthog/tasks/tasks.py @@ -1039,8 +1039,11 @@ def find_flags_with_enriched_analytics() -> None: # migrations) - not worth capturing as an exception, just skip this run. logger.warning("Find flags with enriched analytics skipped, table missing", error=e) except CH_TRANSIENT_CONNECTION_ERRORS as e: - # A ClickHouse connection dropped at connect time or mid-read. This covers both round-trips - # in the task: the materialized-column registry lookup and the main analytics query. The + # A ClickHouse connection dropped at connect time or mid-read. This handler covers the main + # analytics query and a cold-cache materialized-column registry lookup, which both run inline. + # A warm-but-stale registry entry refreshes on a background thread (cache_for with + # background_refresh), so a transient error there stays outside this handler; the task keeps + # running on the stale value, and the SDK thread hook reports that failure separately. The # next 12-hourly run recovers, so skip this one instead of minting an error-tracking issue. logger.warning("Find flags with enriched analytics skipped, transient connection error", error=e) except Exception as e: