Browse Source

ref(dashboards): Clean up `updateDashboard` error handling (#49116)

This fixes a problem I came across will working on updating the way we handle frontend API errors, to wit: `updateDashboard` handles errors thrown by the API client's `requestPromise` method, so code calling it doesn't need to do the same. This therefore removes a redundant `try-catch` surrounding an `updateDashboard` call, and adds comments to the places where we're correctly passing a no-op as the second argument to `updateDashboard(...).then()`. It also adds a comment explaining why success handling and error handling aren't done in the same place.
Katie Byers 1 year ago
parent
commit
03b66ecc25

+ 3 - 0
static/app/actionCreators/dashboards.tsx

@@ -145,6 +145,9 @@ export function updateDashboard(
     }
   );
 
+  // We let the callers of `updateDashboard` handle adding a success message, so
+  // that it can be more specific than just "Dashboard updated," but do the
+  // error-handling here, since it doesn't depend on the caller's context
   promise.catch(response => {
     const errorResponse = response?.responseJSON ?? null;
 

+ 8 - 15
static/app/components/modals/widgetBuilder/addToDashboardModal.tsx

@@ -9,7 +9,7 @@ import {
   fetchDashboards,
   updateDashboard,
 } from 'sentry/actionCreators/dashboards';
-import {addErrorMessage, addSuccessMessage} from 'sentry/actionCreators/indicator';
+import {addSuccessMessage} from 'sentry/actionCreators/indicator';
 import {ModalRenderProps} from 'sentry/actionCreators/modal';
 import {Button} from 'sentry/components/button';
 import ButtonBar from 'sentry/components/buttonBar';
@@ -17,7 +17,6 @@ import SelectControl from 'sentry/components/forms/controls/selectControl';
 import {t, tct} from 'sentry/locale';
 import {space} from 'sentry/styles/space';
 import {DateString, Organization, PageFilters, SelectValue} from 'sentry/types';
-import getXhrErrorResponseHandler from 'sentry/utils/handleXhrErrorResponse';
 import {MetricsCardinalityProvider} from 'sentry/utils/performance/contexts/metricsCardinality';
 import {MEPSettingProvider} from 'sentry/utils/performance/contexts/metricsEnhancedSetting';
 import useApi from 'sentry/utils/useApi';
@@ -160,21 +159,15 @@ function AddToDashboardModal({
       queries: [{...query, orderby}],
     };
 
-    try {
-      const newDashboard = {
-        ...selectedDashboard,
-        widgets: [...selectedDashboard.widgets, newWidget],
-      };
+    const newDashboard = {
+      ...selectedDashboard,
+      widgets: [...selectedDashboard.widgets, newWidget],
+    };
 
-      await updateDashboard(api, organization.slug, newDashboard);
+    await updateDashboard(api, organization.slug, newDashboard);
 
-      closeModal();
-      addSuccessMessage(t('Successfully added widget to dashboard'));
-    } catch (e) {
-      const errorMessage = t('Unable to add widget to dashboard');
-      getXhrErrorResponseHandler(errorMessage)(e);
-      addErrorMessage(errorMessage);
-    }
+    closeModal();
+    addSuccessMessage(t('Successfully added widget to dashboard'));
   }
 
   const canSubmit = selectedDashboardId !== null;

+ 3 - 0
static/app/views/dashboards/detail.tsx

@@ -458,6 +458,7 @@ class DashboardDetail extends Component<Props, State> {
           return;
         }
       },
+      // `updateDashboard` does its own error handling
       () => undefined
     );
   };
@@ -578,6 +579,7 @@ class DashboardDetail extends Component<Props, State> {
                 return;
               }
             },
+            // `updateDashboard` does its own error handling
             () => undefined
           );
 
@@ -885,6 +887,7 @@ class DashboardDetail extends Component<Props, State> {
                                     })
                                   );
                                 },
+                                // `updateDashboard` does its own error handling
                                 () => undefined
                               );
                             }}