Browse Source

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.
Tony Xiao 3 years ago
parent
commit
25e29fe63b

+ 5 - 2
static/app/utils/discover/genericDiscoverQuery.tsx

@@ -121,8 +121,8 @@ class _GenericDiscoverQuery<T, P> extends React.Component<Props<T, P>, State<T>>
   }
 
   componentDidUpdate(prevProps: Props<T, P>) {
-    // 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<T, P> extends React.Component<Props<T, P>, State<T>>
 
     beforeFetch?.(api);
 
+    // clear any inflight requests since they are now stale
+    api.clear();
+
     try {
       const [data, , resp] = await doDiscoverQuery<T>(api, url, apiPayload);
       if (this.state.tableFetchID !== tableFetchID) {

+ 0 - 6
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) {
     <GenericDiscoverQuery<Histograms, HistogramProps>
       route="events-histogram"
       getRequestPayload={getHistogramRequestPayload}
-      beforeFetch={beforeFetch}
       didFetch={didFetch}
       {...omit(props, 'children')}
     >

+ 0 - 2
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<T>({
     <GenericDiscoverQuery<T, AdditionalQueryProps>
       route={`events-trace/${traceId}`}
       getRequestPayload={getTraceFullRequestPayload}
-      beforeFetch={beforeFetch}
       eventView={eventView}
       {...props}
     >

+ 0 - 2
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({
     <GenericDiscoverQuery<TraceLite, AdditionalQueryProps>
       route={`events-trace-light/${traceId}`}
       getRequestPayload={getTraceLiteRequestPayload}
-      beforeFetch={beforeFetch}
       eventView={eventView}
       {...props}
     >

+ 0 - 2
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 (
     <GenericDiscoverQuery<TraceMeta, {}>
       route={`events-trace-meta/${traceId}`}
-      beforeFetch={beforeFetch}
       getRequestPayload={getTraceRequestPayload}
       eventView={eventView}
       {...props}

+ 0 - 5
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']);
 }

+ 0 - 6
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 (
     <GenericDiscoverQuery<HasMeasurements, HasMeasurementsProps>
       route="events-has-measurements"
       getRequestPayload={getHasMeasurementsRequestPayload}
-      beforeFetch={beforeFetch}
       {...omit(props, 'children')}
     >
       {({tableData, ...rest}) => {