fix: bump client side chart timeouts to use the SUPERSET_WEBSERVER_TIMEOUT (#28018)

superset-frontend/src/SqlLab/fixtures.ts

@@ -679,6 +679,7 @@ export const initialState = {
       DISPLAY_MAX_ROW: 100,
       SQLALCHEMY_DOCS_URL: 'test_SQLALCHEMY_DOCS_URL',
       SQLALCHEMY_DISPLAY_TEXT: 'test_SQLALCHEMY_DISPLAY_TEXT',
+      SUPERSET_WEBSERVER_TIMEOUT: '300',
     },
   },
 };

superset-frontend/src/components/Chart/chartAction.js

@@ -248,17 +248,20 @@ export async function getChartDataRequest({
 
 export function runAnnotationQuery({
   annotation,
-  timeout = 60,
+  timeout,
   formData = null,
   key,
   isDashboardRequest = false,
   force = false,
 }) {
   return function (dispatch, getState) {
-    const sliceKey = key || Object.keys(getState().charts)[0];
+    const { charts, common } = getState();
+    const sliceKey = key || Object.keys(charts)[0];
+    const queryTimeout = timeout || common.conf.SUPERSET_WEBSERVER_TIMEOUT;
+
     // make a copy of formData, not modifying original formData
     const fd = {
-      ...(formData || getState().charts[sliceKey].latestQueryFormData),
+      ...(formData || charts[sliceKey].latestQueryFormData),
     };
 
     if (!annotation.sourceType) {
@@ -309,7 +312,7 @@ export function runAnnotationQuery({
     return SupersetClient.post({
       url,
       signal,
-      timeout: timeout * 1000,
+      timeout: queryTimeout * 1000,
       headers: { 'Content-Type': 'application/json' },
       jsonPayload: buildV1ChartDataPayload({
         formData: fd,
@@ -396,18 +399,20 @@ export function handleChartDataResponse(response, json, useLegacyApi) {
 export function exploreJSON(
   formData,
   force = false,
-  timeout = 60,
+  timeout,
   key,
   dashboardId,
   ownState,
 ) {
-  return async dispatch => {
+  return async (dispatch, getState) => {
     const logStart = Logger.getTimestamp();
     const controller = new AbortController();
+    const queryTimeout =
+      timeout || getState().common.conf.SUPERSET_WEBSERVER_TIMEOUT;
 
     const requestParams = {
       signal: controller.signal,
-      timeout: timeout * 1000,
+      timeout: queryTimeout * 1000,
     };
     if (dashboardId) requestParams.dashboard_id = dashboardId;
 
@@ -519,7 +524,7 @@ export const POST_CHART_FORM_DATA = 'POST_CHART_FORM_DATA';
 export function postChartFormData(
   formData,
   force = false,
-  timeout = 60,
+  timeout,
   key,
   dashboardId,
   ownState,

superset-frontend/src/components/Chart/chartActions.test.js

@@ -28,6 +28,27 @@ import * as actions from 'src/components/Chart/chartAction';
 import * as asyncEvent from 'src/middleware/asyncEvent';
 import { handleChartDataResponse } from 'src/components/Chart/chartAction';
 
+import configureMockStore from 'redux-mock-store';
+import thunk from 'redux-thunk';
+import { initialState } from 'src/SqlLab/fixtures';
+
+const middlewares = [thunk];
+const mockStore = configureMockStore(middlewares);
+
+const mockGetState = () => ({
+  charts: {
+    chartKey: {
+      latestQueryFormData: {
+        time_grain_sqla: 'P1D',
+        granularity_sqla: 'Date',
+      },
+    },
+  },
+  common: {
+    conf: {},
+  },
+});
+
 describe('chart actions', () => {
   const MOCK_URL = '/mockURL';
   let dispatch;
@@ -94,7 +115,7 @@ describe('chart actions', () => {
 
     it('should query with the built query', async () => {
       const actionThunk = actions.postChartFormData({}, null);
-      await actionThunk(dispatch);
+      await actionThunk(dispatch, mockGetState);
 
       expect(fetchMock.calls(MOCK_URL)).toHaveLength(1);
       expect(fetchMock.calls(MOCK_URL)[0][1].body).toBe(
@@ -165,7 +186,7 @@ describe('chart actions', () => {
     it('should dispatch CHART_UPDATE_STARTED action before the query', () => {
       const actionThunk = actions.postChartFormData({});
 
-      return actionThunk(dispatch).then(() => {
+      return actionThunk(dispatch, mockGetState).then(() => {
         // chart update, trigger query, update form data, success
         expect(dispatch.callCount).toBe(5);
         expect(fetchMock.calls(MOCK_URL)).toHaveLength(1);
@@ -175,7 +196,7 @@ describe('chart actions', () => {
 
     it('should dispatch TRIGGER_QUERY action with the query', () => {
       const actionThunk = actions.postChartFormData({});
-      return actionThunk(dispatch).then(() => {
+      return actionThunk(dispatch, mockGetState).then(() => {
         // chart update, trigger query, update form data, success
         expect(dispatch.callCount).toBe(5);
         expect(fetchMock.calls(MOCK_URL)).toHaveLength(1);
@@ -185,7 +206,7 @@ describe('chart actions', () => {
 
     it('should dispatch UPDATE_QUERY_FORM_DATA action with the query', () => {
       const actionThunk = actions.postChartFormData({});
-      return actionThunk(dispatch).then(() => {
+      return actionThunk(dispatch, mockGetState).then(() => {
         // chart update, trigger query, update form data, success
         expect(dispatch.callCount).toBe(5);
         expect(fetchMock.calls(MOCK_URL)).toHaveLength(1);
@@ -195,7 +216,7 @@ describe('chart actions', () => {
 
     it('should dispatch logEvent async action', () => {
       const actionThunk = actions.postChartFormData({});
-      return actionThunk(dispatch).then(() => {
+      return actionThunk(dispatch, mockGetState).then(() => {
         // chart update, trigger query, update form data, success
         expect(dispatch.callCount).toBe(5);
         expect(fetchMock.calls(MOCK_URL)).toHaveLength(1);
@@ -209,7 +230,7 @@ describe('chart actions', () => {
 
     it('should dispatch CHART_UPDATE_SUCCEEDED action upon success', () => {
       const actionThunk = actions.postChartFormData({});
-      return actionThunk(dispatch).then(() => {
+      return actionThunk(dispatch, mockGetState).then(() => {
         // chart update, trigger query, update form data, success
         expect(dispatch.callCount).toBe(5);
         expect(fetchMock.calls(MOCK_URL)).toHaveLength(1);
@@ -226,7 +247,7 @@ describe('chart actions', () => {
       const timeoutInSec = 1 / 1000;
       const actionThunk = actions.postChartFormData({}, false, timeoutInSec);
 
-      return actionThunk(dispatch).then(() => {
+      return actionThunk(dispatch, mockGetState).then(() => {
         // chart update, trigger query, update form data, fail
         expect(fetchMock.calls(MOCK_URL)).toHaveLength(1);
         expect(dispatch.callCount).toBe(5);
@@ -245,7 +266,7 @@ describe('chart actions', () => {
       const timeoutInSec = 100; // Set to a time that is longer than the time this will take to fail
       const actionThunk = actions.postChartFormData({}, false, timeoutInSec);
 
-      return actionThunk(dispatch).then(() => {
+      return actionThunk(dispatch, mockGetState).then(() => {
         // chart update, trigger query, update form data, fail
         expect(dispatch.callCount).toBe(5);
         const updateFailedAction = dispatch.args[4][0];
@@ -278,17 +299,6 @@ describe('chart actions', () => {
 
   describe('runAnnotationQuery', () => {
     const mockDispatch = jest.fn();
-    const mockGetState = () => ({
-      charts: {
-        chartKey: {
-          latestQueryFormData: {
-            time_grain_sqla: 'P1D',
-            granularity_sqla: 'Date',
-          },
-        },
-      },
-    });
-
     beforeEach(() => {
       jest.clearAllMocks();
     });
@@ -342,3 +352,72 @@ describe('chart actions', () => {
     });
   });
 });
+
+describe('chart actions timeout', () => {
+  beforeEach(() => {
+    jest.clearAllMocks();
+  });
+
+  it('should use the timeout from arguments when given', () => {
+    const postSpy = jest.spyOn(SupersetClient, 'post');
+    postSpy.mockImplementation(() => Promise.resolve({ json: { result: [] } }));
+    const timeout = 10; // Set the timeout value here
+    const formData = { datasource: 'table__1' }; // Set the formData here
+    const key = 'chartKey'; // Set the chart key here
+
+    const store = mockStore(initialState);
+    store.dispatch(
+      actions.runAnnotationQuery({
+        annotation: {
+          value: 'annotationValue',
+          sourceType: 'Event',
+          overrides: {},
+        },
+        timeout,
+        formData,
+        key,
+      }),
+    );
+
+    const expectedPayload = {
+      url: expect.any(String),
+      signal: expect.any(AbortSignal),
+      timeout: timeout * 1000,
+      headers: { 'Content-Type': 'application/json' },
+      jsonPayload: expect.any(Object),
+    };
+
+    expect(postSpy).toHaveBeenCalledWith(expectedPayload);
+  });
+
+  it('should use the timeout from common.conf when not passed as an argument', () => {
+    const postSpy = jest.spyOn(SupersetClient, 'post');
+    postSpy.mockImplementation(() => Promise.resolve({ json: { result: [] } }));
+    const formData = { datasource: 'table__1' }; // Set the formData here
+    const key = 'chartKey'; // Set the chart key here
+
+    const store = mockStore(initialState);
+    store.dispatch(
+      actions.runAnnotationQuery({
+        annotation: {
+          value: 'annotationValue',
+          sourceType: 'Event',
+          overrides: {},
+        },
+        undefined,
+        formData,
+        key,
+      }),
+    );
+
+    const expectedPayload = {
+      url: expect.any(String),
+      signal: expect.any(AbortSignal),
+      timeout: initialState.common.conf.SUPERSET_WEBSERVER_TIMEOUT * 1000,
+      headers: { 'Content-Type': 'application/json' },
+      jsonPayload: expect.any(Object),
+    };
+
+    expect(postSpy).toHaveBeenCalledWith(expectedPayload);
+  });
+});