Repository: getsentry/sentry
Pull request: ref(event-details): Convert EventErrors to an FC and use react-query
I'm in the process of breaking up EventEntries and making having fewer class components will make that goal a bit easier.

- 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

Problem statement:
Break up the `EventEntries` component
The `EventEntries` component is used in issue details, discover, performance, shared issues. Now that issues are becoming more differentiated from the event details, it no longer makes sense to use it for all these disparate use cases.

This work will enable the issues UI to be more flexible and diverge from the default event UI without adding many extra props and conditionals that will make the code less maintainable.

Hints from issue discussion:

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

Patch to review:
diff --git a/static/app/components/events/errorItem.tsx b/static/app/components/events/errorItem.tsx
index f139a8f6769465..7ebe56a444bca1 100644
--- a/static/app/components/events/errorItem.tsx
+++ b/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 Error = {
   message: React.ReactNode;
   type: string;
   data?: {
diff --git a/static/app/components/events/errors.tsx b/static/app/components/events/errors.tsx
deleted file mode 100644
index 85b9d40cde0133..00000000000000
--- a/static/app/components/events/errors.tsx
+++ /dev/null
@@ -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);
diff --git a/static/app/components/events/eventEntries.spec.tsx b/static/app/components/events/eventEntries.spec.tsx
index 33a5b4751db8ae..bc7bc7f5bb1c8a 100644
--- a/static/app/components/events/eventEntries.spec.tsx
+++ b/static/app/components/events/eventEntries.spec.tsx
@@ -1,7 +1,7 @@
 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 {Error} 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';
diff --git a/static/app/components/events/eventEntries.tsx b/static/app/components/events/eventEntries.tsx
index 99cadf72bd7d7e..eb28e9ab5fe883 100644
--- a/static/app/components/events/eventEntries.tsx
+++ b/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 {Error} 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';
@@ -326,7 +327,6 @@ const EventEntries = ({
       {hasErrors && !isLoading && (
         <EventErrors
           event={event}
-          orgSlug={orgSlug}
           projectSlug={projectSlug}
           proGuardErrors={proGuardErrors}
         />
diff --git a/static/app/components/events/eventErrors.tsx b/static/app/components/events/eventErrors.tsx
new file mode 100644
index 00000000000000..d910bc5636aeb0
--- /dev/null
+++ b/static/app/components/events/eventErrors.tsx
@@ -0,0 +1,152 @@
+import styled from '@emotion/styled';
+import isEqual from 'lodash/isEqual';
+import uniqWith from 'lodash/uniqWith';
+
+import {Alert} from 'sentry/components/alert';
+import {Error, ErrorItem} 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<Error>;
+  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 sourceCodeErrors = (event.errors ?? []).filter(
+    error => error.type === 'js_no_source' && error.data.url
+  );
+
+  const pathNames: Array<string> = [];
+
+  for (const sourceCodeError of sourceCodeErrors) {
+    const url = sourceCodeError.data.url;
+    if (url) {
+      const pathName = getURLPathname(url);
+
+      if (pathName) {
+        pathNames.push(pathName);
+      }
+    }
+  }
+
+  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<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 ? 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;
+  }
+`;


Code review comments:
diff_hunk: @@ -0,0 +1,152 @@
+import styled from '@emotion/styled';
+import isEqual from 'lodash/isEqual';
+import uniqWith from 'lodash/uniqWith';
+
+import {Alert} from 'sentry/components/alert';
+import {Error, ErrorItem} 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<Error>;
+  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 sourceCodeErrors = (event.errors ?? []).filter(
+    error => error.type === 'js_no_source' && error.data.url
+  );
+
+  const pathNames: Array<string> = [];
+
+  for (const sourceCodeError of sourceCodeErrors) {
+    const url = sourceCodeError.data.url;
+    if (url) {
path: static/app/components/events/eventErrors.tsx
text: @user1:
probably don't need to check for url again since you've filtered by it on line 41

diff_hunk: @@ -0,0 +1,152 @@
+import styled from '@emotion/styled';
+import isEqual from 'lodash/isEqual';
+import uniqWith from 'lodash/uniqWith';
+
+import {Alert} from 'sentry/components/alert';
+import {Error, ErrorItem} 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<Error>;
+  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 sourceCodeErrors = (event.errors ?? []).filter(
+    error => error.type === 'js_no_source' && error.data.url
+  );
+
+  const pathNames: Array<string> = [];
+
+  for (const sourceCodeError of sourceCodeErrors) {
path: static/app/components/events/eventErrors.tsx
text: @user1:
might just filter and map the pathnames unless you need part of it later

diff_hunk: @@ -0,0 +1,152 @@
+import styled from '@emotion/styled';
+import isEqual from 'lodash/isEqual';
+import uniqWith from 'lodash/uniqWith';
+
+import {Alert} from 'sentry/components/alert';
+import {Error, ErrorItem} 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<Error>;
+  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 sourceCodeErrors = (event.errors ?? []).filter(
+    error => error.type === 'js_no_source' && error.data.url
+  );
+
+  const pathNames: Array<string> = [];
+
+  for (const sourceCodeError of sourceCodeErrors) {
+    const url = sourceCodeError.data.url;
+    if (url) {
+      const pathName = getURLPathname(url);
+
+      if (pathName) {
+        pathNames.push(pathName);
+      }
+    }
+  }
+
+  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<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={[
path: static/app/components/events/eventErrors.tsx
text: @user1:
does this need to be an array?

diff_hunk: @@ -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 Error = {
path: static/app/components/events/errorItem.tsx
text: @user1:
can we rename this for export

@user1:
Error is the name of a global