Browse Source

fix(perf): Fix error messages coming from Gen. Query (#33341)

The error messages don't always return in the same format, so this is actually an older bug. This will check whether the error is being returned as a string or an object {code, message, extra} with room to expand this in the future if we find more cases.
Kev 2 years ago
parent
commit
d244b2fa92

+ 39 - 5
static/app/utils/discover/genericDiscoverQuery.tsx

@@ -14,8 +14,14 @@ import useOrganization from 'sentry/utils/useOrganization';
 
 export class QueryError {
   message: string;
-  constructor(errorMessage: string) {
+  private originalError: any; // For debugging in case parseError picks a value that doesn't make sense.
+  constructor(errorMessage: string, originalError?: any) {
     this.message = errorMessage;
+    this.originalError = originalError;
+  }
+
+  getOriginalError() {
+    return this.originalError;
   }
 }
 
@@ -112,6 +118,10 @@ type ComponentProps<T, P> = {
    * Allows components to modify the payload before it is set.
    */
   getRequestPayload?: (props: Props<T, P>) => any;
+  /**
+   * An external hook to parse errors in case there are differences for a specific api.
+   */
+  parseError?: (error: any) => QueryError | null;
   /**
    * An external hook in addition to the event view check to check if data should be refetched
    */
@@ -195,6 +205,32 @@ class _GenericDiscoverQuery<T, P> extends React.Component<Props<T, P>, State<T>>
     );
   };
 
+  /**
+   * The error type isn't consistent across APIs. We see detail as just string some times, other times as an object.
+   */
+  _parseError = (error: any): QueryError | null => {
+    if (this.props.parseError) {
+      return this.props.parseError(error);
+    }
+
+    if (!error) {
+      return null;
+    }
+
+    const detail = error.responseJSON?.detail;
+    if (typeof detail === 'string') {
+      return new QueryError(detail, error);
+    }
+
+    const message = detail?.message;
+    if (typeof message === 'string') {
+      return new QueryError(message, error);
+    }
+
+    const unknownError = new QueryError(t('An unknown error occurred.'), error);
+    return unknownError;
+  };
+
   fetchData = async () => {
     const {api, beforeFetch, afterFetch, didFetch, eventView, orgSlug, route, setError} =
       this.props;
@@ -234,9 +270,7 @@ class _GenericDiscoverQuery<T, P> extends React.Component<Props<T, P>, State<T>>
         tableData,
       }));
     } catch (err) {
-      const error = new QueryError(
-        err?.responseJSON?.detail || t('An unknown error occurred.')
-      );
+      const error = this._parseError(err);
       this.setState({
         isLoading: false,
         tableFetchID: undefined,
@@ -244,7 +278,7 @@ class _GenericDiscoverQuery<T, P> extends React.Component<Props<T, P>, State<T>>
         tableData: null,
       });
       if (setError) {
-        setError(error);
+        setError(error ?? undefined);
       }
     }
   };

+ 66 - 0
tests/js/spec/utils/discover/discoverQuery.spec.jsx

@@ -91,4 +91,70 @@ describe('DiscoverQuery', function () {
       })
     );
   });
+
+  it('parses string errors correctly', async function () {
+    MockApiClient.addMockResponse({
+      url: '/organizations/test-org/eventsv2/',
+      body: {
+        detail: 'Error Message',
+      },
+      statusCode: 400,
+    });
+
+    let errorValue;
+    render(
+      <DiscoverQuery
+        orgSlug="test-org"
+        api={api}
+        location={location}
+        eventView={eventView}
+        setError={e => (errorValue = e)}
+      >
+        {({isLoading}) => {
+          if (isLoading) {
+            return 'loading';
+          }
+          return null;
+        }}
+      </DiscoverQuery>
+    );
+    await tick();
+
+    expect(errorValue.message).toEqual('Error Message');
+  });
+
+  it('parses object errors correctly', async function () {
+    MockApiClient.addMockResponse({
+      url: '/organizations/test-org/eventsv2/',
+      body: {
+        detail: {
+          code: '?',
+          message: 'Object Error',
+          extra: {},
+        },
+      },
+      statusCode: 400,
+    });
+
+    let errorValue;
+    render(
+      <DiscoverQuery
+        orgSlug="test-org"
+        api={api}
+        location={location}
+        eventView={eventView}
+        setError={e => (errorValue = e)}
+      >
+        {({isLoading}) => {
+          if (isLoading) {
+            return 'loading';
+          }
+          return null;
+        }}
+      </DiscoverQuery>
+    );
+    await tick();
+
+    expect(errorValue.message).toEqual('Object Error');
+  });
 });