[Discussion] Increase timeout but add another warning timeout for slo…

…w servers - per recommendation/convo with Brandon
cee-chen · Jul 1, 2020 · 98ddbd3 · 98ddbd3
1 parent e813abb
commit 98ddbd3
Show file tree

Hide file tree

Showing 5 changed files with 31 additions and 16 deletions.
diff --git a/x-pack/plugins/enterprise_search/server/index.ts b/x-pack/plugins/enterprise_search/server/index.ts
@@ -15,7 +15,8 @@ export const plugin = (initializerContext: PluginInitializerContext) => {
 export const configSchema = schema.object({
   host: schema.maybe(schema.string()),
   enabled: schema.boolean({ defaultValue: true }),
-  accessCheckTimeout: schema.number({ defaultValue: 200 }),
+  accessCheckTimeout: schema.number({ defaultValue: 5000 }),
+  accessCheckTimeoutWarning: schema.number({ defaultValue: 300 }),
 });
 
 type ConfigType = TypeOf<typeof configSchema>;

diff --git a/x-pack/plugins/enterprise_search/server/lib/enterprise_search_config_api.test.ts b/x-pack/plugins/enterprise_search/server/lib/enterprise_search_config_api.test.ts
@@ -16,6 +16,7 @@ describe('callEnterpriseSearchConfigAPI', () => {
   const mockConfig = {
     host: 'http://localhost:3002',
     accessCheckTimeout: 200,
+    accessCheckTimeoutWarning: 100,
   };
   const mockRequest = {
     url: { path: '/app/kibana' },
@@ -90,11 +91,18 @@ describe('callEnterpriseSearchConfigAPI', () => {
   it('handles timeouts', async () => {
     jest.useFakeTimers();
 
+    // Warning
+    callEnterpriseSearchConfigAPI(mockDependencies);
+    jest.advanceTimersByTime(150);
+    expect(mockDependencies.log.warn).toHaveBeenCalledWith(
+      'Enterprise Search access check took over 100ms. Please ensure your Enterprise Search server is respondingly normally and not adversely impacting Kibana load speeds.'
+    );
+
+    // Timeout
     fetchMock.mockImplementationOnce(async () => {
       jest.advanceTimersByTime(250);
       return Promise.reject({ name: 'AbortError' });
     });
-
     expect(await callEnterpriseSearchConfigAPI(mockDependencies)).toEqual({});
     expect(mockDependencies.log.warn).toHaveBeenCalledWith(
       "Exceeded 200ms timeout while checking http://localhost:3002. Please consider increasing your enterpriseSearch.accessCheckTimeout value so that users aren't prevented from accessing Enterprise Search plugins due to slow responses."

diff --git a/x-pack/plugins/enterprise_search/server/lib/enterprise_search_config_api.ts b/x-pack/plugins/enterprise_search/server/lib/enterprise_search_config_api.ts
@@ -20,7 +20,6 @@ interface IReturn {
   publicUrl?: string;
   access?: IAccess;
 }
-type TTimeout = ReturnType<typeof setTimeout>;
 
 /**
  * Calls an internal Enterprise Search API endpoint which returns
@@ -36,14 +35,20 @@ export const callEnterpriseSearchConfigAPI = async ({
 }: IParams): Promise<IReturn> => {
   if (!config.host) return {};
 
-  let timeout: TTimeout;
-  try {
-    // Timeout if the remote call to Enterprise Search takes too long
-    const controller = new AbortController();
-    timeout = setTimeout(() => {
-      controller.abort();
-    }, config.accessCheckTimeout);
+  const TIMEOUT_WARNING = `Enterprise Search access check took over ${config.accessCheckTimeoutWarning}ms. Please ensure your Enterprise Search server is respondingly normally and not adversely impacting Kibana load speeds.`;
+  const TIMEOUT_MESSAGE = `Exceeded ${config.accessCheckTimeout}ms timeout while checking ${config.host}. Please consider increasing your enterpriseSearch.accessCheckTimeout value so that users aren't prevented from accessing Enterprise Search plugins due to slow responses.`;
+  const CONNECTION_ERROR = 'Could not perform access check to Enterprise Search';
+
+  const warningTimeout = setTimeout(() => {
+    log.warn(TIMEOUT_WARNING);
+  }, config.accessCheckTimeoutWarning);
 
+  const controller = new AbortController();
+  const timeout = setTimeout(() => {
+    controller.abort();
+  }, config.accessCheckTimeout);
+
+  try {
     const enterpriseSearchUrl = encodeURI(`${config.host}${ENDPOINT}`);
     const response = await fetch(enterpriseSearchUrl, {
       headers: { Authorization: request.headers.authorization as string },
@@ -60,15 +65,14 @@ export const callEnterpriseSearchConfigAPI = async ({
     };
   } catch (err) {
     if (err.name === 'AbortError') {
-      const message = `Exceeded ${config.accessCheckTimeout}ms timeout while checking ${config.host}. Please consider increasing your enterpriseSearch.accessCheckTimeout value so that users aren't prevented from accessing Enterprise Search plugins due to slow responses.`;
-      log.warn(message);
+      log.warn(TIMEOUT_MESSAGE);
     } else {
-      const message = `Could not perform access check to Enterprise Search: ${err.toString()}`;
-      log.error(message);
+      log.error(`${CONNECTION_ERROR}: ${err.toString()}`);
       if (err instanceof Error) log.debug(err.stack as string);
     }
     return {};
   } finally {
-    clearTimeout(timeout! as TTimeout);
+    clearTimeout(warningTimeout);
+    clearTimeout(timeout);
   }
 };
diff --git a/x-pack/plugins/enterprise_search/server/plugin.ts b/x-pack/plugins/enterprise_search/server/plugin.ts
@@ -34,6 +34,7 @@ export interface ServerConfigType {
   host?: string;
   enabled: boolean;
   accessCheckTimeout: number;
+  accessCheckTimeoutWarning: number;
 }
 
 export interface IRouteDependencies {

diff --git a/x-pack/plugins/enterprise_search/server/routes/__mocks__/config.mock.ts b/x-pack/plugins/enterprise_search/server/routes/__mocks__/config.mock.ts
@@ -7,5 +7,6 @@
 export const mockConfig = {
   enabled: true,
   host: 'http://localhost:3002',
-  accessCheckTimeout: 200,
+  accessCheckTimeout: 5000,
+  accessCheckTimeoutWarning: 300,
 };