Skip to content

Commit

Permalink
fix: snuba admin system query error messaging (#6763)
Browse files Browse the repository at this point in the history
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"
/>
  • Loading branch information
kylemumma authored Jan 14, 2025
1 parent fd64fa5 commit 3b49305
Show file tree
Hide file tree
Showing 2 changed files with 46 additions and 2 deletions.
46 changes: 45 additions & 1 deletion snuba/admin/static/clickhouse_queries/query_display.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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]);
Expand Down Expand Up @@ -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}>
Expand Down Expand Up @@ -168,6 +208,7 @@ function QueryDisplay(props: {
</div>
<div>
<ExecuteButton
onError={handleQueryError}
onClick={executeQuery}
disabled={
!query.storage || !query.host || !query.port || !query.sql
Expand All @@ -176,6 +217,9 @@ function QueryDisplay(props: {
</div>
</div>
</form>
<div>
{getErrorDomElement()}
</div>
<div>
<h2>Query results</h2>
{queryResultHistory.map((queryResult, idx) => {
Expand Down
2 changes: 1 addition & 1 deletion snuba/admin/static/utils/execute_button.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down

0 comments on commit 3b49305

Please sign in to comment.