Skip to content

Commit e80eb8a

Browse files
authored
fix(replay): prevent dup initial summary requests and ref to use usetimeout (#99182)
Right now there's a chance we'll send additional start summary POST requests after the first finishes. Only one of these should be made per page load (segment count won't update until hard reload). closes [REPLAY-670: frontend should prevent infinite polling](https://linear.app/getsentry/issue/REPLAY-670/frontend-should-prevent-infinite-polling) Updates to polling timeout to useTimeout and simplifies shouldPoll (removing dependency on startSummaryRequestPending)
1 parent bdf7b45 commit e80eb8a

File tree

2 files changed

+43
-78
lines changed

2 files changed

+43
-78
lines changed

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

Lines changed: 2 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -22,10 +22,7 @@ import useOrganization from 'sentry/utils/useOrganization';
2222
import useProjectFromId from 'sentry/utils/useProjectFromId';
2323
import {ChapterList} from 'sentry/views/replays/detail/ai/chapterList';
2424
import {useReplaySummaryContext} from 'sentry/views/replays/detail/ai/replaySummaryContext';
25-
import {
26-
NO_REPLAY_SUMMARY_MESSAGES,
27-
ReplaySummaryStatus,
28-
} from 'sentry/views/replays/detail/ai/utils';
25+
import {NO_REPLAY_SUMMARY_MESSAGES} from 'sentry/views/replays/detail/ai/utils';
2926
import TabItemContainer from 'sentry/views/replays/detail/tabItemContainer';
3027

3128
export default function Ai() {
@@ -141,14 +138,7 @@ export default function Ai() {
141138
);
142139
}
143140

144-
// checking this prevents initial flicker
145-
const summaryNotComplete =
146-
summaryData?.status &&
147-
[ReplaySummaryStatus.NOT_STARTED, ReplaySummaryStatus.PROCESSING].includes(
148-
summaryData?.status
149-
);
150-
151-
if (isSummaryPending || summaryNotComplete) {
141+
if (isSummaryPending) {
152142
return (
153143
<Wrapper data-test-id="replay-details-ai-summary-tab">
154144
<LoadingContainer>

static/app/views/replays/detail/ai/useFetchReplaySummary.tsx

Lines changed: 41 additions & 66 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import type ReplayReader from 'sentry/utils/replays/replayReader';
66
import useApi from 'sentry/utils/useApi';
77
import useOrganization from 'sentry/utils/useOrganization';
88
import useProjectFromId from 'sentry/utils/useProjectFromId';
9+
import useTimeout from 'sentry/utils/useTimeout';
910
import {
1011
ReplaySummaryStatus,
1112
ReplaySummaryTemp,
@@ -22,7 +23,7 @@ export interface UseFetchReplaySummaryResult {
2223
*/
2324
isError: boolean;
2425
/**
25-
* Whether the summary is still processing after the last start request.
26+
* Whether the summary is still processing after the last start request. Does not account for timeouts.
2627
*/
2728
isPending: boolean;
2829
/**
@@ -42,35 +43,26 @@ export interface UseFetchReplaySummaryResult {
4243

4344
const shouldPoll = (
4445
summaryData: SummaryResponse | undefined,
45-
isStartSummaryRequestPending: boolean,
46+
startRequestFailed: boolean,
4647
didTimeout: boolean
4748
) => {
48-
// If timeout occurred, stop polling regardless of status
49-
if (didTimeout) {
49+
if (startRequestFailed || didTimeout) {
5050
return false;
5151
}
5252

5353
if (!summaryData) {
54-
// No data yet - poll if we've started a request
55-
return isStartSummaryRequestPending;
54+
return true;
5655
}
5756

5857
switch (summaryData.status) {
5958
case ReplaySummaryStatus.NOT_STARTED:
60-
// Not started - poll if we've started a request
61-
return isStartSummaryRequestPending;
62-
6359
case ReplaySummaryStatus.PROCESSING:
64-
// Currently processing - poll
6560
return true;
6661

62+
// Final states
6763
case ReplaySummaryStatus.COMPLETED:
6864
case ReplaySummaryStatus.ERROR:
69-
// Final states - no need to poll
70-
return false;
71-
7265
default:
73-
// Unknown status - don't poll
7466
return false;
7567
}
7668
};
@@ -101,22 +93,15 @@ export function useFetchReplaySummary(
10193
// summary data query and 2) we have the stale version of the summary data. The consuming
10294
// component will briefly show a completed state before the summary data query updates.
10395
const startSummaryRequestTime = useRef<number>(0);
96+
const hasMadeStartRequest = useRef<boolean>(false);
10497

105-
const pollingTimeoutRef = useRef<number | null>(null);
106-
const clearPollingTimeout = () => {
107-
if (pollingTimeoutRef.current) {
108-
clearTimeout(pollingTimeoutRef.current);
109-
pollingTimeoutRef.current = null;
110-
}
111-
};
11298
const [didTimeout, setDidTimeout] = useState(false);
113-
114-
// Cleanup polling timeout on unmount
115-
useEffect(() => {
116-
return () => {
117-
clearPollingTimeout();
118-
};
119-
}, []);
99+
const {start: startPollingTimeout, cancel: cancelPollingTimeout} = useTimeout({
100+
timeMs: POLL_TIMEOUT_MS,
101+
onTimeout: () => {
102+
setDidTimeout(true);
103+
},
104+
});
120105

121106
const {
122107
mutate: startSummaryRequestMutate,
@@ -157,15 +142,12 @@ export function useFetchReplaySummary(
157142
return;
158143
}
159144
startSummaryRequestMutate();
145+
hasMadeStartRequest.current = true;
160146

161-
// Clear timeout, if any, and start a new one.
147+
// Start a new timeout.
162148
setDidTimeout(false);
163-
clearPollingTimeout();
164-
pollingTimeoutRef.current = window.setTimeout(() => {
165-
setDidTimeout(true);
166-
clearPollingTimeout();
167-
}, POLL_TIMEOUT_MS);
168-
}, [options?.enabled, startSummaryRequestMutate]);
149+
startPollingTimeout();
150+
}, [options?.enabled, startSummaryRequestMutate, startPollingTimeout]);
169151

170152
const {
171153
data: summaryData,
@@ -178,7 +160,7 @@ export function useFetchReplaySummary(
178160
staleTime: 0,
179161
retry: false,
180162
refetchInterval: query => {
181-
if (shouldPoll(query.state.data?.[0], isStartSummaryRequestPending, didTimeout)) {
163+
if (shouldPoll(query.state.data?.[0], isStartSummaryRequestError, didTimeout)) {
182164
return POLL_INTERVAL_MS;
183165
}
184166
return false;
@@ -188,49 +170,42 @@ export function useFetchReplaySummary(
188170
}
189171
);
190172

173+
// Auto-start logic. Triggered at most once per page load.
174+
const segmentsIncreased =
175+
summaryData?.num_segments !== null &&
176+
summaryData?.num_segments !== undefined &&
177+
segmentCount > summaryData.num_segments;
178+
179+
useEffect(() => {
180+
if (
181+
!hasMadeStartRequest.current &&
182+
(segmentsIncreased || summaryData?.status === ReplaySummaryStatus.NOT_STARTED)
183+
) {
184+
startSummaryRequest();
185+
}
186+
}, [segmentsIncreased, startSummaryRequest, summaryData?.status]);
187+
191188
const isPendingRet =
192189
dataUpdatedAt < startSummaryRequestTime.current ||
193190
isStartSummaryRequestPending ||
194191
isPending ||
192+
summaryData?.status === ReplaySummaryStatus.NOT_STARTED ||
195193
summaryData?.status === ReplaySummaryStatus.PROCESSING;
196-
const isErrorRet =
197-
isStartSummaryRequestError ||
198-
isError ||
199-
summaryData?.status === ReplaySummaryStatus.ERROR;
200194

195+
// Clears the polling timeout when we get valid summary results.
201196
useEffect(() => {
202-
// Clears the polling timeout when we get valid summary results.
203197
if (!isPendingRet) {
204-
clearPollingTimeout();
198+
cancelPollingTimeout();
205199
}
206-
}, [isPendingRet]);
207-
208-
// Auto-start logic.
209-
// TODO: remove the condition segmentCount <= 100
210-
// when BE is able to process more than 100 segments. Without this, generation will loop.
211-
const segmentsIncreased =
212-
summaryData?.num_segments !== null &&
213-
summaryData?.num_segments !== undefined &&
214-
segmentCount <= 100 &&
215-
segmentCount > summaryData.num_segments;
216-
const needsInitialGeneration = summaryData?.status === ReplaySummaryStatus.NOT_STARTED;
217-
218-
useEffect(() => {
219-
if ((segmentsIncreased || needsInitialGeneration) && !isPendingRet && !isErrorRet) {
220-
startSummaryRequest();
221-
}
222-
}, [
223-
segmentsIncreased,
224-
needsInitialGeneration,
225-
isPendingRet,
226-
startSummaryRequest,
227-
isErrorRet,
228-
]);
200+
}, [isPendingRet, cancelPollingTimeout]);
229201

230202
return {
231203
summaryData,
232204
isPending: isPendingRet,
233-
isError: isErrorRet,
205+
isError:
206+
isStartSummaryRequestError ||
207+
isError ||
208+
summaryData?.status === ReplaySummaryStatus.ERROR,
234209
isTimedOut: didTimeout,
235210
startSummaryRequest,
236211
isStartSummaryRequestPending,

0 commit comments

Comments
 (0)