Browse Source

Revert "feat(replays): Improve loading and error display for the Trace tab on Replay Details (#47704)"

This reverts commit fc8500f7f4f256b304289f402f75d05a3a5a326c.

Co-authored-by: eliashussary <7349258+eliashussary@users.noreply.github.com>
getsentry-bot 1 year ago
parent
commit
bdd5f93bd5

+ 0 - 14
static/app/views/replays/detail/emptyState.tsx

@@ -1,14 +0,0 @@
-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;

+ 15 - 5
static/app/views/replays/detail/noRowRenderer.tsx

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

+ 14 - 24
static/app/views/replays/detail/trace/replayTransactionContext.tsx

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

+ 4 - 40
static/app/views/replays/detail/trace/trace.tsx

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