update error handling

PostHog · Dec 4, 2024 · 55e0a63 · 55e0a63
1 parent fc253af
commit 55e0a63
Show file tree

Hide file tree

Showing 4 changed files with 28 additions and 81 deletions.
diff --git a/frontend/src/lib/monaco/CodeEditor.tsx b/frontend/src/lib/monaco/CodeEditor.tsx
@@ -32,6 +32,8 @@ export interface CodeEditorProps extends Omit<EditorProps, 'loading' | 'theme'>
     sourceQuery?: AnyDataNode
     globals?: Record<string, any>
     schema?: Record<string, any> | null
+
+    onError?: (error: string | null, isValidView: boolean) => void
 }
 let codeEditorIndex = 0
 
@@ -118,6 +120,7 @@ export function CodeEditor({
     globals,
     sourceQuery,
     schema,
+    onError,
     ...editorProps
 }: CodeEditorProps): JSX.Element {
     const { isDarkModeOn } = useValues(themeLogic)
@@ -136,6 +139,7 @@ export function CodeEditor({
         sourceQuery,
         monaco: monaco,
         editor: editor,
+        onError,
     })
     useMountedLogic(builtCodeEditorLogic)
 

diff --git a/frontend/src/lib/monaco/codeEditorLogic.tsx b/frontend/src/lib/monaco/codeEditorLogic.tsx
@@ -1,6 +1,7 @@
 import type { Monaco } from '@monaco-editor/react'
 import { actions, connect, kea, key, listeners, path, props, propsChanged, reducers, selectors } from 'kea'
 import { loaders } from 'kea-loaders'
+import { subscriptions } from 'kea-subscriptions'
 import { FEATURE_FLAGS } from 'lib/constants'
 import { featureFlagLogic } from 'lib/logic/featureFlagLogic'
 // Note: we can oly import types and not values from monaco-editor, because otherwise some Monaco code breaks
@@ -48,6 +49,7 @@ export interface CodeEditorLogicProps {
     editor?: editor.IStandaloneCodeEditor | null
     globals?: Record<string, any>
     multitab?: boolean
+    onError?: (error: string | null, isValidView: boolean) => void
 }
 
 export const codeEditorLogic = kea<codeEditorLogicType>([
@@ -270,6 +272,14 @@ export const codeEditorLogic = kea<codeEditorLogicType>([
             },
         ],
     }),
+    subscriptions(({ props, values }) => ({
+        isValidView: (isValidView) => {
+            props.onError?.(values.error, isValidView)
+        },
+        error: (error) => {
+            props.onError?.(error, values.isValidView)
+        },
+    })),
     propsChanged(({ actions, props }, oldProps) => {
         if (
             props.query !== oldProps.query ||

diff --git a/frontend/src/scenes/data-warehouse/editor/QueryWindow.tsx b/frontend/src/scenes/data-warehouse/editor/QueryWindow.tsx
@@ -87,6 +87,9 @@ interface InternalQueryWindowProps {
 }
 
 function InternalQueryWindow({ setQuery, query }: InternalQueryWindowProps): JSX.Element {
+    const [error, setError] = useState<string | null>(null)
+    const [isValidView, setIsValidView] = useState(true)
+
     const [monacoAndEditor, setMonacoAndEditor] = useState(
         null as [Monaco, importedEditor.IStandaloneCodeEditor] | null
     )
@@ -111,17 +114,7 @@ function InternalQueryWindow({ setQuery, query }: InternalQueryWindowProps): JSX
         },
     })
 
-    const {
-        allTabs,
-        activeModelUri,
-        queryInput,
-        activeQuery,
-        hasErrors,
-        error,
-        isValidView,
-        editingView,
-        exportContext,
-    } = useValues(logic)
+    const { allTabs, activeModelUri, queryInput, activeQuery, editingView, exportContext } = useValues(logic)
     const { selectTab, deleteTab, createTab, setQueryInput, runQuery, saveAsView, saveAsInsight } = useActions(logic)
 
     return (
@@ -157,6 +150,10 @@ function InternalQueryWindow({ setQuery, query }: InternalQueryWindowProps): JSX
                             runQuery()
                         }
                     },
+                    onError: (error, isValidView) => {
+                        setError(error)
+                        setIsValidView(isValidView)
+                    },
                 }}
             />
             <OutputPane
@@ -166,9 +163,7 @@ function InternalQueryWindow({ setQuery, query }: InternalQueryWindowProps): JSX
                 onSaveView={saveAsView}
                 onSaveInsight={saveAsInsight}
                 exportContext={exportContext}
-                saveDisabledReason={
-                    hasErrors ? error ?? 'Query has errors' : !isValidView ? 'Some fields may need an alias' : ''
-                }
+                saveDisabledReason={error ? error : !isValidView ? 'Some fields may need an alias' : ''}
             />
         </div>
     )

diff --git a/frontend/src/scenes/data-warehouse/editor/multitabEditorLogic.tsx b/frontend/src/scenes/data-warehouse/editor/multitabEditorLogic.tsx
@@ -5,14 +5,14 @@ import { router } from 'kea-router'
 import { subscriptions } from 'kea-subscriptions'
 import { LemonField } from 'lib/lemon-ui/LemonField'
 import { initModel } from 'lib/monaco/CodeEditor'
-import { codeEditorLogic, ModelMarker } from 'lib/monaco/codeEditorLogic'
-import { editor, MarkerSeverity, Uri } from 'monaco-editor'
+import { codeEditorLogic } from 'lib/monaco/codeEditorLogic'
+import { editor, Uri } from 'monaco-editor'
 import { insightsApi } from 'scenes/insights/utils/api'
 import { urls } from 'scenes/urls'
 
 import { dataNodeLogic } from '~/queries/nodes/DataNode/dataNodeLogic'
 import { queryExportContext } from '~/queries/query'
-import { HogQLMetadataResponse, HogQLNotice, HogQLQuery, NodeKind } from '~/queries/schema'
+import { HogQLMetadataResponse, HogQLQuery, NodeKind } from '~/queries/schema'
 import { DataVisualizationNode } from '~/queries/schema'
 import { DataWarehouseSavedQuery, ExportContext } from '~/types'
 
@@ -69,7 +69,7 @@ export const multitabEditorLogic = kea<multitabEditorLogicType>([
             actions.initialize()
         }
     }),
-    reducers(({ props }) => ({
+    reducers({
         queryInput: [
             '',
             {
@@ -108,55 +108,7 @@ export const multitabEditorLogic = kea<multitabEditorLogicType>([
                 setTabs: (_, { tabs }) => tabs,
             },
         ],
-        metadata: [
-            null as null | [string, HogQLMetadataResponse],
-            {
-                setMetadata: (_, { query, metadata }) => [query, metadata],
-            },
-        ],
-        modelMarkers: [
-            [] as ModelMarker[],
-            {
-                setMetadata: (_, { query, metadata }) => {
-                    const model = props.editor?.getModel()
-                    if (!model || !metadata) {
-                        return []
-                    }
-                    const markers: ModelMarker[] = []
-                    const metadataResponse = metadata
-
-                    function noticeToMarker(error: HogQLNotice, severity: MarkerSeverity): ModelMarker {
-                        const start = model!.getPositionAt(error.start ?? 0)
-                        const end = model!.getPositionAt(error.end ?? query.length)
-                        return {
-                            start: error.start ?? 0,
-                            startLineNumber: start.lineNumber,
-                            startColumn: start.column,
-                            end: error.end ?? query.length,
-                            endLineNumber: end.lineNumber,
-                            endColumn: end.column,
-                            message: error.message ?? 'Unknown error',
-                            severity: severity,
-                            hogQLFix: error.fix,
-                        }
-                    }
-
-                    for (const notice of metadataResponse?.errors ?? []) {
-                        markers.push(noticeToMarker(notice, 8 /* MarkerSeverity.Error */))
-                    }
-                    for (const notice of metadataResponse?.warnings ?? []) {
-                        markers.push(noticeToMarker(notice, 4 /* MarkerSeverity.Warning */))
-                    }
-                    for (const notice of metadataResponse?.notices ?? []) {
-                        markers.push(noticeToMarker(notice, 1 /* MarkerSeverity.Hint */))
-                    }
-
-                    props.monaco?.editor.setModelMarkers(model, 'hogql', markers)
-                    return markers
-                },
-            },
-        ],
-    })),
+    }),
     listeners(({ values, props, actions, asyncActions }) => ({
         createTab: ({ query = '', view }) => {
             const mountedCodeEditorLogic = codeEditorLogic.findMounted()
@@ -428,20 +380,6 @@ export const multitabEditorLogic = kea<multitabEditorLogicType>([
     })),
     selectors({
         activeTabKey: [(s) => [s.activeModelUri], (activeModelUri) => `hogQLQueryEditor/${activeModelUri?.uri.path}`],
-        isValidView: [(s) => [s.metadata], (metadata) => !!(metadata && metadata[1]?.isValidView)],
-        hasErrors: [
-            (s) => [s.modelMarkers],
-            (modelMarkers) => !!(modelMarkers ?? []).filter((e) => e.severity === 8 /* MarkerSeverity.Error */).length,
-        ],
-        error: [
-            (s) => [s.hasErrors, s.modelMarkers],
-            (hasErrors, modelMarkers) => {
-                const firstError = modelMarkers.find((e) => e.severity === 8 /* MarkerSeverity.Error */)
-                return hasErrors && firstError
-                    ? `Error on line ${firstError.startLineNumber}, column ${firstError.startColumn}`
-                    : null
-            },
-        ],
         exportContext: [
             () => [(_, props) => props.sourceQuery],
             (sourceQuery) => {