From 0a244f8049729f428b7aff1b88f87f9c5dde8d7a Mon Sep 17 00:00:00 2001 From: Tony Xiao Date: Thu, 23 Sep 2021 10:21:27 -0400 Subject: [PATCH] fix(discover): Should reload when payload changes while loading (#28653) If the eventView changes while the request is in-flight/loading, we should cancel the in-flight request and fetch again. This is problematic in cases where the request takes a little longer to load and the user changes the query while it is still in-flight. If a reload is not triggered at this time, the results are actually incorrect since it's showing the results from the previous (stale) in-flight request. --- static/app/utils/discover/genericDiscoverQuery.tsx | 7 +++++-- static/app/utils/performance/histogram/histogramQuery.tsx | 6 ------ static/app/utils/performance/quickTrace/traceFullQuery.tsx | 2 -- static/app/utils/performance/quickTrace/traceLiteQuery.tsx | 2 -- static/app/utils/performance/quickTrace/traceMetaQuery.tsx | 2 -- static/app/utils/performance/quickTrace/utils.tsx | 5 ----- .../app/utils/performance/vitals/hasMeasurementsQuery.tsx | 6 ------ 7 files changed, 5 insertions(+), 25 deletions(-) diff --git a/static/app/utils/discover/genericDiscoverQuery.tsx b/static/app/utils/discover/genericDiscoverQuery.tsx index c5acdb7bbd5437..41139a12d72863 100644 --- a/static/app/utils/discover/genericDiscoverQuery.tsx +++ b/static/app/utils/discover/genericDiscoverQuery.tsx @@ -121,8 +121,8 @@ class _GenericDiscoverQuery extends React.Component, State> } componentDidUpdate(prevProps: Props) { - // Reload data if we aren't already loading, - const refetchCondition = !this.state.isLoading && this._shouldRefetchData(prevProps); + // Reload data if the payload changes + const refetchCondition = this._shouldRefetchData(prevProps); // or if we've moved from an invalid view state to a valid one, const eventViewValidation = @@ -199,6 +199,9 @@ class _GenericDiscoverQuery extends React.Component, State> beforeFetch?.(api); + // clear any inflight requests since they are now stale + api.clear(); + try { const [data, , resp] = await doDiscoverQuery(api, url, apiPayload); if (this.state.tableFetchID !== tableFetchID) { diff --git a/static/app/utils/performance/histogram/histogramQuery.tsx b/static/app/utils/performance/histogram/histogramQuery.tsx index 74fed9ae6e31e8..e3e462e762ab53 100644 --- a/static/app/utils/performance/histogram/histogramQuery.tsx +++ b/static/app/utils/performance/histogram/histogramQuery.tsx @@ -1,7 +1,6 @@ import * as React from 'react'; import omit from 'lodash/omit'; -import {Client} from 'app/api'; import GenericDiscoverQuery, { DiscoverQueryProps, GenericChildrenProps, @@ -51,10 +50,6 @@ function getHistogramRequestPayload(props: RequestProps) { return apiPayload; } -function beforeFetch(api: Client) { - api.clear(); -} - function HistogramQuery(props: Props) { const {children, fields, didReceiveMultiAxis} = props; @@ -88,7 +83,6 @@ function HistogramQuery(props: Props) { route="events-histogram" getRequestPayload={getHistogramRequestPayload} - beforeFetch={beforeFetch} didFetch={didFetch} {...omit(props, 'children')} > diff --git a/static/app/utils/performance/quickTrace/traceFullQuery.tsx b/static/app/utils/performance/quickTrace/traceFullQuery.tsx index 4b5a26949d1dce..faf1b9444deb25 100644 --- a/static/app/utils/performance/quickTrace/traceFullQuery.tsx +++ b/static/app/utils/performance/quickTrace/traceFullQuery.tsx @@ -11,7 +11,6 @@ import { TraceRequestProps, } from 'app/utils/performance/quickTrace/types'; import { - beforeFetch, getTraceRequestPayload, makeEventView, } from 'app/utils/performance/quickTrace/utils'; @@ -80,7 +79,6 @@ function GenericTraceFullQuery({ route={`events-trace/${traceId}`} getRequestPayload={getTraceFullRequestPayload} - beforeFetch={beforeFetch} eventView={eventView} {...props} > diff --git a/static/app/utils/performance/quickTrace/traceLiteQuery.tsx b/static/app/utils/performance/quickTrace/traceLiteQuery.tsx index 1eb0d0aada83a6..b8aab1ccbdc955 100644 --- a/static/app/utils/performance/quickTrace/traceLiteQuery.tsx +++ b/static/app/utils/performance/quickTrace/traceLiteQuery.tsx @@ -10,7 +10,6 @@ import { TraceRequestProps, } from 'app/utils/performance/quickTrace/types'; import { - beforeFetch, getTraceRequestPayload, makeEventView, } from 'app/utils/performance/quickTrace/utils'; @@ -69,7 +68,6 @@ function TraceLiteQuery({ route={`events-trace-light/${traceId}`} getRequestPayload={getTraceLiteRequestPayload} - beforeFetch={beforeFetch} eventView={eventView} {...props} > diff --git a/static/app/utils/performance/quickTrace/traceMetaQuery.tsx b/static/app/utils/performance/quickTrace/traceMetaQuery.tsx index 9626921fa1f7c6..a104adab4df0da 100644 --- a/static/app/utils/performance/quickTrace/traceMetaQuery.tsx +++ b/static/app/utils/performance/quickTrace/traceMetaQuery.tsx @@ -7,7 +7,6 @@ import { TraceRequestProps, } from 'app/utils/performance/quickTrace/types'; import { - beforeFetch, getTraceRequestPayload, makeEventView, } from 'app/utils/performance/quickTrace/utils'; @@ -46,7 +45,6 @@ function TraceMetaQuery({ return ( route={`events-trace-meta/${traceId}`} - beforeFetch={beforeFetch} getRequestPayload={getTraceRequestPayload} eventView={eventView} {...props} diff --git a/static/app/utils/performance/quickTrace/utils.tsx b/static/app/utils/performance/quickTrace/utils.tsx index c6089052a09333..eb7ba11c7f1bf3 100644 --- a/static/app/utils/performance/quickTrace/utils.tsx +++ b/static/app/utils/performance/quickTrace/utils.tsx @@ -1,7 +1,6 @@ import omit from 'lodash/omit'; import moment from 'moment-timezone'; -import {Client} from 'app/api'; import {getTraceDateTimeRange} from 'app/components/events/interfaces/spans/utils'; import {ALL_ACCESS_PROJECTS} from 'app/constants/globalSelectionHeader'; import {OrganizationSummary} from 'app/types'; @@ -231,10 +230,6 @@ function sortTraceLite(trace: TraceLite): TraceLite { return trace.sort((a, b) => b['transaction.duration'] - a['transaction.duration']); } -export function beforeFetch(api: Client) { - api.clear(); -} - export function getTraceRequestPayload({eventView, location}: DiscoverQueryProps) { return omit(eventView.getEventsAPIPayload(location), ['field', 'sort', 'per_page']); } diff --git a/static/app/utils/performance/vitals/hasMeasurementsQuery.tsx b/static/app/utils/performance/vitals/hasMeasurementsQuery.tsx index 931a3751b33abd..8acd1f48fe477b 100644 --- a/static/app/utils/performance/vitals/hasMeasurementsQuery.tsx +++ b/static/app/utils/performance/vitals/hasMeasurementsQuery.tsx @@ -1,7 +1,6 @@ import omit from 'lodash/omit'; import pick from 'lodash/pick'; -import {Client} from 'app/api'; import {escapeDoubleQuotes} from 'app/utils'; import GenericDiscoverQuery, { DiscoverQueryProps, @@ -41,16 +40,11 @@ function getHasMeasurementsRequestPayload(props: RequestProps) { return Object.assign(baseApiPayload, additionalApiPayload); } -function beforeFetch(api: Client) { - api.clear(); -} - function HasMeasurementsQuery(props: Props) { return ( route="events-has-measurements" getRequestPayload={getHasMeasurementsRequestPayload} - beforeFetch={beforeFetch} {...omit(props, 'children')} > {({tableData, ...rest}) => {