fix(debug): View errors in /debug for trend queries (#21977)

* View errors in /debug for trend queries * Fixed ts error * Fixed tests * Update UI snapshots for `chromium` (1) * Update UI snapshots for `chromium` (2) * Update UI snapshots for `chromium` (1) * Update UI snapshots for `chromium` (2) --------- Co-authored-by: github-actions <41898282+github-actions[bot]@users.noreply.github.com>
PostHog · Apr 30, 2024 · 6876dd8 · 6876dd8
1 parent 36de77b
commit 6876dd8
Show file tree

Hide file tree

Showing 9 changed files with 174 additions and 14 deletions.
diff --git a/frontend/src/queries/schema.json b/frontend/src/queries/schema.json
@@ -2336,6 +2336,10 @@
         "FunnelsQueryResponse": {
             "additionalProperties": false,
             "properties": {
+                "error": {
+                    "description": "Query error. Returned only if 'explain' or `modifiers.debug` is true. Throws an error otherwise.",
+                    "type": "string"
+                },
                 "hogql": {
                     "type": "string"
                 },
@@ -2720,7 +2724,7 @@
                     "type": "array"
                 },
                 "error": {
-                    "description": "Query error. Returned only if 'explain' is true. Throws an error otherwise.",
+                    "description": "Query error. Returned only if 'explain' or `modifiers.debug` is true. Throws an error otherwise.",
                     "type": "string"
                 },
                 "explain": {
@@ -3275,6 +3279,10 @@
         "LifecycleQueryResponse": {
             "additionalProperties": false,
             "properties": {
+                "error": {
+                    "description": "Query error. Returned only if 'explain' or `modifiers.debug` is true. Throws an error otherwise.",
+                    "type": "string"
+                },
                 "hogql": {
                     "type": "string"
                 },
@@ -3574,6 +3582,10 @@
         "PathsQueryResponse": {
             "additionalProperties": false,
             "properties": {
+                "error": {
+                    "description": "Query error. Returned only if 'explain' or `modifiers.debug` is true. Throws an error otherwise.",
+                    "type": "string"
+                },
                 "hogql": {
                     "type": "string"
                 },
@@ -3808,6 +3820,10 @@
         "QueryResponse": {
             "additionalProperties": false,
             "properties": {
+                "error": {
+                    "description": "Query error. Returned only if 'explain' or `modifiers.debug` is true. Throws an error otherwise.",
+                    "type": "string"
+                },
                 "hogql": {
                     "type": "string"
                 },
@@ -4119,7 +4135,7 @@
                             "type": "array"
                         },
                         "error": {
-                            "description": "Query error. Returned only if 'explain' is true. Throws an error otherwise.",
+                            "description": "Query error. Returned only if 'explain' or `modifiers.debug` is true. Throws an error otherwise.",
                             "type": "string"
                         },
                         "explain": {
@@ -4238,6 +4254,10 @@
                 {
                     "additionalProperties": false,
                     "properties": {
+                        "error": {
+                            "description": "Query error. Returned only if 'explain' or `modifiers.debug` is true. Throws an error otherwise.",
+                            "type": "string"
+                        },
                         "hogql": {
                             "type": "string"
                         },
@@ -4279,6 +4299,10 @@
                             "items": {},
                             "type": "array"
                         },
+                        "error": {
+                            "description": "Query error. Returned only if 'explain' or `modifiers.debug` is true. Throws an error otherwise.",
+                            "type": "string"
+                        },
                         "hasMore": {
                             "type": "boolean"
                         },
@@ -4331,6 +4355,10 @@
                             "items": {},
                             "type": "array"
                         },
+                        "error": {
+                            "description": "Query error. Returned only if 'explain' or `modifiers.debug` is true. Throws an error otherwise.",
+                            "type": "string"
+                        },
                         "hogql": {
                             "type": "string"
                         },
@@ -4370,6 +4398,10 @@
                 {
                     "additionalProperties": false,
                     "properties": {
+                        "error": {
+                            "description": "Query error. Returned only if 'explain' or `modifiers.debug` is true. Throws an error otherwise.",
+                            "type": "string"
+                        },
                         "hogql": {
                             "type": "string"
                         },
@@ -4404,6 +4436,10 @@
                 {
                     "additionalProperties": false,
                     "properties": {
+                        "error": {
+                            "description": "Query error. Returned only if 'explain' or `modifiers.debug` is true. Throws an error otherwise.",
+                            "type": "string"
+                        },
                         "hogql": {
                             "type": "string"
                         },
@@ -4438,6 +4474,10 @@
                 {
                     "additionalProperties": false,
                     "properties": {
+                        "error": {
+                            "description": "Query error. Returned only if 'explain' or `modifiers.debug` is true. Throws an error otherwise.",
+                            "type": "string"
+                        },
                         "hogql": {
                             "type": "string"
                         },
@@ -4472,6 +4512,10 @@
                 {
                     "additionalProperties": false,
                     "properties": {
+                        "error": {
+                            "description": "Query error. Returned only if 'explain' or `modifiers.debug` is true. Throws an error otherwise.",
+                            "type": "string"
+                        },
                         "hogql": {
                             "type": "string"
                         },
@@ -4871,6 +4915,10 @@
         "RetentionQueryResponse": {
             "additionalProperties": false,
             "properties": {
+                "error": {
+                    "description": "Query error. Returned only if 'explain' or `modifiers.debug` is true. Throws an error otherwise.",
+                    "type": "string"
+                },
                 "hogql": {
                     "type": "string"
                 },
@@ -5268,6 +5316,10 @@
         "StickinessQueryResponse": {
             "additionalProperties": false,
             "properties": {
+                "error": {
+                    "description": "Query error. Returned only if 'explain' or `modifiers.debug` is true. Throws an error otherwise.",
+                    "type": "string"
+                },
                 "hogql": {
                     "type": "string"
                 },
@@ -5605,6 +5657,10 @@
         "TrendsQueryResponse": {
             "additionalProperties": false,
             "properties": {
+                "error": {
+                    "description": "Query error. Returned only if 'explain' or `modifiers.debug` is true. Throws an error otherwise.",
+                    "type": "string"
+                },
                 "hogql": {
                     "type": "string"
                 },
@@ -5790,6 +5846,10 @@
         "WebOverviewQueryResponse": {
             "additionalProperties": false,
             "properties": {
+                "error": {
+                    "description": "Query error. Returned only if 'explain' or `modifiers.debug` is true. Throws an error otherwise.",
+                    "type": "string"
+                },
                 "hogql": {
                     "type": "string"
                 },
@@ -5905,6 +5965,10 @@
                     "items": {},
                     "type": "array"
                 },
+                "error": {
+                    "description": "Query error. Returned only if 'explain' or `modifiers.debug` is true. Throws an error otherwise.",
+                    "type": "string"
+                },
                 "hasMore": {
                     "type": "boolean"
                 },
@@ -5995,6 +6059,10 @@
                     "items": {},
                     "type": "array"
                 },
+                "error": {
+                    "description": "Query error. Returned only if 'explain' or `modifiers.debug` is true. Throws an error otherwise.",
+                    "type": "string"
+                },
                 "hogql": {
                     "type": "string"
                 },

diff --git a/frontend/src/queries/schema.ts b/frontend/src/queries/schema.ts
@@ -209,7 +209,7 @@ export interface HogQLQueryResponse {
     clickhouse?: string
     /** Query results */
     results?: any[]
-    /** Query error. Returned only if 'explain' is true. Throws an error otherwise. */
+    /** Query error. Returned only if 'explain' or `modifiers.debug` is true. Throws an error otherwise. */
     error?: string
     /** Returned columns */
     columns?: any[]
@@ -896,6 +896,8 @@ export interface QueryResponse {
     results: unknown
     timings?: QueryTiming[]
     hogql?: string
+    /** Query error. Returned only if 'explain' or `modifiers.debug` is true. Throws an error otherwise. */
+    error?: string
     is_cached?: boolean
     last_refresh?: string
     next_allowed_client_refresh?: string

diff --git a/frontend/src/scenes/debug/DebugScene.tsx b/frontend/src/scenes/debug/DebugScene.tsx
@@ -60,11 +60,11 @@ export function DebugScene(): JSX.Element {
             />
             <div className="flex gap-2">
                 <div className="flex-1 w-1/2">
-                    <DebugSceneQuery query={query1} setQuery={setQuery1} queryKey="hogql-debug-1" />
+                    <DebugSceneQuery query={query1} setQuery={setQuery1} queryKey="new-hogql-debug-1" />
                 </div>
                 {query2 ? (
                     <div className="flex-1 w-1/2">
-                        <DebugSceneQuery query={query2} setQuery={setQuery2} queryKey="hogql-debug-2" />
+                        <DebugSceneQuery query={query2} setQuery={setQuery2} queryKey="new-hogql-debug-2" />
                     </div>
                 ) : null}
             </div>

diff --git a/frontend/src/scenes/debug/DebugSceneQuery.tsx b/frontend/src/scenes/debug/DebugSceneQuery.tsx
@@ -4,15 +4,17 @@ import { Modifiers } from 'scenes/debug/Modifiers'
 import { QueryTabs } from 'scenes/debug/QueryTabs'
 
 import { dataNodeLogic, DataNodeLogicProps } from '~/queries/nodes/DataNode/dataNodeLogic'
+import { insightVizDataNodeKey } from '~/queries/nodes/InsightViz/InsightViz'
 import { QueryEditor } from '~/queries/QueryEditor/QueryEditor'
 import { DataNode, HogQLQuery, Node } from '~/queries/schema'
 import { isDataTableNode, isInsightVizNode } from '~/queries/utils'
 
 interface DebugSceneQueryProps {
-    queryKey: string
+    queryKey: `new-${string}`
     query: string
     setQuery: (query: string) => void
 }
+
 export function DebugSceneQuery({ query, setQuery, queryKey }: DebugSceneQueryProps): JSX.Element {
     let parsed: Record<string, any> | null = null
     try {
@@ -23,9 +25,10 @@ export function DebugSceneQuery({ query, setQuery, queryKey }: DebugSceneQueryPr
     const dataNode =
         parsed && (isInsightVizNode(parsed as Node) || isDataTableNode(parsed as Node)) ? parsed.source : parsed
 
+    const dataNodeKey = insightVizDataNodeKey({ dashboardItemId: queryKey })
     const dataNodeLogicProps: DataNodeLogicProps = {
         query: dataNode as DataNode,
-        key: queryKey,
+        key: dataNodeKey,
         dataNodeCollectionId: queryKey,
         modifiers: { debug: true },
     }

diff --git a/frontend/src/scenes/debug/HogQLDebug.tsx b/frontend/src/scenes/debug/HogQLDebug.tsx
@@ -13,7 +13,7 @@ import { DataNode, HogQLQuery, HogQLQueryResponse } from '~/queries/schema'
 import { QueryTabs } from './QueryTabs'
 
 interface HogQLDebugProps {
-    queryKey: string
+    queryKey: `new-${string}`
     query: HogQLQuery
     setQuery: (query: DataNode) => void
 }

diff --git a/frontend/src/scenes/debug/QueryTabs.tsx b/frontend/src/scenes/debug/QueryTabs.tsx
@@ -35,7 +35,7 @@ function toColumn(hogql: string, position: number): number {
 }
 interface QueryTabsProps {
     query: Node
-    queryKey: string
+    queryKey: `new-${string}`
     response?: Record<string, any> | null
     setQuery: (query: DataNode) => void
 }
@@ -62,7 +62,21 @@ export function QueryTabs({ query, queryKey, setQuery, response }: QueryTabsProp
               isInsightVizNode(query) && {
                   key: 'viz',
                   label: 'Visualization',
-                  content: <Query uniqueKey={queryKey} query={query} setQuery={(query) => setQuery(query)} />,
+                  content: (
+                      <Query
+                          uniqueKey={queryKey}
+                          query={query}
+                          setQuery={(query) => setQuery(query)}
+                          context={{
+                              insightProps: {
+                                  dashboardItemId: queryKey,
+                                  query,
+                                  setQuery: (query) => setQuery(query),
+                                  dataNodeCollectionId: queryKey,
+                              },
+                          }}
+                      />
+                  ),
               },
               isInsightQueryNode(query) && {
                   key: 'insight',

diff --git a/posthog/hogql_queries/insights/trends/trends_query_runner.py b/posthog/hogql_queries/insights/trends/trends_query_runner.py
@@ -292,6 +292,7 @@ def calculate(self):
         res_matrix: list[list[Any] | Any | None] = [None] * len(queries)
         timings_matrix: list[list[QueryTiming] | None] = [None] * len(queries)
         errors: list[Exception] = []
+        debug_errors: list[str] = []
 
         def run(index: int, query: ast.SelectQuery | ast.SelectUnionQuery, is_parallel: bool):
             try:
@@ -308,6 +309,8 @@ def run(index: int, query: ast.SelectQuery | ast.SelectUnionQuery, is_parallel:
 
                 timings_matrix[index] = response.timings
                 res_matrix[index] = self.build_series_response(response, series_with_extra, len(queries))
+                if response.error:
+                    debug_errors.append(response.error)
             except Exception as e:
                 errors.append(e)
             finally:
@@ -362,7 +365,9 @@ def run(index: int, query: ast.SelectQuery | ast.SelectUnionQuery, is_parallel:
             with self.timings.measure("apply_formula"):
                 res = self.apply_formula(self.query.trendsFilter.formula, res)
 
-        return TrendsQueryResponse(results=res, timings=timings, hogql=response_hogql, modifiers=self.modifiers)
+        return TrendsQueryResponse(
+            results=res, timings=timings, hogql=response_hogql, modifiers=self.modifiers, error=". ".join(debug_errors)
+        )
 
     def build_series_response(self, response: HogQLQueryResponse, series: SeriesWithExtras, series_count: int):
         if response.results is None:

diff --git a/posthog/hogql_queries/query_runner.py b/posthog/hogql_queries/query_runner.py
@@ -83,6 +83,7 @@ class QueryResponse(BaseModel, Generic[DataT]):
     timings: Optional[list[QueryTiming]] = None
     types: Optional[list[Union[tuple[str, str], str]]] = None
     columns: Optional[list[str]] = None
+    error: Optional[str] = None
     hogql: Optional[str] = None
     hasMore: Optional[bool] = None
     limit: Optional[int] = None
@@ -397,7 +398,12 @@ def run(
         fresh_response_dict["cache_key"] = cache_key
         fresh_response_dict["timezone"] = self.team.timezone
         fresh_response = CachedQueryResponse(**fresh_response_dict)
-        cache.set(cache_key, fresh_response, settings.CACHED_RESULTS_TTL)
+
+        # Dont cache debug queries with errors
+        has_error = fresh_response_dict.get("error", None)
+        if has_error is None or len(has_error) == 0:
+            cache.set(cache_key, fresh_response, settings.CACHED_RESULTS_TTL)
+
         QUERY_CACHE_WRITE_COUNTER.labels(team_id=self.team.pk).inc()
         return fresh_response