feat(replays): Improve loading and error display for the Trace tab on Replay Details v2 (#47857)

Note: Updated after revert of
https://github.com/getsentry/sentry/pull/47704
The changed part is visible in
https://github.com/getsentry/sentry/pull/47857/commits/014b3836de6ba9ba241e56b0b800b0c71286680a

There are three UI stages that you'll see when loading the trace tab
inside replay.
1. Page is initializing, loading has not started yet (no spinner)
- we're waiting for the replayRecord, once we have that record we can
show the header
2. We're starting to fetch data (spinner)
- We're waiting for the first page of traceIds, and then some
traceDetails to arrive
3. One of a) no traces b) errors happened c) data is loading
incrementally
- Some trace specific endpoints have returned, and we can show results.

Screen shots of each:
| Example | Stage 1 - Page init | Stage 2 - Data fetch | Stage 3 -
Result |
| --- | --- | --- | --- |
| Result is no traces | <img width="1364" alt="preload"
src="https://user-images.githubusercontent.com/187460/233372694-f613cdce-9969-4928-acf8-5e6148321964.png">
| <img width="1364" alt="fetch started"
src="https://user-images.githubusercontent.com/187460/233372783-17e9afbc-8552-4c65-88d2-bf9e95c340b3.png">
| <img width="1364" alt="no traces"
src="https://user-images.githubusercontent.com/187460/233372830-f46e4ff3-693f-4113-914b-b441c07dd978.png">
|
| Results in errors | <img width="1364" alt="preload"
src="https://user-images.githubusercontent.com/187460/233372694-f613cdce-9969-4928-acf8-5e6148321964.png">
| <img width="1364" alt="fetch started"
src="https://user-images.githubusercontent.com/187460/233372783-17e9afbc-8552-4c65-88d2-bf9e95c340b3.png">
| <img width="1364" alt="with error"
src="https://user-images.githubusercontent.com/187460/233372877-032b7c5b-651d-4c9a-acc3-443a70777ca0.png">
|
| Results in seeing the trace! | <img width="1364" alt="preload"
src="https://user-images.githubusercontent.com/187460/233372694-f613cdce-9969-4928-acf8-5e6148321964.png">
| <img width="1364" alt="fetch started"
src="https://user-images.githubusercontent.com/187460/233372783-17e9afbc-8552-4c65-88d2-bf9e95c340b3.png">
| <img width="1364" alt="done"
src="https://user-images.githubusercontent.com/187460/233372889-65ed8dbf-08cc-4d70-9c72-9a5ec7582a50.png">
|

This also replaces `location` as a dependency in the hook with an empty
object. We don't rely on the url to setup our pagination/sort/etc for
this component, but having that var as a dependency meant that the
memoized `useCallback` results would change too often. Without that
dependency we don't change object identity anymore, and re-fetch the
data. Data is scoped to each instance of the context provider.

Fixes https://github.com/getsentry/team-replay/issues/60

static/app/views/replays/detail/emptyState.tsx

@@ -0,0 +1,14 @@
+import styled from '@emotion/styled';
+
+import EmptyStateWarning from 'sentry/components/emptyStateWarning';
+
+const StyledEmptyStateWarning = styled(EmptyStateWarning)`
+  height: 100%;
+  width: 100%;
+  display: flex;
+  flex-direction: column;
+  align-items: center;
+  justify-content: center;
+`;
+
+export default StyledEmptyStateWarning;

static/app/views/replays/detail/noRowRenderer.tsx

@@ -1,10 +1,9 @@
 import {ReactNode} from 'react';
-import styled from '@emotion/styled';
 
 import {Button} from 'sentry/components/button';
-import EmptyStateWarning from 'sentry/components/emptyStateWarning';
 import {IconClose} from 'sentry/icons';
 import {t} from 'sentry/locale';
+import EmptyState from 'sentry/views/replays/detail/emptyState';
 
 type Props = {
   children: ReactNode;
@@ -14,11 +13,11 @@ type Props = {
 
 function NoRowRenderer({children, unfilteredItems, clearSearchTerm}: Props) {
   return unfilteredItems.length === 0 ? (
-    <StyledEmptyStateWarning>
+    <EmptyState>
       <p>{children}</p>
-    </StyledEmptyStateWarning>
+    </EmptyState>
   ) : (
-    <StyledEmptyStateWarning>
+    <EmptyState>
       <p>{t('No results found')}</p>
       <Button
         icon={<IconClose color="gray500" size="sm" isCircled />}
@@ -27,17 +26,8 @@ function NoRowRenderer({children, unfilteredItems, clearSearchTerm}: Props) {
       >
         {t('Clear filters')}
       </Button>
-    </StyledEmptyStateWarning>
+    </EmptyState>
   );
 }
 
-const StyledEmptyStateWarning = styled(EmptyStateWarning)`
-  height: 100%;
-  width: 100%;
-  display: flex;
-  flex-direction: column;
-  align-items: center;
-  justify-content: center;
-`;
-
 export default NoRowRenderer;

static/app/views/replays/detail/trace/replayTransactionContext.tsx

@@ -21,7 +21,6 @@ import {
   makeEventView,
 } from 'sentry/utils/performance/quickTrace/utils';
 import useApi from 'sentry/utils/useApi';
-import {useLocation} from 'sentry/utils/useLocation';
 import useOrganization from 'sentry/utils/useOrganization';
 import type {ReplayRecord} from 'sentry/views/replays/types';
 
@@ -34,6 +33,7 @@ type InternalState = {
   detailsErrors: Error[];
   detailsRequests: number;
   detailsResponses: number;
+  didInit: boolean;
   indexComplete: boolean;
   indexError: undefined | Error;
   isFetching: boolean;
@@ -41,6 +41,7 @@ type InternalState = {
 };
 
 type ExternalState = {
+  didInit: boolean;
   errors: Error[];
   isFetching: boolean;
   traces: undefined | TraceFullDetailed[];
@@ -50,6 +51,7 @@ const INITIAL_STATE: InternalState = {
   detailsErrors: [],
   detailsRequests: 0,
   detailsResponses: 0,
+  didInit: false,
   indexComplete: true,
   indexError: undefined,
   isFetching: false,
@@ -65,12 +67,11 @@ type TxnContextProps = {
 const TxnContext = createContext<TxnContextProps>({
   eventView: null,
   fetchTransactionData: () => {},
-  state: {errors: [], isFetching: false, traces: []},
+  state: {didInit: false, errors: [], isFetching: false, traces: []},
 });
 
 function ReplayTransactionContext({children, replayRecord}: Options) {
   const api = useApi();
-  const location = useLocation();
   const organization = useOrganization();
 
   const [state, setState] = useState<InternalState>(INITIAL_STATE);
@@ -99,13 +100,13 @@ function ReplayTransactionContext({children, replayRecord}: Options) {
     });
   }, [replayRecord]);
 
-  const tracePayload = useMemo(() => {
+  const singleTracePayload = useMemo(() => {
     const start = getUtcDateString(replayRecord?.started_at.getTime());
     const end = getUtcDateString(replayRecord?.finished_at.getTime());
 
     const traceEventView = makeEventView({start, end});
-    return getTraceRequestPayload({eventView: traceEventView, location});
-  }, [replayRecord, location]);
+    return getTraceRequestPayload({eventView: traceEventView, location: {} as Location});
+  }, [replayRecord]);
 
   const fetchSingleTraceData = useCallback(
     async traceId => {
@@ -113,7 +114,7 @@ function ReplayTransactionContext({children, replayRecord}: Options) {
         const [trace, , _traceResp] = await doDiscoverQuery(
           api,
           `/organizations/${orgSlug}/events-trace/${traceId}/`,
-          tracePayload
+          singleTracePayload
         );
 
         setState(prev => ({
@@ -130,7 +131,7 @@ function ReplayTransactionContext({children, replayRecord}: Options) {
         }));
       }
     },
-    [api, orgSlug, tracePayload]
+    [api, orgSlug, singleTracePayload]
   );
 
   const fetchTransactionData = useCallback(async () => {
@@ -144,6 +145,7 @@ function ReplayTransactionContext({children, replayRecord}: Options) {
       detailsErrors: [],
       detailsRequests: 0,
       detailsResponses: 0,
+      didInit: true,
       indexComplete: false,
       indexError: undefined,
       isFetching: true,
@@ -196,21 +198,23 @@ function ReplayTransactionContext({children, replayRecord}: Options) {
 
         const pageLinks = listResp?.getResponseHeader('Link') ?? null;
         cursor = parseLinkHeader(pageLinks)?.next;
+        const indexComplete = !cursor.results;
+        setState(prev => ({...prev, indexComplete} as InternalState));
       } catch (indexError) {
-        setState(prev => ({...prev, indexError} as InternalState));
+        setState(prev => ({...prev, indexError, indexComplete: true} as InternalState));
         cursor = {cursor: '', results: false, href: ''} as ParsedHeader;
       }
     }
-
-    setState(prev => ({...prev, indexComplete: true} as InternalState));
   }, [api, fetchSingleTraceData, listEventView, orgSlug, replayRecord]);
 
+  const externalState = useMemo(() => internalToExternalState(state), [state]);
+
   return (
     <TxnContext.Provider
       value={{
         eventView: listEventView,
         fetchTransactionData,
-        state: internalToExternalState(state),
+        state: externalState,
       }}
     >
       {children}
@@ -219,9 +223,9 @@ function ReplayTransactionContext({children, replayRecord}: Options) {
 }
 
 function internalToExternalState({
-  detailsErrors,
   detailsRequests,
   detailsResponses,
+  didInit,
   indexComplete,
   indexError,
   traces,
@@ -229,7 +233,8 @@ function internalToExternalState({
   const isComplete = indexComplete && detailsRequests === detailsResponses;
 
   return {
-    errors: indexError ? [indexError] : detailsErrors,
+    didInit,
+    errors: indexError ? [indexError] : [], // Ignoring detailsErrors for now
     isFetching: !isComplete,
     traces,
   };
@@ -238,9 +243,13 @@ function internalToExternalState({
 export default ReplayTransactionContext;
 
 export const useFetchTransactions = () => {
-  const {fetchTransactionData} = useContext(TxnContext);
+  const {fetchTransactionData, state} = useContext(TxnContext);
 
-  useEffect(fetchTransactionData, [fetchTransactionData]);
+  useEffect(() => {
+    if (!state.isFetching && state.traces === undefined) {
+      fetchTransactionData();
+    }
+  }, [fetchTransactionData, state]);
 };
 
 export const useTransactionData = () => {

static/app/views/replays/detail/trace/trace.spec.tsx

@@ -0,0 +1,90 @@
+import {render, screen} from 'sentry-test/reactTestingLibrary';
+
+import {TraceFullDetailed} from 'sentry/utils/performance/quickTrace/types';
+import {useTransactionData} from 'sentry/views/replays/detail/trace/replayTransactionContext';
+import Trace from 'sentry/views/replays/detail/trace/trace';
+
+jest.mock('sentry/views/replays/detail/trace/replayTransactionContext');
+
+const mockUseTransactionData = useTransactionData as jest.MockedFunction<
+  typeof useTransactionData
+>;
+
+const mockTraceFullDetailed = {} as TraceFullDetailed;
+
+function setMockTransactionState({
+  didInit = false,
+  errors = [],
+  isFetching = false,
+  traces = undefined,
+}: Partial<ReturnType<typeof useTransactionData>['state']>) {
+  const eventView = null;
+  mockUseTransactionData.mockReturnValue({
+    state: {didInit, errors, isFetching, traces},
+    eventView,
+  });
+}
+
+describe('trace', () => {
+  beforeEach(() => {
+    mockUseTransactionData.mockReset();
+  });
+
+  it('should show the blank screen if there is no replayRecord', () => {
+    setMockTransactionState({});
+
+    render(<Trace replayRecord={undefined} />);
+
+    const placeholder = screen.getByTestId('loading-placeholder');
+    expect(placeholder).toBeInTheDocument();
+    expect(placeholder).toBeEmptyDOMElement();
+  });
+
+  it('should show the blank screen if the hook has not initialized yet', () => {
+    setMockTransactionState({});
+
+    render(<Trace replayRecord={TestStubs.ReplayRecord()} />);
+
+    const placeholder = screen.getByTestId('loading-placeholder');
+    expect(placeholder).toBeInTheDocument();
+    expect(placeholder).toBeEmptyDOMElement();
+  });
+
+  it('should show a loading spinner if the hook is fetching, but there are no traces returned yet', () => {
+    setMockTransactionState({didInit: true, isFetching: true});
+
+    render(<Trace replayRecord={TestStubs.ReplayRecord()} />);
+
+    const placeholder = screen.getByTestId('loading-placeholder');
+    expect(placeholder).toBeInTheDocument();
+    expect(placeholder).not.toBeEmptyDOMElement();
+  });
+
+  it('should show errors if there are any', () => {
+    setMockTransactionState({
+      didInit: true,
+      isFetching: true,
+      traces: [mockTraceFullDetailed],
+      errors: [new Error('Something went wrong')],
+    });
+
+    render(<Trace replayRecord={TestStubs.ReplayRecord()} />);
+
+    const emptyState = screen.getByTestId('empty-state');
+    expect(emptyState).toHaveTextContent('Unable to retrieve traces');
+  });
+
+  it('should show a `no traces found` message if fetching is done, and there are no traces returned', () => {
+    setMockTransactionState({
+      didInit: true,
+      isFetching: false,
+      traces: [],
+      errors: [],
+    });
+
+    render(<Trace replayRecord={TestStubs.ReplayRecord()} />);
+
+    const emptyState = screen.getByTestId('empty-state');
+    expect(emptyState).toHaveTextContent('No traces found');
+  });
+});

static/app/views/replays/detail/trace/trace.tsx

@@ -2,9 +2,12 @@ import styled from '@emotion/styled';
 
 import Loading from 'sentry/components/loadingIndicator';
 import Placeholder from 'sentry/components/placeholder';
+import {IconSad} from 'sentry/icons';
+import {t} from 'sentry/locale';
 import {useLocation} from 'sentry/utils/useLocation';
 import useOrganization from 'sentry/utils/useOrganization';
 import TraceView from 'sentry/views/performance/traceDetails/traceView';
+import EmptyState from 'sentry/views/replays/detail/emptyState';
 import FluidHeight from 'sentry/views/replays/detail/layout/fluidHeight';
 import {
   useFetchTransactions,
@@ -19,23 +22,49 @@ type Props = {
 function Trace({replayRecord}: Props) {
   const location = useLocation();
   const organization = useOrganization();
-  const {state, eventView} = useTransactionData();
+  const {
+    state: {didInit, errors, isFetching, traces},
+    eventView,
+  } = useTransactionData();
 
   useFetchTransactions();
 
-  if (!replayRecord || !state.traces?.length) {
+  if (!replayRecord || !didInit || (isFetching && !traces?.length)) {
+    // Show the blank screen until we start fetching, thats when you get a spinner
     return (
       <StyledPlaceholder height="100%">
-        {state.isFetching && <Loading />}
+        {isFetching ? <Loading /> : null}
       </StyledPlaceholder>
     );
   }
 
+  if (errors.length) {
+    // Same style as <EmptyStateWarning>
+    return (
+      <BorderedSection>
+        <EmptyState withIcon={false}>
+          <IconSad legacySize="54px" />
+          <p>{t('Unable to retrieve traces')}</p>
+        </EmptyState>
+      </BorderedSection>
+    );
+  }
+
+  if (!traces?.length) {
+    return (
+      <BorderedSection>
+        <EmptyState>
+          <p>{t('No traces found')}</p>
+        </EmptyState>
+      </BorderedSection>
+    );
+  }
+
   return (
     <FluidHeight>
       <TraceView
         meta={null}
-        traces={state.traces ?? null}
+        traces={traces ?? null}
         location={location}
         organization={organization}
         traceEventView={eventView!}
@@ -45,9 +74,16 @@ function Trace({replayRecord}: Props) {
   );
 }
 
+// This has the gray background, to match other loaders on Replay Details
 const StyledPlaceholder = styled(Placeholder)`
   border: 1px solid ${p => p.theme.border};
   border-radius: ${p => p.theme.borderRadius};
 `;
 
+// White background, to match the loaded component
+const BorderedSection = styled(FluidHeight)`
+  border: 1px solid ${p => p.theme.border};
+  border-radius: ${p => p.theme.borderRadius};
+`;
+
 export default Trace;