diff --git a/posthog/errors.py b/posthog/errors.py index efa9747f42a6..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 @@ -1052,3 +1052,11 @@ class CHQueryErrorUnknownTable(ExposedCHQueryError): ClickHouseAtCapacity, ClickHouseClusterMemoryLimitExceeded, ) + +# 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/tasks/tasks.py b/posthog/tasks/tasks.py index 0ccdf4f5ca7e..78860b6a170b 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,14 @@ 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 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: 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..7c77ee4b71a5 100644 --- a/posthog/test/test_feature_flag_analytics.py +++ b/posthog/test/test_feature_flag_analytics.py @@ -17,6 +17,9 @@ from django.core.cache import cache +from clickhouse_driver.errors import NetworkError, SocketTimeoutError +from parameterized import parameterized + from posthog import redis from posthog.constants import FlagRequestType from posthog.errors import CHQueryErrorUnknownTable @@ -1091,9 +1094,18 @@ 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")), + ("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") - 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()