feat: when query variables fail to load, show a message to the user (#3444)

Author: hoorayimhelping

cypress/e2e/shared/dashboardsView.test.ts

@@ -884,9 +884,17 @@ csv.from(csv: data) |> filter(fn: (r) => r.bucket == v.bucketsCSV)`
             // and cause the rest to exist in loading states
             cy.getByTestID('variable-dropdown--build').should(
               'contain',
-              'Loading'
+              'Error'
             )
 
+            // An error notification should tell the user that this failed
+            cy.getByTestID('notification-error').contains(
+              'could not find bucket "beans"'
+            )
+            cy.getByTestID('notification-error--dismiss').click({
+              multiple: true,
+            })
+
             cy.getByTestIDSubStr('cell--view-empty')
 
             // But selecting a nonempty bucket should load some data
@@ -1039,11 +1047,17 @@ csv.from(csv: data) |> filter(fn: (r) => r.bucket == v.bucketsCSV)`
           'beans'
         )
 
-        // and cause the rest to exist in loading states
+        // and cause the rest to exist in error states
         cy.getByTestIDSubStr('variable-dropdown--dependent').should(
           'contain',
-          'Loading'
+          'Error'
+        )
+
+        // An error notification should tell the user that this failed
+        cy.getByTestID('notification-error').contains(
+          'could not find bucket "beans"'
         )
+        cy.getByTestID('notification-error--dismiss').click()
 
         cy.getByTestIDSubStr('cell--view-empty')
 

src/shared/copy/notifications.ts

@@ -428,6 +428,12 @@ export const getVariableFailed = (): Notification => ({
   message: 'Failed to fetch variable',
 })
 
+export const getVariableFailedWithMessage = (name, message): Notification => ({
+  ...defaultErrorNotification,
+  duration: INDEFINITE,
+  message: `Failed to fetch variable ${name}: ${message}`,
+})
+
 export const createVariableFailed = (error: string): Notification => ({
   ...defaultErrorNotification,
   icon: IconFont.Cube,

src/variables/actions/thunks.ts

@@ -82,6 +82,7 @@ export const getVariables = (controller?: AbortController) => async (
       {query: {orgID: org.id}},
       {signal: controller?.signal}
     )
+
     if (!resp) {
       return
     }
@@ -186,6 +187,12 @@ export const hydrateVariables = (
       dispatch(setVariable(variable.id, RemoteDataState.Done, _variable))
     }
   })
+
+  hydration.on('error', (variable, error) => {
+    dispatch(setVariable(variable.id, RemoteDataState.Error))
+    dispatch(notify(copy.getVariableFailedWithMessage(variable?.name, error)))
+  })
+
   await hydration.promise
 }
 

src/variables/components/TypeAheadVariableDropdown.tsx

@@ -267,7 +267,11 @@ class TypeAheadVariableDropdown extends PureComponent<Props, MyState> {
     }
 
     const getInnerComponent = () => {
-      if (status === RemoteDataState.Loading || this.noValuesPresent()) {
+      if (
+        status === RemoteDataState.Loading ||
+        status === RemoteDataState.Error ||
+        this.noValuesPresent()
+      ) {
         return placeHolderText
       } else {
         return (
@@ -385,6 +389,10 @@ class TypeAheadVariableDropdown extends PureComponent<Props, MyState> {
     if (status === RemoteDataState.Loading) {
       return 'Loading...'
     }
+    if (status === RemoteDataState.Error) {
+      return 'Error'
+    }
+
     if (this.noFilteredValuesPresent()) {
       return 'No Values'
     }

src/variables/mocks/index.ts

@@ -46,7 +46,8 @@ export const defaultVariableAssignments: VariableAssignment[] = [
 export const createVariable = (
   name: string,
   query: string,
-  selected?: string
+  selected?: string,
+  status: RemoteDataState = RemoteDataState.Done
 ): Variable => ({
   name,
   id: name,
@@ -60,7 +61,7 @@ export const createVariable = (
       language: 'flux',
     },
   },
-  status: RemoteDataState.Done,
+  status,
 })
 
 export const createMapVariable = (

src/variables/utils/hydrateVars.test.ts

@@ -177,12 +177,16 @@ describe('hydrate vars', () => {
 
     fetcher.setResponse(b, Promise.reject('oopsy whoopsies'))
 
-    const actual = await hydrateVars(vars, vars, {
+    const response = hydrateVars(vars, vars, {
       url: '',
       orgID: '',
       selections: {},
       fetcher,
-    }).promise
+    })
+
+    const mockFire = jest.spyOn(response.on, 'fire')
+
+    const actual = await response.promise
 
     // We expect the following end state:
     //
@@ -204,6 +208,12 @@ describe('hydrate vars', () => {
       actual.filter(v => v.id === 'c')[0].arguments.values.results
     ).toEqual(['cVal'])
     expect(actual.filter(v => v.id === 'c')[0].selected).toEqual(['cVal'])
+
+    const errorCall = mockFire.mock.calls[4]
+    const [eventName, variable, errorMessage] = errorCall
+    expect(eventName).toBe('error')
+    expect(variable).toEqual(b)
+    expect(errorMessage).toBe('oopsy whoopsies')
   })
 
   test('works with map template variables', async () => {
@@ -356,4 +366,33 @@ describe('hydrate vars', () => {
     }).promise
     expect(actual.length).toEqual(2)
   })
+
+  it('fires an error event when an error is caught', async () => {
+    const error = 'four_oh_four not found'
+    const fetcher = new FakeFetcher()
+    const queryVariable = createVariable(
+      'errorVariable',
+      'f(x: v.four_oh_four)',
+      null,
+      RemoteDataState.Error
+    )
+
+    fetcher.setResponse(queryVariable, Promise.reject(error))
+
+    const actual = hydrateVars([queryVariable], [queryVariable], {
+      url: '',
+      orgID: '',
+      selections: {},
+      fetcher,
+    })
+    const mockFire = jest.spyOn(actual.on, 'fire')
+
+    await actual.promise
+
+    const [_, errorCall] = mockFire.mock.calls
+    const [eventName, variable, errorMessage] = errorCall
+    expect(eventName).toBe('error')
+    expect(variable).toEqual(queryVariable)
+    expect(errorMessage).toBe(error)
+  })
 })

src/variables/utils/hydrateVars.ts

@@ -338,11 +338,13 @@ const invalidateCycles = (graph: VariableNode[]): void => {
 /*
   Given a node, mark all ancestors of that node as `Error`.
 */
-const invalidateAncestors = (node: VariableNode): void => {
+const invalidateAncestors = (node: VariableNode, e: Error, on: any): void => {
   const ancestors = collectAncestors(node)
 
   for (const ancestor of ancestors) {
     ancestor.status = RemoteDataState.Error
+    on.fire('error', ancestor.variable, e)
+
     if (ancestor.variable.arguments.type === 'query') {
       ancestor.variable.arguments.values.results = []
     }
@@ -465,7 +467,8 @@ export const hydrateVars = (
       node.status = RemoteDataState.Error
       node.variable.arguments.values.results = []
 
-      invalidateAncestors(node)
+      on.fire('error', node.variable, e)
+      invalidateAncestors(node, e, on)
     }
   }