Skip to content
This repository has been archived by the owner on Aug 9, 2022. It is now read-only.

Add semaphore to block on puppeteer chromium execution #284

Merged
merged 8 commits into from
Jan 6, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions kibana-reports/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
2 changes: 2 additions & 0 deletions kibana-reports/public/components/context_menu/context_menu.js
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,8 @@ const generateInContextReport = (
} else {
if (response.status === 403) {
addSuccessOrFailureToast('permissionsFailure');
} else if (response.status === 503) {
addSuccessOrFailureToast('timeoutFailure', reportSource);
} else {
addSuccessOrFailureToast('failure');
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ export const displayLoadingModal = () => {
}
};

export const addSuccessOrFailureToast = (status) => {
export const addSuccessOrFailureToast = (status, reportSource) => {
const generateToast = document.querySelectorAll('.euiGlobalToastList');
if (generateToast) {
try {
Expand All @@ -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 () {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand All @@ -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">
Expand All @@ -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>
`;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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.'
);
});
});
9 changes: 5 additions & 4 deletions kibana-reports/public/components/main/main.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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 = () => {
Expand Down
5 changes: 5 additions & 0 deletions kibana-reports/public/components/main/main_utils.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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.`
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

does 503 always because of timeout?

);
} else {
handleErrorToast();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 = () => {
Expand Down
7 changes: 7 additions & 0 deletions kibana-reports/server/plugin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -83,6 +89,7 @@ export class OpendistroKibanaReportsPlugin
(context, request) => {
return {
logger: this.logger,
semaphore: this.semaphore,
notificationClient,
esReportsClient,
};
Expand Down
22 changes: 15 additions & 7 deletions kibana-reports/server/routes/lib/createReport.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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
Expand Down Expand Up @@ -102,13 +105,18 @@ export const createReport = async (
}
});
}
createReportResult = await createVisualReport(
reportParams,
completeQueryUrl,
logger,
cookieObject,
timezone
);
const [value, release] = await semaphore.acquire();
penghuo marked this conversation as resolved.
Show resolved Hide resolved
try {
createReportResult = await createVisualReport(
reportParams,
completeQueryUrl,
logger,
cookieObject,
timezone
);
} finally {
release();
}
}
// update report state to "created"
// TODO: temporarily remove the following
Expand Down
12 changes: 12 additions & 0 deletions kibana-reports/yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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"
Expand Down