Browse Source

ref(event-details): Convert EventErrors to an FC and use react-query (#43710)

- Convert EventErrors component to an FC
- Use useQuery for the request it was making
- Rename from `errors.tsx` to `eventErrors.tsx`
- Remove re-export of type in the ErrorItem file, consumers can just
import from there
Malachi Willey 2 years ago
parent
commit
4a4917db72

+ 2 - 2
static/app/components/events/errorItem.tsx

@@ -12,7 +12,7 @@ import {JavascriptProcessingErrors} from 'sentry/constants/eventErrors';
 import {t, tct} from 'sentry/locale';
 import space from 'sentry/styles/space';
 
-type Error = {
+export type EventErrorData = {
   message: React.ReactNode;
   type: string;
   data?: {
@@ -33,7 +33,7 @@ const keyMapping = {
 };
 
 export type ErrorItemProps = {
-  error: Error;
+  error: EventErrorData;
   meta?: Record<any, any>;
 };
 

+ 0 - 206
static/app/components/events/errors.tsx

@@ -1,206 +0,0 @@
-import {Component} from 'react';
-import styled from '@emotion/styled';
-import * as Sentry from '@sentry/react';
-import isEqual from 'lodash/isEqual';
-import uniqWith from 'lodash/uniqWith';
-
-import {Client} from 'sentry/api';
-import {Alert} from 'sentry/components/alert';
-import {ErrorItem, ErrorItemProps} from 'sentry/components/events/errorItem';
-import List from 'sentry/components/list';
-import {JavascriptProcessingErrors} from 'sentry/constants/eventErrors';
-import {t, tn} from 'sentry/locale';
-import space from 'sentry/styles/space';
-import {Artifact, Organization, Project} from 'sentry/types';
-import {Event} from 'sentry/types/event';
-import withApi from 'sentry/utils/withApi';
-
-import {DataSection} from './styles';
-
-const MAX_ERRORS = 100;
-
-export type Error = ErrorItemProps['error'];
-
-type Props = {
-  api: Client;
-  event: Event;
-  orgSlug: Organization['slug'];
-  proGuardErrors: Array<Error>;
-  projectSlug: Project['slug'];
-};
-
-type State = {
-  releaseArtifacts?: Array<Artifact>;
-};
-
-class Errors extends Component<Props, State> {
-  state: State = {};
-
-  componentDidMount() {
-    this.checkSourceCodeErrors();
-  }
-
-  shouldComponentUpdate(nextProps: Props) {
-    return this.props.event.id !== nextProps.event.id;
-  }
-
-  componentDidUpdate(prevProps: Props) {
-    if (this.props.event.id !== prevProps.event.id) {
-      this.checkSourceCodeErrors();
-    }
-  }
-
-  async fetchReleaseArtifacts(query: string) {
-    const {api, orgSlug, event, projectSlug} = this.props;
-    const {release} = event;
-    const releaseVersion = release?.version;
-
-    if (!releaseVersion || !query) {
-      return;
-    }
-
-    try {
-      const releaseArtifacts = await api.requestPromise(
-        `/projects/${orgSlug}/${projectSlug}/releases/${encodeURIComponent(
-          releaseVersion
-        )}/files/?query=${query}`,
-        {
-          method: 'GET',
-        }
-      );
-
-      this.setState({releaseArtifacts});
-    } catch (error) {
-      Sentry.captureException(error);
-      // do nothing, the UI will not display extra error details
-    }
-  }
-
-  checkSourceCodeErrors() {
-    const {event} = this.props;
-    const {errors} = event;
-
-    const sourceCodeErrors = (errors ?? []).filter(
-      error => error.type === 'js_no_source' && error.data.url
-    );
-
-    if (!sourceCodeErrors.length) {
-      return;
-    }
-
-    const pathNames: Array<string> = [];
-
-    for (const sourceCodeError of sourceCodeErrors) {
-      const url = sourceCodeError.data.url;
-      if (url) {
-        const pathName = this.getURLPathname(url);
-
-        if (pathName) {
-          pathNames.push(encodeURIComponent(pathName));
-        }
-      }
-    }
-
-    this.fetchReleaseArtifacts(pathNames.join('&query='));
-  }
-
-  getURLPathname(url: string) {
-    try {
-      return new URL(url).pathname;
-    } catch {
-      return undefined;
-    }
-  }
-
-  render() {
-    const {event, proGuardErrors} = this.props;
-    const {releaseArtifacts} = this.state;
-    const {dist: eventDistribution, errors: eventErrors = [], _meta} = event;
-
-    // XXX: uniqWith returns unique errors and is not performant with large datasets
-    const otherErrors: Array<Error> =
-      eventErrors.length > MAX_ERRORS ? eventErrors : uniqWith(eventErrors, isEqual);
-
-    const errors = [...otherErrors, ...proGuardErrors];
-
-    return (
-      <StyledDataSection>
-        <StyledAlert
-          type="error"
-          showIcon
-          data-test-id="event-error-alert"
-          expand={[
-            <ErrorList
-              key="event-error-details"
-              data-test-id="event-error-details"
-              symbol="bullet"
-            >
-              {errors.map((error, errorIdx) => {
-                const data = error.data ?? {};
-                const meta = _meta?.errors?.[errorIdx];
-
-                if (
-                  error.type === JavascriptProcessingErrors.JS_MISSING_SOURCE &&
-                  data.url &&
-                  !!releaseArtifacts?.length
-                ) {
-                  const releaseArtifact = releaseArtifacts.find(releaseArt => {
-                    const pathname = data.url ? this.getURLPathname(data.url) : undefined;
-
-                    if (pathname) {
-                      return releaseArt.name.includes(pathname);
-                    }
-                    return false;
-                  });
-
-                  const releaseArtifactDistribution = releaseArtifact?.dist ?? null;
-
-                  // Neither event nor file have dist -> matching
-                  // Event has dist, file doesn’t -> not matching
-                  // File has dist, event doesn’t -> not matching
-                  // Both have dist, same value -> matching
-                  // Both have dist, different values -> not matching
-                  if (releaseArtifactDistribution !== eventDistribution) {
-                    error.message = t(
-                      'Source code was not found because the distribution did not match'
-                    );
-                    data['expected-distribution'] = eventDistribution;
-                    data['current-distribution'] = releaseArtifactDistribution;
-                  }
-                }
-
-                return <ErrorItem key={errorIdx} error={{...error, data}} meta={meta} />;
-              })}
-            </ErrorList>,
-          ]}
-        >
-          {tn(
-            'There was %s problem processing this event',
-            'There were %s problems processing this event',
-            errors.length
-          )}
-        </StyledAlert>
-      </StyledDataSection>
-    );
-  }
-}
-
-const StyledDataSection = styled(DataSection)`
-  border-top: none;
-
-  @media (min-width: ${p => p.theme.breakpoints.small}) {
-    padding-top: 0;
-  }
-`;
-
-const StyledAlert = styled(Alert)`
-  margin: ${space(0.5)} 0;
-`;
-
-const ErrorList = styled(List)`
-  li:last-child {
-    margin-bottom: 0;
-  }
-`;
-
-export const EventErrors = withApi(Errors);

+ 3 - 3
static/app/components/events/eventEntries.spec.tsx

@@ -1,14 +1,14 @@
 import {initializeData} from 'sentry-test/performance/initializePerformanceData';
 import {act, render, screen, userEvent, within} from 'sentry-test/reactTestingLibrary';
 
-import type {Error} from 'sentry/components/events/errors';
+import type {EventErrorData} from 'sentry/components/events/errorItem';
 import {EventEntries} from 'sentry/components/events/eventEntries';
 import {Group, IssueCategory} from 'sentry/types';
 import {EntryType, Event} from 'sentry/types/event';
 
 const {organization, project, router} = initializeData();
 
-async function renderComponent(event: Event, errors?: Array<Error>) {
+async function renderComponent(event: Event, errors?: Array<EventErrorData>) {
   render(
     <EventEntries
       organization={organization}
@@ -52,7 +52,7 @@ describe('EventEntries', function () {
 
   describe('EventError', function () {
     it('renders', async function () {
-      const errors: Array<Error> = [
+      const errors: Array<EventErrorData> = [
         {
           type: 'invalid_data',
           data: {

+ 3 - 3
static/app/components/events/eventEntries.tsx

@@ -7,6 +7,7 @@ import uniq from 'lodash/uniq';
 import {addErrorMessage} from 'sentry/actionCreators/indicator';
 import {CommitRow} from 'sentry/components/commitRow';
 import ErrorBoundary from 'sentry/components/errorBoundary';
+import type {EventErrorData} from 'sentry/components/events/errorItem';
 import ExternalLink from 'sentry/components/links/externalLink';
 import {t, tct} from 'sentry/locale';
 import space from 'sentry/styles/space';
@@ -35,11 +36,11 @@ import findBestThread from './interfaces/threads/threadSelector/findBestThread';
 import getThreadException from './interfaces/threads/threadSelector/getThreadException';
 import {EventContexts} from './contexts';
 import {EventDevice} from './device';
-import {Error, EventErrors} from './errors';
 import {EventAttachments} from './eventAttachments';
 import {EventCause} from './eventCause';
 import {EventDataSection} from './eventDataSection';
 import {EventEntry} from './eventEntry';
+import {EventErrors} from './eventErrors';
 import {EventEvidence} from './eventEvidence';
 import {EventExtraData} from './eventExtraData';
 import {EventSdk} from './eventSdk';
@@ -84,7 +85,7 @@ function hasThreadOrExceptionMinifiedFrameData(definedEvent: Event, bestThread?:
     : bestThread?.stacktrace?.frames?.find(frame => isDataMinified(frame.module)));
 }
 
-type ProGuardErrors = Array<Error>;
+type ProGuardErrors = Array<EventErrorData>;
 
 type Props = {
   location: Location;
@@ -326,7 +327,6 @@ const EventEntries = ({
       {hasErrors && !isLoading && (
         <EventErrors
           event={event}
-          orgSlug={orgSlug}
           projectSlug={projectSlug}
           proGuardErrors={proGuardErrors}
         />

+ 138 - 0
static/app/components/events/eventErrors.tsx

@@ -0,0 +1,138 @@
+import styled from '@emotion/styled';
+import isEqual from 'lodash/isEqual';
+import uniqWith from 'lodash/uniqWith';
+
+import {Alert} from 'sentry/components/alert';
+import {ErrorItem, EventErrorData} from 'sentry/components/events/errorItem';
+import List from 'sentry/components/list';
+import {JavascriptProcessingErrors} from 'sentry/constants/eventErrors';
+import {t, tn} from 'sentry/locale';
+import space from 'sentry/styles/space';
+import {Artifact, Project} from 'sentry/types';
+import {Event} from 'sentry/types/event';
+import {defined} from 'sentry/utils';
+import {useQuery} from 'sentry/utils/queryClient';
+import useOrganization from 'sentry/utils/useOrganization';
+
+import {DataSection} from './styles';
+
+const MAX_ERRORS = 100;
+
+type EventErrorsProps = {
+  event: Event;
+  proGuardErrors: Array<EventErrorData>;
+  projectSlug: Project['slug'];
+};
+
+const getURLPathname = (url: string) => {
+  try {
+    return new URL(url).pathname;
+  } catch {
+    return undefined;
+  }
+};
+
+export const EventErrors = ({event, proGuardErrors, projectSlug}: EventErrorsProps) => {
+  const organization = useOrganization();
+  const orgSlug = organization.slug;
+
+  const releaseVersion = event.release?.version;
+  const pathNames = (event.errors ?? [])
+    .filter(
+      error =>
+        error.type === 'js_no_source' && error.data.url && getURLPathname(error.data.url)
+    )
+    .map(sourceCodeError => getURLPathname(sourceCodeError.data.url));
+
+  const {data: releaseArtifacts} = useQuery<Artifact[]>(
+    [
+      `/projects/${orgSlug}/${projectSlug}/releases/${encodeURIComponent(
+        releaseVersion ?? ''
+      )}/files/`,
+      {query: {query: pathNames}},
+    ],
+    {staleTime: Infinity, enabled: pathNames.length > 0 && defined(releaseVersion)}
+  );
+
+  const {dist: eventDistribution, errors: eventErrors = [], _meta} = event;
+
+  // XXX: uniqWith returns unique errors and is not performant with large datasets
+  const otherErrors: Array<EventErrorData> =
+    eventErrors.length > MAX_ERRORS ? eventErrors : uniqWith(eventErrors, isEqual);
+
+  const errors = [...otherErrors, ...proGuardErrors];
+
+  return (
+    <StyledDataSection>
+      <StyledAlert
+        type="error"
+        showIcon
+        data-test-id="event-error-alert"
+        expand={
+          <ErrorList data-test-id="event-error-details" symbol="bullet">
+            {errors.map((error, errorIdx) => {
+              const data = error.data ?? {};
+              const meta = _meta?.errors?.[errorIdx];
+
+              if (
+                error.type === JavascriptProcessingErrors.JS_MISSING_SOURCE &&
+                data.url &&
+                !!releaseArtifacts?.length
+              ) {
+                const releaseArtifact = releaseArtifacts.find(releaseArt => {
+                  const pathname = data.url ? getURLPathname(data.url) : undefined;
+
+                  if (pathname) {
+                    return releaseArt.name.includes(pathname);
+                  }
+                  return false;
+                });
+
+                const releaseArtifactDistribution = releaseArtifact?.dist ?? null;
+
+                // Neither event nor file have dist -> matching
+                // Event has dist, file doesn’t -> not matching
+                // File has dist, event doesn’t -> not matching
+                // Both have dist, same value -> matching
+                // Both have dist, different values -> not matching
+                if (releaseArtifactDistribution !== eventDistribution) {
+                  error.message = t(
+                    'Source code was not found because the distribution did not match'
+                  );
+                  data['expected-distribution'] = eventDistribution;
+                  data['current-distribution'] = releaseArtifactDistribution;
+                }
+              }
+
+              return <ErrorItem key={errorIdx} error={{...error, data}} meta={meta} />;
+            })}
+          </ErrorList>
+        }
+      >
+        {tn(
+          'There was %s problem processing this event',
+          'There were %s problems processing this event',
+          errors.length
+        )}
+      </StyledAlert>
+    </StyledDataSection>
+  );
+};
+
+const StyledDataSection = styled(DataSection)`
+  border-top: none;
+
+  @media (min-width: ${p => p.theme.breakpoints.small}) {
+    padding-top: 0;
+  }
+`;
+
+const StyledAlert = styled(Alert)`
+  margin: ${space(0.5)} 0;
+`;
+
+const ErrorList = styled(List)`
+  li:last-child {
+    margin-bottom: 0;
+  }
+`;