From c8bf4327d8a845b99258c8268002bbbfd4dfaa0d Mon Sep 17 00:00:00 2001 From: Joyee Cheung Date: Thu, 31 Jan 2019 00:35:57 +0800 Subject: [PATCH] process: move process mutation into bootstrap/node.js MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This patch moves the part in the report initialization that mutates the process object into bootstrap/node.js so it's easier to tell the side effect of the initialization on the global state during bootstrap. PR-URL: https://github.com/nodejs/node/pull/25821 Reviewed-By: Anna Henningsen Reviewed-By: Gus Caplan Reviewed-By: Richard Lau Reviewed-By: Colin Ihrig Reviewed-By: Michaël Zasso --- lib/internal/bootstrap/node.js | 13 +- lib/internal/process/report.js | 276 ++++++++++++++++----------------- 2 files changed, 149 insertions(+), 140 deletions(-) diff --git a/lib/internal/bootstrap/node.js b/lib/internal/bootstrap/node.js index 534f43bad6f241..d0b57f136e7ff9 100644 --- a/lib/internal/bootstrap/node.js +++ b/lib/internal/bootstrap/node.js @@ -333,7 +333,18 @@ if (process.env.NODE_V8_COVERAGE) { } if (getOptionValue('--experimental-report')) { - NativeModule.require('internal/process/report').setup(); + const { + config, + handleSignal, + report, + syncConfig + } = NativeModule.require('internal/process/report'); + process.report = report; + // Download the CLI / ENV config into JS land. + syncConfig(config, false); + if (config.events.includes('signal')) { + process.on(config.signal, handleSignal); + } } function setupTraceCategoryState() { diff --git a/lib/internal/process/report.js b/lib/internal/process/report.js index f83282ca1920b9..4ef03885ec64a7 100644 --- a/lib/internal/process/report.js +++ b/lib/internal/process/report.js @@ -3,158 +3,156 @@ const { emitExperimentalWarning } = require('internal/util'); const { ERR_INVALID_ARG_TYPE, - ERR_SYNTHETIC } = require('internal/errors').codes; + ERR_SYNTHETIC +} = require('internal/errors').codes; -exports.setup = function() { - const REPORTEVENTS = 1; - const REPORTSIGNAL = 2; - const REPORTFILENAME = 3; - const REPORTPATH = 4; - const REPORTVERBOSE = 5; +const REPORTEVENTS = 1; +const REPORTSIGNAL = 2; +const REPORTFILENAME = 3; +const REPORTPATH = 4; +const REPORTVERBOSE = 5; - // If report is enabled, extract the binding and - // wrap the APIs with thin layers, with some error checks. - // user options can come in from CLI / ENV / API. - // CLI and ENV is intercepted in C++ and the API call here (JS). - // So sync up with both sides as appropriate - initially from - // C++ to JS and from JS to C++ whenever the API is called. - // Some events are controlled purely from JS (signal | exception) - // and some from C++ (fatalerror) so this sync-up is essential for - // correct behavior and alignment with the supplied tunables. - const nr = internalBinding('report'); +// If report is enabled, extract the binding and +// wrap the APIs with thin layers, with some error checks. +// user options can come in from CLI / ENV / API. +// CLI and ENV is intercepted in C++ and the API call here (JS). +// So sync up with both sides as appropriate - initially from +// C++ to JS and from JS to C++ whenever the API is called. +// Some events are controlled purely from JS (signal | exception) +// and some from C++ (fatalerror) so this sync-up is essential for +// correct behavior and alignment with the supplied tunables. +const nr = internalBinding('report'); - // Keep it un-exposed; lest programs play with it - // leaving us with a lot of unwanted sanity checks. - let config = { - events: [], - signal: 'SIGUSR2', - filename: '', - path: '', - verbose: false - }; - const report = { - setDiagnosticReportOptions(options) { - emitExperimentalWarning('report'); - // Reuse the null and undefined checks. Save - // space when dealing with large number of arguments. - const list = parseOptions(options); +// Keep it un-exposed; lest programs play with it +// leaving us with a lot of unwanted sanity checks. +let config = { + events: [], + signal: 'SIGUSR2', + filename: '', + path: '', + verbose: false +}; +const report = { + setDiagnosticReportOptions(options) { + emitExperimentalWarning('report'); + // Reuse the null and undefined checks. Save + // space when dealing with large number of arguments. + const list = parseOptions(options); - // Flush the stale entries from report, as - // we are refreshing it, items that the users did not - // touch may be hanging around stale otherwise. - config = {}; + // Flush the stale entries from report, as + // we are refreshing it, items that the users did not + // touch may be hanging around stale otherwise. + config = {}; - // The parseOption method returns an array that include - // the indices at which valid params are present. - list.forEach((i) => { - switch (i) { - case REPORTEVENTS: - if (Array.isArray(options.events)) - config.events = options.events; - else - throw new ERR_INVALID_ARG_TYPE('events', - 'Array', - options.events); - break; - case REPORTSIGNAL: - if (typeof options.signal !== 'string') { - throw new ERR_INVALID_ARG_TYPE('signal', - 'String', - options.signal); - } - process.removeListener(config.signal, handleSignal); - if (config.events.includes('signal')) - process.on(options.signal, handleSignal); - config.signal = options.signal; - break; - case REPORTFILENAME: - if (typeof options.filename !== 'string') { - throw new ERR_INVALID_ARG_TYPE('filename', - 'String', - options.filename); - } - config.filename = options.filename; - break; - case REPORTPATH: - if (typeof options.path !== 'string') - throw new ERR_INVALID_ARG_TYPE('path', 'String', options.path); - config.path = options.path; - break; - case REPORTVERBOSE: - if (typeof options.verbose !== 'string' && - typeof options.verbose !== 'boolean') { - throw new ERR_INVALID_ARG_TYPE('verbose', - 'Booelan | String' + - ' (true|false|yes|no)', - options.verbose); - } - config.verbose = options.verbose; - break; - } - }); - // Upload this new config to C++ land - nr.syncConfig(config, true); - }, + // The parseOption method returns an array that include + // the indices at which valid params are present. + list.forEach((i) => { + switch (i) { + case REPORTEVENTS: + if (Array.isArray(options.events)) + config.events = options.events; + else + throw new ERR_INVALID_ARG_TYPE('events', + 'Array', + options.events); + break; + case REPORTSIGNAL: + if (typeof options.signal !== 'string') { + throw new ERR_INVALID_ARG_TYPE('signal', + 'String', + options.signal); + } + process.removeListener(config.signal, handleSignal); + if (config.events.includes('signal')) + process.on(options.signal, handleSignal); + config.signal = options.signal; + break; + case REPORTFILENAME: + if (typeof options.filename !== 'string') { + throw new ERR_INVALID_ARG_TYPE('filename', + 'String', + options.filename); + } + config.filename = options.filename; + break; + case REPORTPATH: + if (typeof options.path !== 'string') + throw new ERR_INVALID_ARG_TYPE('path', 'String', options.path); + config.path = options.path; + break; + case REPORTVERBOSE: + if (typeof options.verbose !== 'string' && + typeof options.verbose !== 'boolean') { + throw new ERR_INVALID_ARG_TYPE('verbose', + 'Booelan | String' + + ' (true|false|yes|no)', + options.verbose); + } + config.verbose = options.verbose; + break; + } + }); + // Upload this new config to C++ land + nr.syncConfig(config, true); + }, - triggerReport(file, err) { - emitExperimentalWarning('report'); - if (err == null) { - if (file == null) { - return nr.triggerReport(new ERR_SYNTHETIC().stack); - } - if (typeof file !== 'string') - throw new ERR_INVALID_ARG_TYPE('file', 'String', file); - return nr.triggerReport(file, new ERR_SYNTHETIC().stack); + triggerReport(file, err) { + emitExperimentalWarning('report'); + if (err == null) { + if (file == null) { + return nr.triggerReport(new ERR_SYNTHETIC().stack); } - if (typeof err !== 'object') - throw new ERR_INVALID_ARG_TYPE('err', 'Object', err); - if (file == null) - return nr.triggerReport(err.stack); if (typeof file !== 'string') throw new ERR_INVALID_ARG_TYPE('file', 'String', file); - return nr.triggerReport(file, err.stack); - }, - getReport(err) { - emitExperimentalWarning('report'); - if (err == null) { - return nr.getReport(new ERR_SYNTHETIC().stack); - } else if (typeof err !== 'object') { - throw new ERR_INVALID_ARG_TYPE('err', 'Object', err); - } else { - return nr.getReport(err.stack); - } + return nr.triggerReport(file, new ERR_SYNTHETIC().stack); + } + if (typeof err !== 'object') + throw new ERR_INVALID_ARG_TYPE('err', 'Object', err); + if (file == null) + return nr.triggerReport(err.stack); + if (typeof file !== 'string') + throw new ERR_INVALID_ARG_TYPE('file', 'String', file); + return nr.triggerReport(file, err.stack); + }, + getReport(err) { + emitExperimentalWarning('report'); + if (err == null) { + return nr.getReport(new ERR_SYNTHETIC().stack); + } else if (typeof err !== 'object') { + throw new ERR_INVALID_ARG_TYPE('err', 'Object', err); + } else { + return nr.getReport(err.stack); } - }; - - // Download the CLI / ENV config into JS land. - nr.syncConfig(config, false); - - function handleSignal(signo) { - if (typeof signo !== 'string') - signo = config.signal; - nr.onUserSignal(signo); } +}; - if (config.events.includes('signal')) { - process.on(config.signal, handleSignal); - } +function handleSignal(signo) { + if (typeof signo !== 'string') + signo = config.signal; + nr.onUserSignal(signo); +} - function parseOptions(obj) { - const list = []; - if (obj == null) - return list; - if (obj.events != null) - list.push(REPORTEVENTS); - if (obj.signal != null) - list.push(REPORTSIGNAL); - if (obj.filename != null) - list.push(REPORTFILENAME); - if (obj.path != null) - list.push(REPORTPATH); - if (obj.verbose != null) - list.push(REPORTVERBOSE); +function parseOptions(obj) { + const list = []; + if (obj == null) return list; - } - process.report = report; + if (obj.events != null) + list.push(REPORTEVENTS); + if (obj.signal != null) + list.push(REPORTSIGNAL); + if (obj.filename != null) + list.push(REPORTFILENAME); + if (obj.path != null) + list.push(REPORTPATH); + if (obj.verbose != null) + list.push(REPORTVERBOSE); + return list; +} + +module.exports = { + config, + handleSignal, + report, + syncConfig: nr.syncConfig };