From d390f20d635ad3529814c33f938a23b3b623db35 Mon Sep 17 00:00:00 2001 From: Phil Bracikowski Date: Mon, 19 Aug 2024 21:22:04 -0700 Subject: [PATCH 1/4] chore: improve error notifications in annotations and notebooks I noticed that annotations and notebooks could display the api error when requests fail - they are not consistent about it. --- src/annotations/components/annotationForm/AnnotationForm.tsx | 2 +- src/flows/context/flow.list.tsx | 5 +++-- src/shared/copy/notifications/categories/notebooks.ts | 4 ++-- 3 files changed, 6 insertions(+), 5 deletions(-) diff --git a/src/annotations/components/annotationForm/AnnotationForm.tsx b/src/annotations/components/annotationForm/AnnotationForm.tsx index 2b6fe207c6..ae1df2de08 100644 --- a/src/annotations/components/annotationForm/AnnotationForm.tsx +++ b/src/annotations/components/annotationForm/AnnotationForm.tsx @@ -140,7 +140,7 @@ export const AnnotationForm: FC = (props: Props) => { } try { - dispatch(deleteAnnotations(editedAnnotation)) + await dispatch(deleteAnnotations(editedAnnotation)) event(`annotations.delete_annotation.success`, { prefix: props.eventPrefix, }) diff --git a/src/flows/context/flow.list.tsx b/src/flows/context/flow.list.tsx index 802334f7d3..f9b54f4e7a 100644 --- a/src/flows/context/flow.list.tsx +++ b/src/flows/context/flow.list.tsx @@ -34,6 +34,7 @@ import { } from 'src/shared/copy/notifications' import {setCloneName} from 'src/utils/naming' import {CLOUD} from 'src/shared/constants' +import {getErrorMessage} from 'src/utils/api' export interface FlowListContextType extends FlowList { add: (flow?: Flow) => Promise @@ -238,8 +239,8 @@ export const FlowListProvider: FC = ({children}) => { return flow.id }) - .catch(() => { - dispatch(notify(notebookCreateFail())) + .catch(err => { + dispatch(notify(notebookCreateFail(getErrorMessage(err)))) }) } diff --git a/src/shared/copy/notifications/categories/notebooks.ts b/src/shared/copy/notifications/categories/notebooks.ts index c98b8c684b..45389a97b4 100644 --- a/src/shared/copy/notifications/categories/notebooks.ts +++ b/src/shared/copy/notifications/categories/notebooks.ts @@ -21,9 +21,9 @@ export const panelCopyLinkFail = (): Notification => ({ message: `Failed to copy the panel link`, }) -export const notebookCreateFail = (): Notification => ({ +export const notebookCreateFail = (error: string): Notification => ({ ...defaultErrorNotification, - message: `Failed to create Notebook, please try again.`, + message: `Failed to create Notebook, please try again: ${error}`, }) export const notebookUpdateFail = (): Notification => ({ From c509e4f16cb53f1a0a2d4f504a275a660ded4994 Mon Sep 17 00:00:00 2001 From: Phil Bracikowski Date: Fri, 23 Aug 2024 16:28:46 -0700 Subject: [PATCH 2/4] fix: improve failure error message notifications Including the underlying error in error messages, this improves notebooks, dashboards, labels, and annotations. Many other error notifications all need improving e.g. in tasks and secrets. --- .../components/annotationForm/AnnotationForm.tsx | 5 +++-- src/dashboards/actions/thunks.ts | 7 ++++--- src/flows/context/api.tsx | 5 +++-- src/flows/context/flow.current.tsx | 3 ++- src/flows/context/flow.list.tsx | 2 +- src/flows/context/version.publish.tsx | 3 ++- src/labels/actions/thunks.ts | 7 ++++--- .../copy/notifications/categories/dashboard.ts | 8 ++++---- .../copy/notifications/categories/notebooks.ts | 14 +++++++------- src/shared/copy/notifications/categories/tasks.ts | 12 ++++++------ 10 files changed, 36 insertions(+), 30 deletions(-) diff --git a/src/annotations/components/annotationForm/AnnotationForm.tsx b/src/annotations/components/annotationForm/AnnotationForm.tsx index ae1df2de08..1f89368eb1 100644 --- a/src/annotations/components/annotationForm/AnnotationForm.tsx +++ b/src/annotations/components/annotationForm/AnnotationForm.tsx @@ -5,6 +5,7 @@ import {useDispatch} from 'react-redux' // Utils import {event} from 'src/cloud/utils/reporting' import classnames from 'classnames' +import {getErrorMessage} from 'src/utils/api' // Components import { @@ -129,7 +130,7 @@ export const AnnotationForm: FC = (props: Props) => { } } - const handleDelete = () => { + const handleDelete = async () => { const editedAnnotation = { summary, startTime, @@ -150,7 +151,7 @@ export const AnnotationForm: FC = (props: Props) => { event(`annotations.delete_annotation.failure`, { prefix: props.eventPrefix, }) - dispatch(notify(deleteAnnotationFailed(err))) + dispatch(notify(deleteAnnotationFailed(getErrorMessage(err)))) } } diff --git a/src/dashboards/actions/thunks.ts b/src/dashboards/actions/thunks.ts index a9edf38546..322753e740 100644 --- a/src/dashboards/actions/thunks.ts +++ b/src/dashboards/actions/thunks.ts @@ -50,6 +50,7 @@ import {setCloneName} from 'src/utils/naming' import {isLimitError} from 'src/cloud/utils/limits' import {getOrg} from 'src/organizations/selectors' import {getByID, getStatus} from 'src/resources/selectors' +import {getErrorMessage} from 'src/utils/api' // Constants import * as copy from 'src/shared/copy/notifications' @@ -111,7 +112,7 @@ export const createDashboard = if (isLimitError(error)) { dispatch(notify(copy.resourceLimitReached('dashboards'))) } else { - dispatch(notify(copy.dashboardCreateFailed())) + dispatch(notify(copy.dashboardCreateFailed(getErrorMessage(error)))) } } } @@ -199,7 +200,7 @@ export const cloneDashboard = if (isLimitError(error)) { dispatch(notify(copy.resourceLimitReached('dashboards'))) } else { - dispatch(notify(copy.dashboardCreateFailed())) + dispatch(notify(copy.dashboardCreateFailed(getErrorMessage(error)))) } } } @@ -442,7 +443,7 @@ export const updateDashboard = dispatch(creators.editDashboard(updatedDashboard)) } catch (error) { console.error(error) - dispatch(notify(copy.dashboardUpdateFailed())) + dispatch(notify(copy.dashboardUpdateFailed(getErrorMessage(error)))) } } diff --git a/src/flows/context/api.tsx b/src/flows/context/api.tsx index e6055db534..590bffe8e0 100644 --- a/src/flows/context/api.tsx +++ b/src/flows/context/api.tsx @@ -9,6 +9,7 @@ import { } from 'src/client/notebooksRoutes' import {notebookUpdateFail} from 'src/shared/copy/notifications' import {notify} from 'src/shared/actions/notifications' +import {getErrorMessage} from 'src/utils/api' const DEFAULT_API_FLOW: PatchNotebookParams = { id: '', @@ -99,10 +100,10 @@ export const migrateLocalFlowsToAPI = async ( delete flows[localID] flows[id] = flow }) - ).catch(() => { + ).catch((err) => { // do not throw the error because some flows might have saved and we // need to save the new IDs to avoid creating duplicates next time. - dispatch(notify(notebookUpdateFail())) + dispatch(notify(notebookUpdateFail(getErrorMessage(err)))) }) } return flows diff --git a/src/flows/context/flow.current.tsx b/src/flows/context/flow.current.tsx index ebf9692d1c..694bf8b2e3 100644 --- a/src/flows/context/flow.current.tsx +++ b/src/flows/context/flow.current.tsx @@ -24,6 +24,7 @@ import PageSpinner from 'src/perf/components/PageSpinner' import {pageTitleSuffixer} from 'src/shared/utils/pageTitles' import {RemoteDataState} from '@influxdata/clockface' import {setCloneName} from 'src/utils/naming' +import {getErrorMessage} from 'src/utils/api' const prettyid = customAlphabet('abcdefghijklmnop0123456789', 12) @@ -84,7 +85,7 @@ export const FlowProvider: FC = ({children}) => { dispatch(notify(notebookDeleteSuccess())) return id } catch (error) { - dispatch(notify(notebookDeleteFail())) + dispatch(notify(notebookDeleteFail(getErrorMessage(error)))) } }, [dispatch, id]) diff --git a/src/flows/context/flow.list.tsx b/src/flows/context/flow.list.tsx index f9b54f4e7a..1641ebdbfb 100644 --- a/src/flows/context/flow.list.tsx +++ b/src/flows/context/flow.list.tsx @@ -296,7 +296,7 @@ export const FlowListProvider: FC = ({children}) => { await deleteAPI({id}) dispatch(notify(notebookDeleteSuccess())) } catch (error) { - dispatch(notify(notebookDeleteFail())) + dispatch(notify(notebookDeleteFail(getErrorMessage(error)))) } delete _flows[id] diff --git a/src/flows/context/version.publish.tsx b/src/flows/context/version.publish.tsx index e30bc85447..c21770b722 100644 --- a/src/flows/context/version.publish.tsx +++ b/src/flows/context/version.publish.tsx @@ -27,6 +27,7 @@ import { publishNotebookSuccessful, } from 'src/shared/copy/notifications' import {event} from 'src/cloud/utils/reporting' +import {getErrorMessage} from 'src/utils/api' // Types import {RemoteDataState} from 'src/types' @@ -103,7 +104,7 @@ export const VersionPublishProvider: FC = ({children}) => { setPublishLoading(RemoteDataState.Done) handleGetNotebookVersions() } catch (error) { - dispatch(notify(publishNotebookFailed(flow.name))) + dispatch(notify(publishNotebookFailed(flow.name, getErrorMessage(error)))) setPublishLoading(RemoteDataState.Error) } }, [dispatch, handleGetNotebookVersions, flow.id, flow.name]) diff --git a/src/labels/actions/thunks.ts b/src/labels/actions/thunks.ts index 4b7e924873..96967e1665 100644 --- a/src/labels/actions/thunks.ts +++ b/src/labels/actions/thunks.ts @@ -42,6 +42,7 @@ import { import {getOrg} from 'src/organizations/selectors' import {viewableLabels} from 'src/labels/selectors' import {getStatus} from 'src/resources/selectors' +import {getErrorMessage} from 'src/utils/api' export const getLabels = () => async (dispatch: Dispatch, getState: GetState) => { @@ -99,7 +100,7 @@ export const createLabel = dispatch(setLabel(resp.data.label.id, RemoteDataState.Done, label)) } catch (error) { console.error(error) - dispatch(notify(createLabelFailed())) + dispatch(notify(createLabelFailed(getErrorMessage(error)))) } } @@ -121,7 +122,7 @@ export const updateLabel = dispatch(setLabel(id, RemoteDataState.Done, label)) } catch (error) { console.error(error) - dispatch(notify(updateLabelFailed())) + dispatch(notify(updateLabelFailed(getErrorMessage(error)))) } } @@ -137,6 +138,6 @@ export const deleteLabel = dispatch(removeLabel(id)) } catch (error) { console.error(error) - dispatch(notify(deleteLabelFailed())) + dispatch(notify(deleteLabelFailed(getErrorMessage(error)))) } } diff --git a/src/shared/copy/notifications/categories/dashboard.ts b/src/shared/copy/notifications/categories/dashboard.ts index 7179550615..bd13480058 100644 --- a/src/shared/copy/notifications/categories/dashboard.ts +++ b/src/shared/copy/notifications/categories/dashboard.ts @@ -23,10 +23,10 @@ export const dashboardsGetFailed = (error: string): Notification => ({ message: `Failed to retrieve dashboards: ${error}`, }) -export const dashboardUpdateFailed = (): Notification => ({ +export const dashboardUpdateFailed = (error: string): Notification => ({ ...defaultErrorNotification, icon: IconFont.DashH, - message: 'Could not update dashboard', + message: `Could not update dashboard: ${error}`, }) export const dashboardDeleted = (name: string): Notification => ({ @@ -35,9 +35,9 @@ export const dashboardDeleted = (name: string): Notification => ({ message: `Dashboard ${name} deleted successfully.`, }) -export const dashboardCreateFailed = () => ({ +export const dashboardCreateFailed = (error: string) => ({ ...defaultErrorNotification, - message: 'Failed to create dashboard.', + message: `Failed to create dashboard: ${error}`, }) export const dashboardCreateSuccess = () => ({ diff --git a/src/shared/copy/notifications/categories/notebooks.ts b/src/shared/copy/notifications/categories/notebooks.ts index 45389a97b4..ee0a983155 100644 --- a/src/shared/copy/notifications/categories/notebooks.ts +++ b/src/shared/copy/notifications/categories/notebooks.ts @@ -23,17 +23,17 @@ export const panelCopyLinkFail = (): Notification => ({ export const notebookCreateFail = (error: string): Notification => ({ ...defaultErrorNotification, - message: `Failed to create Notebook, please try again: ${error}`, + message: `Failed to create Notebook: ${error}`, }) -export const notebookUpdateFail = (): Notification => ({ +export const notebookUpdateFail = (error: string): Notification => ({ ...defaultErrorNotification, - message: `Failed to save changes to Notebook, please try again.`, + message: `Failed to save changes to Notebook: ${error}`, }) -export const notebookDeleteFail = (): Notification => ({ +export const notebookDeleteFail = (error: string): Notification => ({ ...defaultErrorNotification, - message: `Failed to delete Notebook, please try again.`, + message: `Failed to delete Notebook: ${error}`, }) export const notebookDeleteSuccess = (): Notification => ({ @@ -58,7 +58,7 @@ export const publishNotebookSuccessful = (name: string): Notification => ({ message: `Successfully saved this version to ${name}'s version history.`, }) -export const publishNotebookFailed = (name: string): Notification => ({ +export const publishNotebookFailed = (name: string, error: string): Notification => ({ ...defaultErrorNotification, - message: `Failed to save this version to ${name}'s version history`, + message: `Failed to save this version to ${name}'s version history: ${error}`, }) diff --git a/src/shared/copy/notifications/categories/tasks.ts b/src/shared/copy/notifications/categories/tasks.ts index 2b58f4a676..c4905177db 100644 --- a/src/shared/copy/notifications/categories/tasks.ts +++ b/src/shared/copy/notifications/categories/tasks.ts @@ -23,19 +23,19 @@ export const getLabelsFailed = (): Notification => ({ message: 'Failed to fetch labels', }) -export const createLabelFailed = (): Notification => ({ +export const createLabelFailed = (error: string): Notification => ({ ...defaultErrorNotification, - message: 'Failed to create label', + message: `Failed to create label: ${error}`, }) -export const updateLabelFailed = (): Notification => ({ +export const updateLabelFailed = (error: string): Notification => ({ ...defaultErrorNotification, - message: 'Failed to update label', + message: `Failed to update label: ${error}`, }) -export const deleteLabelFailed = (): Notification => ({ +export const deleteLabelFailed = (error: string): Notification => ({ ...defaultErrorNotification, - message: 'Failed to delete label', + message: `Failed to delete label: ${error}`, }) export const taskNotCreated = (additionalMessage: string): Notification => ({ From 5f5d22d0c9eebd2311587b2eeae9e964c8bda104 Mon Sep 17 00:00:00 2001 From: Phil Bracikowski Date: Mon, 26 Aug 2024 11:24:29 -0700 Subject: [PATCH 3/4] chore: run yarn prettier:fix --- src/flows/context/api.tsx | 2 +- src/shared/copy/notifications/categories/notebooks.ts | 5 ++++- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/src/flows/context/api.tsx b/src/flows/context/api.tsx index 590bffe8e0..9f61233979 100644 --- a/src/flows/context/api.tsx +++ b/src/flows/context/api.tsx @@ -100,7 +100,7 @@ export const migrateLocalFlowsToAPI = async ( delete flows[localID] flows[id] = flow }) - ).catch((err) => { + ).catch(err => { // do not throw the error because some flows might have saved and we // need to save the new IDs to avoid creating duplicates next time. dispatch(notify(notebookUpdateFail(getErrorMessage(err)))) diff --git a/src/shared/copy/notifications/categories/notebooks.ts b/src/shared/copy/notifications/categories/notebooks.ts index ee0a983155..1a9226563c 100644 --- a/src/shared/copy/notifications/categories/notebooks.ts +++ b/src/shared/copy/notifications/categories/notebooks.ts @@ -58,7 +58,10 @@ export const publishNotebookSuccessful = (name: string): Notification => ({ message: `Successfully saved this version to ${name}'s version history.`, }) -export const publishNotebookFailed = (name: string, error: string): Notification => ({ +export const publishNotebookFailed = ( + name: string, + error: string +): Notification => ({ ...defaultErrorNotification, message: `Failed to save this version to ${name}'s version history: ${error}`, }) From 9e1c7615dd36b755e6dad805e814fd10ec9f1396 Mon Sep 17 00:00:00 2001 From: Phil Bracikowski Date: Wed, 28 Aug 2024 14:08:14 -0700 Subject: [PATCH 4/4] chore: whitespace to trigger ci --- src/annotations/components/annotationForm/AnnotationForm.tsx | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/annotations/components/annotationForm/AnnotationForm.tsx b/src/annotations/components/annotationForm/AnnotationForm.tsx index 1f89368eb1..1a1ae7bb79 100644 --- a/src/annotations/components/annotationForm/AnnotationForm.tsx +++ b/src/annotations/components/annotationForm/AnnotationForm.tsx @@ -59,7 +59,8 @@ export const START_TIME_IN_FUTURE_MESSAGE = 'Start Time cannot be in the future' /** * Form for editing and creating annotations. - * It does support multi-line annotations, but the tradeoff is that the user cannot then press 'return' to submit the form. + * It does support multi-line annotations, but the tradeoff is that the user + * cannot then press 'return' to submit the form. * */ export const AnnotationForm: FC = (props: Props) => { const [startTime, setStartTime] = useState(props.startTime)