From e116bdac71ff052225cab46accd5218cf825726c Mon Sep 17 00:00:00 2001 From: Joshua <joshuali925@gmail.com> Date: Wed, 6 Jan 2021 23:04:51 +0000 Subject: [PATCH] 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 --- kibana-reports/package.json | 1 + .../components/context_menu/context_menu.js | 2 ++ .../context_menu/context_menu_helpers.js | 10 +++++++- .../context_menu/context_menu_ui.js | 9 ++++--- .../main/__tests__/main_utils.test.tsx | 24 +++++++++++++++++++ .../public/components/main/main.tsx | 9 +++---- .../public/components/main/main_utils.tsx | 5 ++++ .../main/report_details/report_details.tsx | 9 +++---- kibana-reports/server/plugin.ts | 7 ++++++ .../server/routes/lib/createReport.ts | 22 +++++++++++------ kibana-reports/yarn.lock | 12 ++++++++++ 11 files changed, 91 insertions(+), 19 deletions(-) diff --git a/kibana-reports/package.json b/kibana-reports/package.json index 077fccb7..06e28237 100644 --- 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 index b1e1ba8b..b46049c5 100644 --- 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 index 27535ef0..fbc47fd4 100644 --- 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 index a61e1976..184f7639 100644 --- 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 index 3386061a..27e0b7d8 100644 --- 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 index 72e8b76d..9f2c5545 100644 --- a/kibana-reports/public/components/main/main.tsx +++ b/kibana-reports/public/components/main/main.tsx @@ -103,9 +103,10 @@ 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', @@ -113,8 +114,8 @@ export function Main(props) { 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 index 1405533f..2fa4f414 100644 --- 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 index 3f597988..158556eb 100644 --- a/kibana-reports/public/components/main/report_details/report_details.tsx +++ b/kibana-reports/public/components/main/report_details/report_details.tsx @@ -94,9 +94,10 @@ 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', @@ -104,8 +105,8 @@ export function ReportDetails(props) { 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 index e0cd6e76..e6ddda63 100644 --- 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 index 5fed5a9c..e11f4460 100644 --- 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 index 46fe5d46..7ed15bdd 100644 --- 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"