Skip to content

Commit

Permalink
chore: improve error notifications in annotations and notebooks (#6927)
Browse files Browse the repository at this point in the history
* 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.

* 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.

* chore: run yarn prettier:fix

* chore: whitespace to trigger ci
  • Loading branch information
philjb authored Sep 10, 2024
1 parent 66803f5 commit 71f8c45
Show file tree
Hide file tree
Showing 10 changed files with 46 additions and 35 deletions.
10 changes: 6 additions & 4 deletions src/annotations/components/annotationForm/AnnotationForm.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -58,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: Props) => {
const [startTime, setStartTime] = useState(props.startTime)
Expand Down Expand Up @@ -129,7 +131,7 @@ export const AnnotationForm: FC<Props> = (props: Props) => {
}
}

const handleDelete = () => {
const handleDelete = async () => {
const editedAnnotation = {
summary,
startTime,
Expand All @@ -140,7 +142,7 @@ export const AnnotationForm: FC<Props> = (props: Props) => {
}

try {
dispatch(deleteAnnotations(editedAnnotation))
await dispatch(deleteAnnotations(editedAnnotation))
event(`annotations.delete_annotation.success`, {
prefix: props.eventPrefix,
})
Expand All @@ -150,7 +152,7 @@ export const AnnotationForm: FC<Props> = (props: Props) => {
event(`annotations.delete_annotation.failure`, {
prefix: props.eventPrefix,
})
dispatch(notify(deleteAnnotationFailed(err)))
dispatch(notify(deleteAnnotationFailed(getErrorMessage(err))))
}
}

Expand Down
7 changes: 4 additions & 3 deletions src/dashboards/actions/thunks.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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'
Expand Down Expand Up @@ -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))))
}
}
}
Expand Down Expand Up @@ -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))))
}
}
}
Expand Down Expand Up @@ -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))))
}
}

Expand Down
5 changes: 3 additions & 2 deletions src/flows/context/api.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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: '',
Expand Down Expand Up @@ -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
Expand Down
3 changes: 2 additions & 1 deletion src/flows/context/flow.current.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down Expand Up @@ -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])

Expand Down
7 changes: 4 additions & 3 deletions src/flows/context/flow.list.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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<string | void>
Expand Down Expand Up @@ -238,8 +239,8 @@ export const FlowListProvider: FC = ({children}) => {

return flow.id
})
.catch(() => {
dispatch(notify(notebookCreateFail()))
.catch(err => {
dispatch(notify(notebookCreateFail(getErrorMessage(err))))
})
}

Expand Down Expand Up @@ -295,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]
Expand Down
3 changes: 2 additions & 1 deletion src/flows/context/version.publish.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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'
Expand Down Expand Up @@ -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])
Expand Down
7 changes: 4 additions & 3 deletions src/labels/actions/thunks.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<Action>, getState: GetState) => {
Expand Down Expand Up @@ -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))))
}
}

Expand All @@ -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))))
}
}

Expand All @@ -137,6 +138,6 @@ export const deleteLabel =
dispatch(removeLabel(id))
} catch (error) {
console.error(error)
dispatch(notify(deleteLabelFailed()))
dispatch(notify(deleteLabelFailed(getErrorMessage(error))))
}
}
8 changes: 4 additions & 4 deletions src/shared/copy/notifications/categories/dashboard.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 => ({
Expand All @@ -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 = () => ({
Expand Down
19 changes: 11 additions & 8 deletions src/shared/copy/notifications/categories/notebooks.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,19 +21,19 @@ 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: ${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 => ({
Expand All @@ -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): 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}`,
})
12 changes: 6 additions & 6 deletions src/shared/copy/notifications/categories/tasks.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 => ({
Expand Down

0 comments on commit 71f8c45

Please sign in to comment.