fix: Remove synchronous operation with file system. (#879)

This patch:
- introduces `helper.promisify` - a simple polyfill for the `util.promisify`. The 
  `util.promisify` could not be used due to Node6 compatibility issues.
- migrates all sync filesystem operations to the async replicas

Fixes #884.
This commit is contained in:
Gleb Azarov 2017-10-11 10:55:48 +03:00 committed by Andrey Lushnikov
parent 7a8aa73466
commit ff08e45785
4 changed files with 42 additions and 13 deletions

View File

@ -15,7 +15,7 @@
*/ */
const os = require('os'); const os = require('os');
const path = require('path'); const path = require('path');
const removeSync = require('rimraf').sync; const removeFolder = require('rimraf');
const childProcess = require('child_process'); const childProcess = require('child_process');
const Downloader = require('../utils/ChromiumDownloader'); const Downloader = require('../utils/ChromiumDownloader');
const {Connection} = require('./Connection'); const {Connection} = require('./Connection');
@ -26,6 +26,9 @@ const {helper} = require('./helper');
// @ts-ignore // @ts-ignore
const ChromiumRevision = require('../package.json').puppeteer.chromium_revision; const ChromiumRevision = require('../package.json').puppeteer.chromium_revision;
const mkdtempAsync = helper.promisify(fs.mkdtemp);
const removeFolderAsync = helper.promisify(removeFolder);
const CHROME_PROFILE_PATH = path.join(os.tmpdir(), 'puppeteer_dev_profile-'); const CHROME_PROFILE_PATH = path.join(os.tmpdir(), 'puppeteer_dev_profile-');
const DEFAULT_ARGS = [ const DEFAULT_ARGS = [
@ -69,7 +72,7 @@ class Launcher {
if (!options.args || !options.args.some(arg => arg.startsWith('--user-data-dir'))) { if (!options.args || !options.args.some(arg => arg.startsWith('--user-data-dir'))) {
if (!options.userDataDir) if (!options.userDataDir)
temporaryUserDataDir = fs.mkdtempSync(CHROME_PROFILE_PATH); temporaryUserDataDir = await mkdtempAsync(CHROME_PROFILE_PATH);
chromeArguments.push(`--user-data-dir=${options.userDataDir || temporaryUserDataDir}`); chromeArguments.push(`--user-data-dir=${options.userDataDir || temporaryUserDataDir}`);
} }
@ -107,12 +110,16 @@ class Launcher {
chromeProcess.stderr.pipe(process.stderr); chromeProcess.stderr.pipe(process.stderr);
} }
const waitForChromeToClose = new Promise(fulfill => { const waitForChromeToClose = new Promise((fulfill, reject) => {
chromeProcess.once('close', () => { chromeProcess.once('close', () => {
// Cleanup as processes exit. // Cleanup as processes exit.
if (temporaryUserDataDir) if (temporaryUserDataDir) {
removeSync(temporaryUserDataDir); removeFolderAsync(temporaryUserDataDir)
.then(() => fulfill())
.catch(err => console.error(err));
} else {
fulfill(); fulfill();
}
}); });
}); });
@ -153,7 +160,7 @@ class Launcher {
if (temporaryUserDataDir) { if (temporaryUserDataDir) {
// Attempt to remove temporary profile directory to avoid littering. // Attempt to remove temporary profile directory to avoid littering.
try { try {
removeSync(temporaryUserDataDir); removeFolder.sync(temporaryUserDataDir);
} catch (e) { } } catch (e) { }
} }
return waitForChromeToClose; return waitForChromeToClose;

View File

@ -26,6 +26,8 @@ const {Keyboard, Mouse, Touchscreen} = require('./Input');
const Tracing = require('./Tracing'); const Tracing = require('./Tracing');
const {helper, debugError} = require('./helper'); const {helper, debugError} = require('./helper');
const writeFileAsync = helper.promisify(fs.writeFile);
class Page extends EventEmitter { class Page extends EventEmitter {
/** /**
* @param {!Puppeteer.Session} client * @param {!Puppeteer.Session} client
@ -655,7 +657,7 @@ class Page extends EventEmitter {
const buffer = new Buffer(result.data, 'base64'); const buffer = new Buffer(result.data, 'base64');
if (options.path) if (options.path)
fs.writeFileSync(options.path, buffer); await writeFileAsync(options.path, buffer);
return buffer; return buffer;
} }
@ -703,7 +705,7 @@ class Page extends EventEmitter {
}); });
const buffer = new Buffer(result.data, 'base64'); const buffer = new Buffer(result.data, 'base64');
if (options.path) if (options.path)
fs.writeFileSync(options.path, buffer); await writeFileAsync(options.path, buffer);
return buffer; return buffer;
} }

View File

@ -13,8 +13,12 @@
* See the License for the specific language governing permissions and * See the License for the specific language governing permissions and
* limitations under the License. * limitations under the License.
*/ */
const fs = require('fs');
const {helper} = require('./helper'); const {helper} = require('./helper');
const fs = require('fs');
const openAsync = helper.promisify(fs.open);
const writeAsync = helper.promisify(fs.write);
const closeAsync = helper.promisify(fs.close);
class Tracing { class Tracing {
/** /**
@ -68,14 +72,14 @@ class Tracing {
*/ */
async _readStream(handle, path) { async _readStream(handle, path) {
let eof = false; let eof = false;
const file = fs.openSync(path, 'w'); const file = await openAsync(path, 'w');
while (!eof) { while (!eof) {
const response = await this._client.send('IO.read', {handle}); const response = await this._client.send('IO.read', {handle});
eof = response.eof; eof = response.eof;
if (path) if (path)
fs.writeSync(file, response.data); await writeAsync(file, response.data);
} }
fs.closeSync(file); await closeAsync(file);
await this._client.send('IO.close', {handle}); await this._client.send('IO.close', {handle});
} }
} }

View File

@ -209,6 +209,22 @@ class Helper {
static isNumber(obj) { static isNumber(obj) {
return typeof obj === 'number' || obj instanceof Number; return typeof obj === 'number' || obj instanceof Number;
} }
static promisify(nodeFunction) {
function promisified(...args) {
return new Promise((resolve, reject) => {
function callback(err, ...result) {
if (err)
return reject(err);
if (result.length === 1)
return resolve(result[0]);
return resolve(result);
}
nodeFunction.call(null, ...args, callback);
});
}
return promisified;
}
} }
module.exports = { module.exports = {