From 3b49305e30c4ecd63701f5a756a6d2e4407ae754 Mon Sep 17 00:00:00 2001
From: Kyle Mumma <kyle.mumma@sentry.io>
Date: Tue, 14 Jan 2025 15:46:14 -0800
Subject: [PATCH] fix: snuba admin system query error messaging (#6763)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

this fixes error messaging in the system queries tool of snuba admin. as
a follow up i will see if i can expand this new error messaging to the
rest of the tools.

heres how it used to look
<img width="1227" alt="Screenshot 2025-01-13 at 4 20 59 PM"
src="https://github.com/user-attachments/assets/bc5d65fe-0a51-4629-9c47-72eef72ec26c"
/>

and heres how it looks now
<img width="1207" alt="Screenshot 2025-01-14 at 3 05 30 PM"
src="https://github.com/user-attachments/assets/35575aaf-14e1-466f-8a09-ffccc7141980"
/>
---
 .../clickhouse_queries/query_display.tsx      | 46 ++++++++++++++++++-
 snuba/admin/static/utils/execute_button.tsx   |  2 +-
 2 files changed, 46 insertions(+), 2 deletions(-)

diff --git a/snuba/admin/static/clickhouse_queries/query_display.tsx b/snuba/admin/static/clickhouse_queries/query_display.tsx
index 1e9d2bd0d08..8ffa1c479e3 100644
--- a/snuba/admin/static/clickhouse_queries/query_display.tsx
+++ b/snuba/admin/static/clickhouse_queries/query_display.tsx
@@ -4,7 +4,7 @@ import { Collapse } from "SnubaAdmin/collapse";
 import QueryEditor from "SnubaAdmin/query_editor";
 import ExecuteButton from "SnubaAdmin/utils/execute_button";
 
-import { SelectItem, Switch } from "@mantine/core";
+import { SelectItem, Switch, Alert } from "@mantine/core";
 import { Prism } from "@mantine/prism";
 import { RichTextEditor } from "@mantine/tiptap";
 import { useEditor } from "@tiptap/react";
@@ -37,6 +37,12 @@ function QueryDisplay(props: {
     getRecentHistory(HISTORY_KEY)
   );
 
+  type QueryError = {
+    title: string;
+    body: string;
+  }
+  const [queryError, setQueryError] = useState<QueryError | null>(null);
+
   useEffect(() => {
     props.api.getClickhouseNodes().then((res) => {
       setNodeData(res);
@@ -90,6 +96,7 @@ function QueryDisplay(props: {
     return props.api
       .executeSystemQuery(query as QueryRequest)
       .then((result) => {
+        setQueryError(null);  // clear any previous error
         result.input_query = `${query.sql} (${query.storage},${query.host}:${query.port})`;
         setRecentHistory(HISTORY_KEY, result);
         setQueryResultHistory((prevHistory) => [result, ...prevHistory]);
@@ -127,6 +134,39 @@ function QueryDisplay(props: {
     return [];
   }
 
+  function getErrorDomElement() {
+    if (queryError !== null) {
+      const bodyDOM = queryError.body.split("\n").map((line) => <React.Fragment>{line}< br /></React.Fragment>)
+      return <Alert title={queryError.title} color="red">{bodyDOM}</Alert>;
+    }
+    return "";
+  }
+
+  function handleQueryError(error: any) {
+    try {
+      // this block assumes that the error is an object with an error property,
+      // if its not it will be caught by the catch block
+      const lines = error.error.split("\n");
+      if (lines.length > 1) {
+        setQueryError({
+          title: lines[0],
+          body: lines.slice(1).join("\n"),
+        })
+      } else {
+        setQueryError({
+          title: "Error",
+          body: lines[0]
+        });
+      }
+    } catch (e) {
+      if (typeof error === "object") {
+        setQueryError({ title: "Error", body: JSON.stringify(error) });
+      } else {
+        setQueryError({ title: "Error", body: error.toString() });
+      }
+    }
+  }
+
   return (
     <div>
       <form style={query.sudo ? sudoForm : standardForm}>
@@ -168,6 +208,7 @@ function QueryDisplay(props: {
           </div>
           <div>
             <ExecuteButton
+              onError={handleQueryError}
               onClick={executeQuery}
               disabled={
                 !query.storage || !query.host || !query.port || !query.sql
@@ -176,6 +217,9 @@ function QueryDisplay(props: {
           </div>
         </div>
       </form>
+      <div>
+        {getErrorDomElement()}
+      </div>
       <div>
         <h2>Query results</h2>
         {queryResultHistory.map((queryResult, idx) => {
diff --git a/snuba/admin/static/utils/execute_button.tsx b/snuba/admin/static/utils/execute_button.tsx
index bab518c8891..ae14aadb1f8 100644
--- a/snuba/admin/static/utils/execute_button.tsx
+++ b/snuba/admin/static/utils/execute_button.tsx
@@ -4,7 +4,7 @@ import { Button } from "@mantine/core";
 function ExecuteButton(props: {
   disabled: boolean;
   onClick: () => Promise<any>;
-  onError?: (error: any) => any;
+  onError?: (error: any) => any;  // TODO: we should make the type of error we return more specific
   label?: string;
 }) {
   const [isExecuting, setIsExecuting] = useState<boolean>(false);