Add semaphore to block on puppeteer chromium execution (#284)

* Add mutex to lock on creating report using puppeteer * Use semaphore instead of mutex * change semaphore limit to 1 * Add 30 seconds timeout * Change timeout to 180 sec, add error message * Update error message * Add unit test for timeout error
opendistro-for-elasticsearch · Jan 6, 2021 · e116bda · e116bda
1 parent 35dfe7d
commit e116bda
Show file tree

Hide file tree

Showing 11 changed files with 91 additions and 19 deletions.
diff --git a/kibana-reports/package.json b/kibana-reports/package.json
@@ -20,6 +20,7 @@
     "plugin_helpers": "node ../../scripts/plugin_helpers"
   },
   "dependencies": {
+    "async-mutex": "^0.2.6",
     "babel-polyfill": "^6.26.0",
     "cheerio": "0.22.0",
     "cron-validator": "^1.1.1",

diff --git a/kibana-reports/public/components/context_menu/context_menu.js b/kibana-reports/public/components/context_menu/context_menu.js
@@ -126,6 +126,8 @@ const generateInContextReport = (
       } else {
         if (response.status === 403) {
           addSuccessOrFailureToast('permissionsFailure');
+        } else if (response.status === 503) {
+          addSuccessOrFailureToast('timeoutFailure', reportSource);
         } else {
           addSuccessOrFailureToast('failure');
         }

diff --git a/kibana-reports/public/components/context_menu/context_menu_helpers.js b/kibana-reports/public/components/context_menu/context_menu_helpers.js
@@ -86,7 +86,7 @@ export const displayLoadingModal = () => {
   }
 };
 
-export const addSuccessOrFailureToast = (status) => {
+export const addSuccessOrFailureToast = (status, reportSource) => {
   const generateToast = document.querySelectorAll('.euiGlobalToastList');
   if (generateToast) {
     try {
@@ -101,6 +101,14 @@ export const addSuccessOrFailureToast = (status) => {
         setTimeout(function () {
           document.getElementById('reportFailureToast').style.display = 'none';
         }, 6000);
+      } else if (status === 'timeoutFailure') {
+        generateInProgressToast.innerHTML = reportGenerationFailure(
+          'Error generating report.',
+          `Timed out generating on-demand report from ${reportSource}. Try again later.`
+        );
+        setTimeout(function () {
+          document.getElementById('reportFailureToast').style.display = 'none';
+        }, 6000);
       } else if (status === 'permissionsFailure') {
         generateInProgressToast.innerHTML = permissionsMissingOnGeneration();
         setTimeout(function () {

diff --git a/kibana-reports/public/components/context_menu/context_menu_ui.js b/kibana-reports/public/components/context_menu/context_menu_ui.js
@@ -244,7 +244,10 @@ export const reportGenerationSuccess = () => {
   `;
 };
 
-export const reportGenerationFailure = () => {
+export const reportGenerationFailure = (
+  title = 'Download error',
+  text = 'There was an error generating this report.'
+) => {
   return `
   <div class="euiToast euiToast--danger" id="reportFailureToast">
     <p class="euiScreenReaderOnly">A new notification appears</p>
@@ -255,7 +258,7 @@ export const reportGenerationFailure = () => {
       role="img" aria-hidden="true">
         <path fill-rule="evenodd" d="M7.59 10.059L7.35 5.18h1.3L8.4 10.06h-.81zm.394 1.901a.61.61 0 01-.448-.186.606.606 0 01-.186-.444c0-.174.062-.323.186-.446a.614.614 0 01.448-.184c.169 0 .315.06.44.182.124.122.186.27.186.448a.6.6 0 01-.189.446.607.607 0 01-.437.184zM2 14a1 1 0 01-.878-1.479l6-11a1 1 0 011.756 0l6 11A1 1 0 0114 14H2zm0-1h12L8 2 2 13z"></path>
       </svg>
-      <span class="euiToastHeader__title">Download error</span>
+      <span class="euiToastHeader__title">${title}</span>
     </div>
     <button type="button" class="euiToast__closeButton" aria-label="Dismiss toast" id="closeReportFailureToast"
     data-test-subj="toastCloseButton">
@@ -265,7 +268,7 @@ export const reportGenerationFailure = () => {
       </svg>
     </button>
     <div class="euiText euiText--small euiToastBody">
-      <p>There was an error generating this report.</p>
+      <p>${text}</p>
     </div>
   </div>
   `;

diff --git a/kibana-reports/public/components/main/__tests__/main_utils.test.tsx b/kibana-reports/public/components/main/__tests__/main_utils.test.tsx
@@ -129,4 +129,28 @@ describe('main_utils tests', () => {
       handleErrorToast
     );
   });
+
+  test('test generateReportById timeout error handling', async () => {
+    expect.assertions(1);
+    const reportId = '1';
+    const handleSuccessToast = jest.fn();
+    const handleErrorToast = jest.fn();
+    const handlePermissionsMissingToast = jest.fn();
+
+    httpClientMock.get.mockReturnValue(
+      Promise.reject({ body: { statusCode: 503 } })
+    );
+
+    await generateReportById(
+      reportId,
+      httpClientMock,
+      handleSuccessToast,
+      handleErrorToast,
+      handlePermissionsMissingToast
+    );
+    expect(handleErrorToast).toHaveBeenCalledWith(
+      'Error generating report.',
+      'Timed out generating report ID 1. Try again later.'
+    );
+  });
 });
diff --git a/kibana-reports/public/components/main/main.tsx b/kibana-reports/public/components/main/main.tsx
@@ -103,18 +103,19 @@ export function Main(props) {
     addReportDefinitionsTableErrorToastHandler(errorType);
   };
 
-  const addErrorOnDemandDownloadToastHandler = () => {
+  const addErrorOnDemandDownloadToastHandler = (title = 'Error downloading report.', text = '') => {
     const errorToast = {
-      title: 'Error downloading report.',
+      title,
+      text,
       color: 'danger',
       iconType: 'alert',
       id: 'onDemandDownloadErrorToast',
     };
     setToasts(toasts.concat(errorToast));
   };
 
-  const handleOnDemandDownloadErrorToast = () => {
-    addErrorOnDemandDownloadToastHandler();
+  const handleOnDemandDownloadErrorToast = (title?: string, text?: string) => {
+    addErrorOnDemandDownloadToastHandler(title, text);
   };
 
   const addSuccessOnDemandDownloadToastHandler = () => {

diff --git a/kibana-reports/public/components/main/main_utils.tsx b/kibana-reports/public/components/main/main_utils.tsx
@@ -198,6 +198,11 @@ export const generateReportById = async (
       console.log('error on generating report by id:', error);
       if (error.body.statusCode === 403) {
         handlePermissionsMissingToast();
+      } else if (error.body.statusCode === 503) {
+        handleErrorToast(
+          'Error generating report.',
+          `Timed out generating report ID ${reportId}. Try again later.`
+        );
       } else {
         handleErrorToast();
       }

diff --git a/kibana-reports/public/components/main/report_details/report_details.tsx b/kibana-reports/public/components/main/report_details/report_details.tsx
@@ -94,18 +94,19 @@ export function ReportDetails(props) {
     addPermissionsMissingDownloadToastHandler();
   }
 
-  const addErrorToastHandler = () => {
+  const addErrorToastHandler = (title = 'Error loading report details.', text = '') => {
     const errorToast = {
-      title: 'Error loading report details.',
+      title,
+      text,
       color: 'danger',
       iconType: 'alert',
       id: 'reportDetailsErrorToast',
     };
     setToasts(toasts.concat(errorToast));
   };
 
-  const handleErrorToast = () => {
-    addErrorToastHandler();
+  const handleErrorToast = (title?: string, text?: string) => {
+    addErrorToastHandler(title, text);
   };
 
   const addSuccessToastHandler = () => {

diff --git a/kibana-reports/server/plugin.ts b/kibana-reports/server/plugin.ts
@@ -22,6 +22,7 @@ import {
   ILegacyClusterClient,
 } from '../../../src/core/server';
 import { setIntervalAsync } from 'set-interval-async/dynamic';
+import { Semaphore, SemaphoreInterface, withTimeout } from 'async-mutex';
 import esReportsPlugin from './backend/opendistro-es-reports-plugin';
 import notificationPlugin from './backend/opendistro-notification-plugin';
 import {
@@ -50,9 +51,14 @@ export class OpendistroKibanaReportsPlugin
       OpendistroKibanaReportsPluginStart
     > {
   private readonly logger: Logger;
+  private readonly semaphore: SemaphoreInterface;
 
   constructor(initializerContext: PluginInitializerContext) {
     this.logger = initializerContext.logger.get();
+
+    const timeoutError = new Error('Server busy');
+    timeoutError.statusCode = 503;
+    this.semaphore = withTimeout(new Semaphore(1), 180000, timeoutError);
   }
 
   public setup(core: CoreSetup) {
@@ -83,6 +89,7 @@ export class OpendistroKibanaReportsPlugin
       (context, request) => {
         return {
           logger: this.logger,
+          semaphore: this.semaphore,
           notificationClient,
           esReportsClient,
         };

diff --git a/kibana-reports/server/routes/lib/createReport.ts b/kibana-reports/server/routes/lib/createReport.ts
@@ -35,6 +35,7 @@ import { SetCookie } from 'puppeteer-core';
 import { deliverReport } from './deliverReport';
 import { updateReportState } from './updateReportState';
 import { saveReport } from './saveReport';
+import { SemaphoreInterface } from 'async-mutex';
 
 export const createReport = async (
   request: KibanaRequest,
@@ -45,6 +46,8 @@ export const createReport = async (
   const isScheduledTask = false;
   //@ts-ignore
   const logger: Logger = context.reporting_plugin.logger;
+  //@ts-ignore
+  const semaphore: SemaphoreInterface = context.reporting_plugin.semaphore;
   // @ts-ignore
   const notificationClient: ILegacyScopedClusterClient = context.reporting_plugin.notificationClient.asScoped(
     request
@@ -102,13 +105,18 @@ export const createReport = async (
           }
         });
       }
-      createReportResult = await createVisualReport(
-        reportParams,
-        completeQueryUrl,
-        logger,
-        cookieObject,
-        timezone
-      );
+      const [value, release] = await semaphore.acquire();
+      try {
+        createReportResult = await createVisualReport(
+          reportParams,
+          completeQueryUrl,
+          logger,
+          cookieObject,
+          timezone
+        );
+      } finally {
+        release();
+      }
     }
     // update report state to "created"
     // TODO: temporarily remove the following

diff --git a/kibana-reports/yarn.lock b/kibana-reports/yarn.lock
@@ -1041,6 +1041,13 @@ async-limiter@~1.0.0:
   resolved "https://registry.yarnpkg.com/async-limiter/-/async-limiter-1.0.1.tgz#dd379e94f0db8310b08291f9d64c3209766617fd"
   integrity sha512-csOlWGAcRFJaI6m+F2WKdnMKr4HhdhFVBk0H/QbJFMCr+uO2kwohwXQPxw/9OCxp05r5ghVBFSyioixx3gfkNQ==
 
+async-mutex@^0.2.6:
+  version "0.2.6"
+  resolved "https://registry.yarnpkg.com/async-mutex/-/async-mutex-0.2.6.tgz#0d7a3deb978bc2b984d5908a2038e1ae2e54ff40"
+  integrity sha512-Hs4R+4SPgamu6rSGW8C7cV9gaWUKEHykfzCCvIRuaVv636Ju10ZdeUbvb4TBEW0INuq2DHZqXbK4Nd3yG4RaRw==
+  dependencies:
+    tslib "^2.0.0"
+
 asynckit@^0.4.0:
   version "0.4.0"
   resolved "https://registry.yarnpkg.com/asynckit/-/asynckit-0.4.0.tgz#c79ed97f7f34cb8f2ba1bc9790bcc366474b4b79"
@@ -5453,6 +5460,11 @@ tslib@^1.9.0:
   resolved "https://registry.yarnpkg.com/tslib/-/tslib-1.14.1.tgz#cf2d38bdc34a134bcaf1091c41f6619e2f672d00"
   integrity sha512-Xni35NKzjgMrwevysHTCArtLDpPvye8zV/0E4EyYn43P7/7qvQwPh9BGkHewbMulVntbigmcT7rdX3BNo9wRJg==
 
+tslib@^2.0.0:
+  version "2.0.3"
+  resolved "https://registry.yarnpkg.com/tslib/-/tslib-2.0.3.tgz#8e0741ac45fc0c226e58a17bfc3e64b9bc6ca61c"
+  integrity sha512-uZtkfKblCEQtZKBF6EBXVZeQNl82yqtDQdv+eck8u7tdPxjLu2/lp5/uPW+um2tpuxINHWy3GhiccY7QgEaVHQ==
+
 tty-browserify@0.0.0:
   version "0.0.0"
   resolved "https://registry.yarnpkg.com/tty-browserify/-/tty-browserify-0.0.0.tgz#a157ba402da24e9bf957f9aa69d524eed42901a6"