Bmansurov has uploaded a new change for review. (
https://gerrit.wikimedia.org/r/390144 )
Change subject: Terminate browser process after certain time
......................................................................
Terminate browser process after certain time
By default after 5 minutes the puppeteer will terminate the browser
subprocess if it hasn't finished returning a PDF yet.
Bug: T178501
Change-Id: I904fc15826b7de3a34e5c977b40bfd8518bb1679
---
M config.dev.yaml
M lib/queue.js
M lib/renderer.js
M routes/html2pdf-v1.js
M test/lib/queue.js
A test/lib/renderer.js
6 files changed, 132 insertions(+), 46 deletions(-)
git pull ssh://gerrit.wikimedia.org:29418/mediawiki/services/chromium-render
refs/changes/44/390144/1
diff --git a/config.dev.yaml b/config.dev.yaml
index 6b5b682..00d6f4d 100644
--- a/config.dev.yaml
+++ b/config.dev.yaml
@@ -91,12 +91,17 @@
# some room for page numbers
bottom: '0.75in'
left: '0.5in'
- puppeteer_flags:
- - '--no-sandbox'
- - '--disable-setuid-sandbox'
+ #
https://github.com/GoogleChrome/puppeteer/blob/v0.11.0/docs/api.md#puppeteerlaunchoptions
+ puppeteer_options:
+ timeout: 30000
+ args:
+ - '--no-sandbox'
+ - '--disable-setuid-sandbox'
# the maximum number of puppeteer instances that can be launched at a
time
render_concurrency: 1
# don't wait to render a PDF after this many seconds
render_queue_timeout: 90
# maximum allowed number of pending jobs
- render_queue_count: 3
\ No newline at end of file
+ render_queue_count: 3
+ # the number of seconds before puppeteer terminates the browser instance
+ render_execution_timout: 300
\ No newline at end of file
diff --git a/lib/queue.js b/lib/queue.js
index 4fcea12..1750858 100644
--- a/lib/queue.js
+++ b/lib/queue.js
@@ -19,22 +19,24 @@
class Queue {
/**
* @param {number} concurrency number of concurrent render instances
- * @param {number} timeout number of seconds after which the
+ * @param {number} queueTimeout number of seconds after which the
* yet-to-start renders are aborted
* @param {number} maxCount number of tasks the queue should hold.
* New tasks will be rejected once the number of tasks in the queue
* is equal to this number.
- * @param {Object} puppeteerFlags flags used to in starting puppeteer
+ * @param {number} executionTimeout number of seconds after which
puppeteer
+ * closes the browser
+ * @param {Object} puppeteerOptions options used to in starting puppeteer
* @param {Object} pdfOptions pdf options passed to Chromium
* @param {Object} logger app logger
*/
- constructor(
- concurrency, timeout, maxCount, puppeteerFlags, pdfOptions, logger
- ) {
+ constructor(concurrency, queueTimeout, maxCount, executionTimeout,
+ puppeteerOptions, pdfOptions, logger) {
this._queueObject = asyncQueue(this._worker.bind(this), concurrency);
- this._timeout = timeout;
+ this._queueTimeout = queueTimeout;
this._maxCount = maxCount;
- this._puppeteerFlags = puppeteerFlags;
+ this._executionTimeout = executionTimeout;
+ this._puppeteerOptions = puppeteerOptions;
this._pdfOptions = pdfOptions;
this._logger = logger;
}
@@ -67,14 +69,14 @@
if (worker.data._id === data._id) {
that._logger.log('trace/info', {
msg: `Queue is still busy after waiting ` +
- `for ${that._timeout} secs.`
+ `for ${that._queueTimeout} secs.`
});
callback(queueErrors.timeout, null);
return true;
}
return false;
});
- }, this._timeout * 1000);
+ }, this._queueTimeout * 1000);
q.push(data, callback);
}
@@ -85,8 +87,8 @@
clearTimeout(data._timeoutID);
renderer
- .articleToPdf(data.uri, data.format, this._puppeteerFlags,
- this._pdfOptions)
+ .articleToPdf(data.uri, data.format, this._puppeteerOptions,
+ this._pdfOptions, this._executionTimeout)
.then((pdf) => {
callback(null, pdf);
})
diff --git a/lib/renderer.js b/lib/renderer.js
index dae34f1..518f849 100644
--- a/lib/renderer.js
+++ b/lib/renderer.js
@@ -6,36 +6,51 @@
* Renders content from `url` in PDF
* @param {string} url URL to get content from
* @param {string} format Page size, e.g. Letter or A4, passed to understands
- * @param {Object} puppeteerFlags
+ * @param {Object} puppeteerOptions
* @param {Object} pdfOptions
+ * @param {number} executionTimeout number of seconds after which puppeteer
+ * closes the browser
* @return {<Promise<Buffer>>} Promise which resolves with PDF buffer
*/
-exports.articleToPdf = function articleToPdf(url, format, puppeteerFlags,
pdfOptions) {
+exports.articleToPdf = function articleToPdf(
+ url, format, puppeteerOptions, pdfOptions, executionTimeout
+) {
let browser;
let page;
- return puppeteer.launch({ args: puppeteerFlags })
- .then((browser_) => {
- browser = browser_;
- return browser.newPage();
- })
- .then((page_) => {
- page = page_;
- return page.goto(url, { waitUntil: 'networkidle' });
- })
- .then(() => {
- return page.pdf(Object.assign(
- {}, pdfOptions, { format }
- ));
- })
- .catch((error) => {
+ return new Promise((resolve, reject) => {
+ const timeoutID = setTimeout(() => {
if (browser) {
browser.close();
}
- throw error;
- })
- .then((pdf) => {
- browser.close();
- return pdf;
- });
+ reject('timeout');
+ }, executionTimeout * 1000);
+
+ puppeteer.launch(puppeteerOptions)
+ .then((browser_) => {
+ browser = browser_;
+ return browser.newPage();
+ })
+ .then((page_) => {
+ page = page_;
+ return page.goto(url, { waitUntil: 'networkidle' });
+ })
+ .then(() => {
+ return page.pdf(Object.assign(
+ {}, pdfOptions, { format }
+ ));
+ })
+ .catch((error) => {
+ clearTimeout(timeoutID);
+ if (browser) {
+ browser.close();
+ }
+ reject(error);
+ })
+ .then((pdf) => {
+ clearTimeout(timeoutID);
+ browser.close();
+ resolve(pdf);
+ });
+ });
};
diff --git a/routes/html2pdf-v1.js b/routes/html2pdf-v1.js
index 17d7766..5847f48 100644
--- a/routes/html2pdf-v1.js
+++ b/routes/html2pdf-v1.js
@@ -65,7 +65,8 @@
conf.render_concurrency,
conf.render_queue_timeout,
conf.render_queue_count,
- conf.puppeteer_flags,
+ conf.render_execution_timout,
+ conf.puppeteer_options,
conf.pdf_options,
app.logger
);
diff --git a/test/lib/queue.js b/test/lib/queue.js
index db0e26e..aeb5422 100644
--- a/test/lib/queue.js
+++ b/test/lib/queue.js
@@ -3,10 +3,13 @@
const assert = require('../utils/assert.js');
const { queueErrors, Queue } = require('../../lib/queue');
const logger = { log: () => {} };
-const puppeteerFlags = [
- '--no-sandbox',
- '--disable-setuid-sandbox'
-];
+const puppeteerOptions = {
+ timeout: 3000,
+ args: [
+ '--no-sandbox',
+ '--disable-setuid-sandbox'
+ ]
+};
const pdfOptions = {
scale: 1,
displayHeaderFooter: false,
@@ -39,7 +42,7 @@
}, data.timeout);
};
}
- const q = new QueueTest(1, 90, 10, puppeteerFlags, pdfOptions, logger);
+ const q = new QueueTest(1, 90, 10, 300, puppeteerOptions, pdfOptions,
logger);
// first worker must finish after 1 sec
q.push({
@@ -82,7 +85,7 @@
}, data.timeout);
};
}
- const q = new QueueTest(1, 5, 1, puppeteerFlags, pdfOptions, logger);
+ const q = new QueueTest(1, 5, 1, 300, puppeteerOptions, pdfOptions,
logger);
// first worker completes in 3 seconds
q.push({
@@ -119,7 +122,7 @@
}, data.timeout);
};
}
- const q = new QueueTest(1, 1, 10, puppeteerFlags, pdfOptions, logger);
+ const q = new QueueTest(1, 1, 10, 300, puppeteerOptions, pdfOptions,
logger);
// first worker completes in 3 seconds
q.push({
diff --git a/test/lib/renderer.js b/test/lib/renderer.js
new file mode 100644
index 0000000..b778c71
--- /dev/null
+++ b/test/lib/renderer.js
@@ -0,0 +1,60 @@
+'use strict';
+
+const preq = require('preq');
+const assert = require('../utils/assert.js');
+const renderer = require('../../lib/renderer');
+
+const logger = { log: () => {} };
+const puppeteerOptions = {
+ timeout: 3000,
+ args: [
+ '--no-sandbox',
+ '--disable-setuid-sandbox'
+ ]
+};
+const pdfOptions = {
+ scale: 1,
+ displayHeaderFooter: false,
+ printBackground: false,
+ landscape: false,
+ pageRanges: '',
+ format: 'Letter',
+ margin: {
+ top: '0.5in',
+ right: '0.5in',
+ // some room for page numbers
+ bottom: '0.75in',
+ left: '0.5in'
+ }
+};
+const baseUrl = 'https://en.wikipedia.org/api/rest_v1/page/html/';
+
+describe('renderer', function() {
+ it('should return a promise that resolves with PDF', function() {
+ return renderer.articleToPdf(
+ `${baseUrl}Book_(disambiguation)`,
+ 'Letter',
+ puppeteerOptions,
+ pdfOptions,
+ 300
+ ).then((pdf) => {
+ assert.ok(Buffer.isBuffer(pdf));
+ }).catch((error) => {
+ assert.ok(false, `Unexepected error ${error}`);
+ });;
+ });
+
+ it('should reject when there is not enough time to render PDF', function()
{
+ return renderer.articleToPdf(
+ `${baseUrl}Book_(disambiguation)`,
+ 'Letter',
+ puppeteerOptions,
+ pdfOptions,
+ 0.1
+ ).then((pdf) => {
+ assert.ok(false, 'Was not expecting a PDF');
+ }).catch((error) => {
+ assert.ok(error === 'timeout');
+ });
+ });
+});
--
To view, visit https://gerrit.wikimedia.org/r/390144
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I904fc15826b7de3a34e5c977b40bfd8518bb1679
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/services/chromium-render
Gerrit-Branch: master
Gerrit-Owner: Bmansurov <bmansu...@wikimedia.org>
_______________________________________________
MediaWiki-commits mailing list
MediaWiki-commits@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
