From b294ef8ded247f883303d167a4cb1792eb5e9758 Mon Sep 17 00:00:00 2001 From: "posthog[bot]" <206114724+posthog[bot]@users.noreply.github.com> Date: Wed, 26 Aug 2026 18:00:50 +0000 Subject: [PATCH 1/4] fix(max): keep the real cause when an async query fails A non-user-safe async query failure (a ClickHouse memory limit, an internal error, a worker crash) hid its message text and set no error code, so the AI query executor raised an opaque "Query failed". That plain exception was excluded from the AI report pipeline's retryable set, so the step dead-ended with no retry and no diagnosable cause. Carry the exception class as a machine-readable error_code on the failed status, even when the message text stays hidden. Classify the resulting exception as MaxToolRetryableError so transient ClickHouse failures retry and surface the cause instead of collapsing to "Query failed". Generated-By: PostHog Desktop Task-Id: 5b2bf3ce-ead7-4283-8180-6fdff34dc0f4 --- ee/hogai/context/insight/query_executor.py | 8 +++++- .../insight/test/test_query_executor.py | 26 +++++++++++++++++++ posthog/clickhouse/client/execute_async.py | 7 ++++- .../client/test/test_execute_async.py | 17 ++++++++++++ 4 files changed, 56 insertions(+), 2 deletions(-) diff --git a/ee/hogai/context/insight/query_executor.py b/ee/hogai/context/insight/query_executor.py index 29a553b3b7f1..16b422886236 100644 --- a/ee/hogai/context/insight/query_executor.py +++ b/ee/hogai/context/insight/query_executor.py @@ -442,7 +442,12 @@ def process_query_dict_with_tags() -> dict | BaseModel: if query_status.get("error"): if error_message := query_status.get("error_message"): raise APIException(error_message) - raise Exception("Query failed") + # The status hides the message text because it's not user-safe, but error_code + # carries the machine-readable cause (a ClickHouse memory limit, an internal + # error, a worker crash). These are transient, so retry instead of dead-ending + # on an opaque "Query failed" that the report pipeline never retries. + error_code = query_status.get("error_code") + raise MaxToolRetryableError(f"Query failed: {error_code}" if error_code else "Query failed") # Use the completed query results response_dict = query_status["results"] @@ -453,6 +458,7 @@ def process_query_dict_with_tags() -> dict | BaseModel: HogQLNotImplementedError, ExposedCHQueryError, UserAccessControlError, + MaxToolRetryableError, ) as err: elapsed = time.time() - start_time # Handle known query execution errors with user-friendly messages diff --git a/ee/hogai/context/insight/test/test_query_executor.py b/ee/hogai/context/insight/test/test_query_executor.py index abc866dbf664..3d897abf171d 100644 --- a/ee/hogai/context/insight/test/test_query_executor.py +++ b/ee/hogai/context/insight/test/test_query_executor.py @@ -396,6 +396,32 @@ async def test_async_query_polling_with_error(self, mock_get_query_status, mock_ self.assertIn("Query failed with error", str(context.exception)) + @patch("ee.hogai.context.insight.query_executor.process_query_dict") + @patch("ee.hogai.context.insight.query_executor.get_query_status") + async def test_async_query_polling_error_without_message_is_retryable( + self, mock_get_query_status, mock_process_query + ): + # A non-user-safe failure hides the message text but carries error_code. The step must retry + # (MaxToolRetryableError) and surface the cause, not dead-end on an opaque "Query failed". + mock_process_query.return_value = {"query_status": {"id": "test-query-id", "complete": False}} + mock_get_query_status.return_value = Mock( + model_dump=lambda mode: { + "id": "test-query-id", + "complete": True, + "error": True, + "error_message": None, + "error_code": "CHQueryErrorMemoryLimitExceeded", + } + ) + + query = AssistantTrendsQuery(series=[]) + + with patch("ee.hogai.context.insight.query_executor.asyncio.sleep"): + with self.assertRaises(MaxToolRetryableError) as context: + await self.query_runner.arun_and_format_query(query) + + self.assertIn("CHQueryErrorMemoryLimitExceeded", str(context.exception)) + @override_settings(TEST=False) @patch("ee.hogai.context.insight.query_executor.process_query_dict") async def test_execution_mode_in_production(self, mock_process_query): diff --git a/posthog/clickhouse/client/execute_async.py b/posthog/clickhouse/client/execute_async.py index 4d2881ec2411..65e1c70c8874 100644 --- a/posthog/clickhouse/client/execute_async.py +++ b/posthog/clickhouse/client/execute_async.py @@ -20,7 +20,7 @@ from posthog.clickhouse.query_tagging import get_query_tags, tag_queries from posthog.constants import AvailableFeature from posthog.direct_query_cancellation import build_direct_query_cancellation_token, request_direct_query_cancellation -from posthog.errors import ExposedCHQueryError +from posthog.errors import ExposedCHQueryError, clickhouse_error_type from posthog.exceptions import ClickHouseAtCapacity from posthog.exceptions_capture import capture_exception from posthog.renderers import SafeJSONRenderer @@ -311,6 +311,11 @@ def execute_process_query( codes = err.get_codes() if isinstance(codes, str): query_status.error_code = codes + if not query_status.error_code: + # The message text stays hidden when it's not user-safe, but the exception class is a + # stable, machine-readable cause (e.g. "CHQueryErrorMemoryLimitExceeded"). Carry it so + # callers can classify the failure instead of dead-ending on an opaque "Query failed". + query_status.error_code = clickhouse_error_type(err) logger.exception("Error processing query async", team_id=team_id, query_id=query_id, exc_info=True) if not is_user_safe_error: # User-safe errors (e.g. a malformed HogQL query) are already returned to the user as a 400, diff --git a/posthog/clickhouse/client/test/test_execute_async.py b/posthog/clickhouse/client/test/test_execute_async.py index 7fc596e75918..efac6d536953 100644 --- a/posthog/clickhouse/client/test/test_execute_async.py +++ b/posthog/clickhouse/client/test/test_execute_async.py @@ -363,6 +363,23 @@ def test_async_query_user_safe_error_carries_error_code(self): assert result.error_message self.assertEqual(result.error_code, ClickHouseQueryMemoryLimitExceeded.default_code) + def test_async_query_non_user_safe_error_carries_error_code_without_message(self): + query = build_query("SELECT * FROM events") + query_id = uuid.uuid4().hex + + with patch("posthog.api.services.query.process_query_dict", side_effect=Exception("sensitive detail")): + client.enqueue_process_query_task( + self.team, self.user.id, query, query_id=query_id, _test_only_bypass_celery=True + ) + + result = client.get_query_status(self.team.id, query_id) + self.assertTrue(result.error) + self.assertTrue(result.complete) + # The message stays hidden for a non-user-safe error, but the class name is carried as a + # machine-readable cause so callers can classify the failure. + self.assertIsNone(result.error_message) + self.assertEqual(result.error_code, "Exception") + def test_async_query_server_errors(self): query = build_query("SELECT * FROM events") From 79b29023b55e33b5611e6d588817f913d1d7f672 Mon Sep 17 00:00:00 2001 From: "posthog[bot]" <206114724+posthog[bot]@users.noreply.github.com> Date: Wed, 26 Aug 2026 18:33:49 +0000 Subject: [PATCH 2/4] docs(query): describe error_code's broadened contract The async query status now carries the exception or ClickHouse error type in error_code for non-user-safe failures, not only the DRF exception code. Update the QueryStatus.error_code schema description so the documented public contract matches what the field can hold, including when the message text is hidden. Regenerated schema.json and posthog/schema.py from the TypeScript source. Generated-By: PostHog Desktop Task-Id: de4edea4-3d44-49da-93d3-0a39abdc791b --- frontend/src/queries/schema.json | 2 +- frontend/src/queries/schema/schema-general.ts | 2 +- posthog/schema.py | 6 +++++- 3 files changed, 7 insertions(+), 3 deletions(-) diff --git a/frontend/src/queries/schema.json b/frontend/src/queries/schema.json index 51c33cb61e94..7e3e61cb8418 100644 --- a/frontend/src/queries/schema.json +++ b/frontend/src/queries/schema.json @@ -47480,7 +47480,7 @@ }, "error_code": { "default": null, - "description": "Stable machine-readable code for the error (the DRF exception code), when known.", + "description": "Stable machine-readable code for the error (the DRF exception code when user-safe, otherwise the exception or ClickHouse error type), set even when the message text is hidden.", "type": ["string", "null"] }, "error_message": { diff --git a/frontend/src/queries/schema/schema-general.ts b/frontend/src/queries/schema/schema-general.ts index 515583368fae..a9dfea749443 100644 --- a/frontend/src/queries/schema/schema-general.ts +++ b/frontend/src/queries/schema/schema-general.ts @@ -2669,7 +2669,7 @@ export type QueryStatus = { /** @default null */ error_message: string | null /** - * Stable machine-readable code for the error (the DRF exception code), when known. + * Stable machine-readable code for the error (the DRF exception code when user-safe, otherwise the exception or ClickHouse error type), set even when the message text is hidden. * @default null */ error_code: string | null diff --git a/posthog/schema.py b/posthog/schema.py index abbad6a784c7..350807116be3 100644 --- a/posthog/schema.py +++ b/posthog/schema.py @@ -6446,7 +6446,11 @@ class QueryStatus(BaseModel): ) error_code: str | None = Field( default=None, - description=("Stable machine-readable code for the error (the DRF exception code), when known."), + description=( + "Stable machine-readable code for the error (the DRF exception code when" + " user-safe, otherwise the exception or ClickHouse error type), set even" + " when the message text is hidden." + ), ) error_message: str | None = None expiration_time: AwareDatetime | None = None From 550d64494ec4c1ebe04769542a4b9c7e094783a5 Mon Sep 17 00:00:00 2001 From: "tests-posthog[bot]" <250237707+tests-posthog[bot]@users.noreply.github.com> Date: Wed, 26 Aug 2026 18:39:32 +0000 Subject: [PATCH 3/4] chore: update OpenAPI generated types --- products/dashboards/frontend/generated/api.schemas.ts | 2 +- products/endpoints/frontend/generated/api.schemas.ts | 2 +- products/product_analytics/frontend/generated/api.schemas.ts | 2 +- services/mcp/src/api/generated.ts | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/products/dashboards/frontend/generated/api.schemas.ts b/products/dashboards/frontend/generated/api.schemas.ts index f69379c62e56..685fd8af6b02 100644 --- a/products/dashboards/frontend/generated/api.schemas.ts +++ b/products/dashboards/frontend/generated/api.schemas.ts @@ -1753,7 +1753,7 @@ export interface QueryStatusApi { end_time?: string | null /** If the query failed, this will be set to true. More information can be found in the error_message field. */ error?: boolean | null - /** Stable machine-readable code for the error (the DRF exception code), when known. */ + /** Stable machine-readable code for the error (the DRF exception code when user-safe, otherwise the exception or ClickHouse error type), set even when the message text is hidden. */ error_code?: string | null error_message?: string | null expiration_time?: string | null diff --git a/products/endpoints/frontend/generated/api.schemas.ts b/products/endpoints/frontend/generated/api.schemas.ts index 395cc1f99c3e..fcd59686e77a 100644 --- a/products/endpoints/frontend/generated/api.schemas.ts +++ b/products/endpoints/frontend/generated/api.schemas.ts @@ -1016,7 +1016,7 @@ export interface QueryStatusApi { end_time?: string | null /** If the query failed, this will be set to true. More information can be found in the error_message field. */ error?: boolean | null - /** Stable machine-readable code for the error (the DRF exception code), when known. */ + /** Stable machine-readable code for the error (the DRF exception code when user-safe, otherwise the exception or ClickHouse error type), set even when the message text is hidden. */ error_code?: string | null error_message?: string | null expiration_time?: string | null diff --git a/products/product_analytics/frontend/generated/api.schemas.ts b/products/product_analytics/frontend/generated/api.schemas.ts index 64a689ea4c0c..53642af6f046 100644 --- a/products/product_analytics/frontend/generated/api.schemas.ts +++ b/products/product_analytics/frontend/generated/api.schemas.ts @@ -915,7 +915,7 @@ export interface QueryStatusApi { end_time?: string | null /** If the query failed, this will be set to true. More information can be found in the error_message field. */ error?: boolean | null - /** Stable machine-readable code for the error (the DRF exception code), when known. */ + /** Stable machine-readable code for the error (the DRF exception code when user-safe, otherwise the exception or ClickHouse error type), set even when the message text is hidden. */ error_code?: string | null error_message?: string | null expiration_time?: string | null diff --git a/services/mcp/src/api/generated.ts b/services/mcp/src/api/generated.ts index f6f1e7ea8a95..582d2ab60364 100644 --- a/services/mcp/src/api/generated.ts +++ b/services/mcp/src/api/generated.ts @@ -1001,7 +1001,7 @@ export namespace Schemas { end_time?: string | null; /** If the query failed, this will be set to true. More information can be found in the error_message field. */ error?: boolean | null; - /** Stable machine-readable code for the error (the DRF exception code), when known. */ + /** Stable machine-readable code for the error (the DRF exception code when user-safe, otherwise the exception or ClickHouse error type), set even when the message text is hidden. */ error_code?: string | null; error_message?: string | null; expiration_time?: string | null; From 8941853224bc817021c212619d370bf315c22186 Mon Sep 17 00:00:00 2001 From: "posthog[bot]" <206114724+posthog[bot]@users.noreply.github.com> Date: Wed, 26 Aug 2026 18:41:04 +0000 Subject: [PATCH 4/4] fix(max): carry cause code on outer task failures too MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A non-user-safe async query failure that happens outside execute_process_query — concurrency-limit exhaustion after retries, or worker loss — reaches the Celery on_failure callback instead of the handler the PR already fixed. That callback set no error_code, so the AI executor still saw an opaque "Query failed" for this path. Record the exception class as a machine-readable error_code in _process_query_task_failure for non-user-safe failures, mirroring execute_process_query, so the AI executor and polling clients get the cause. The user-safe APIException branch is unchanged (it already surfaces a message). Parameterize the existing callback test to cover both the APIException path and a non-API failure that must now carry the class name as its code. Generated-By: PostHog Desktop Task-Id: de4edea4-3d44-49da-93d3-0a39abdc791b --- .../client/test/test_execute_async.py | 19 ++++++++++++++++--- posthog/tasks/tasks.py | 8 +++++++- 2 files changed, 23 insertions(+), 4 deletions(-) diff --git a/posthog/clickhouse/client/test/test_execute_async.py b/posthog/clickhouse/client/test/test_execute_async.py index efac6d536953..c56cb5bc913a 100644 --- a/posthog/clickhouse/client/test/test_execute_async.py +++ b/posthog/clickhouse/client/test/test_execute_async.py @@ -22,6 +22,7 @@ ) from posthog.clickhouse.client.async_task_chain import execute_task_chain, task_chain_context from posthog.clickhouse.client.execute_async import QueryNotFoundError, QueryStatusManager, execute_process_query +from posthog.clickhouse.client.limit import ConcurrencyLimitExceeded from posthog.clickhouse.query_tagging import get_query_tags, tag_queries from posthog.constants import AvailableFeature from posthog.direct_query_cancellation import ( @@ -71,13 +72,24 @@ def test_no_status(self): self.query_status.expiration_time = None # We don't care about expiration time in this test self.assertEqual(self.manager.get_query_status(True), self.query_status) - def test_process_query_task_on_failure_marks_status_errored(self): + @parameterized.expand( + [ + # User-safe APIException: message is surfaced, no machine-readable code needed. + ("api_exception", ClickHouseAtCapacity(), ClickHouseAtCapacity.default_detail, None), + # Non-user-safe outer-task failure (retries exhausted / worker loss) reaches the + # callback with its message hidden, so it must still record the class as the cause. + ("non_api_exception", ConcurrencyLimitExceeded("over limit"), None, "ConcurrencyLimitExceeded"), + ] + ) + def test_process_query_task_on_failure_marks_status_errored( + self, _name, exc, expected_message, expected_error_code + ): from posthog.tasks.tasks import process_query_task self.manager.store_query_status(self.query_status) process_query_task.on_failure( - exc=ClickHouseAtCapacity(), + exc=exc, task_id="celery-task-id", args=(self.team_id, None, self.query_id), kwargs={}, @@ -87,7 +99,8 @@ def test_process_query_task_on_failure_marks_status_errored(self): result = self.manager.get_query_status() self.assertTrue(result.complete) self.assertTrue(result.error) - self.assertEqual(result.error_message, ClickHouseAtCapacity.default_detail) + self.assertEqual(result.error_message, expected_message) + self.assertEqual(result.error_code, expected_error_code) self.assertIsNotNone(result.end_time) def test_store_clickhouse_query_progress(self): diff --git a/posthog/tasks/tasks.py b/posthog/tasks/tasks.py index eaabe56033af..9280ee23704d 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_ERRORS, CHQueryErrorUnknownTable, clickhouse_error_type from posthog.exceptions import ClickHouseAtCapacity from posthog.exceptions_capture import capture_exception from posthog.metrics import pushed_metrics_registry @@ -419,6 +419,12 @@ def _process_query_task_failure( if isinstance(exc, APIException): # User-safe message (e.g. ClickHouseAtCapacity's "try again later" copy) query_status.error_message = str(exc.detail) + elif not query_status.error_code: + # A non-user-safe task failure (concurrency-limit exhaustion, worker loss) reaches this + # callback, not execute_process_query's handler, so it hides its message and would set no + # cause. Record the exception class as a machine-readable code so the AI executor and + # polling clients get the cause instead of an opaque "Query failed". + query_status.error_code = clickhouse_error_type(exc) query_status.end_time = datetime.datetime.now(datetime.UTC) manager.store_query_status(query_status)