report: refactor report option validation

PR-URL: #25990 Reviewed-By: Richard Lau <riclau@uk.ibm.com> Reviewed-By: Anna Henningsen <anna@addaleax.net>
nodejs · Feb 10, 2019 · c8ceece · c8ceece
1 parent f40e0fc
commit c8ceece
Showing 1 changed file with 50 additions and 83 deletions.
diff --git a/lib/internal/process/report.js b/lib/internal/process/report.js
@@ -1,17 +1,13 @@
 'use strict';
-
-const { emitExperimentalWarning } = require('internal/util');
+const {
+  convertToValidSignal,
+  emitExperimentalWarning
+} = require('internal/util');
 const {
   ERR_INVALID_ARG_TYPE,
   ERR_SYNTHETIC
 } = require('internal/errors').codes;
 
-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.
@@ -35,68 +31,56 @@ let config = {
 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);
+    const previousConfig = config;
+    const newConfig = {};
 
-    // 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 = {};
+    if (options === null || typeof options !== 'object')
+      options = {};
 
-    // 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);
-  },
+    if (Array.isArray(options.events))
+      newConfig.events = options.events.slice();
+    else if (options.events === undefined)
+      newConfig.events = [];
+    else
+      throw new ERR_INVALID_ARG_TYPE('events', 'Array', options.events);
+
+    if (typeof options.filename === 'string')
+      newConfig.filename = options.filename;
+    else if (options.filename === undefined)
+      newConfig.filename = '';
+    else
+      throw new ERR_INVALID_ARG_TYPE('filename', 'string', options.filename);
+
+    if (typeof options.path === 'string')
+      newConfig.path = options.path;
+    else if (options.path === undefined)
+      newConfig.path = '';
+    else
+      throw new ERR_INVALID_ARG_TYPE('path', 'string', options.path);
 
+    if (typeof options.verbose === 'boolean')
+      newConfig.verbose = options.verbose;
+    else if (options.verbose === undefined)
+      newConfig.verbose = false;
+    else
+      throw new ERR_INVALID_ARG_TYPE('verbose', 'boolean', options.verbose);
 
+    if (typeof options.signal === 'string')
+      newConfig.signal = convertToValidSignal(options.signal);
+    else if (options.signal === undefined)
+      newConfig.signal = 'SIGUSR2';
+    else
+      throw new ERR_INVALID_ARG_TYPE('signal', 'string', options.signal);
+
+    if (previousConfig.signal)
+      process.removeListener(previousConfig.signal, handleSignal);
+
+    if (newConfig.events.includes('signal'))
+      process.on(newConfig.signal, handleSignal);
+
+    config = newConfig;
+    nr.syncConfig(config, true);
+  },
   triggerReport(file, err) {
     emitExperimentalWarning('report');
     if (err == null) {
@@ -133,23 +117,6 @@ function handleSignal(signo) {
   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);
-  return list;
-}
-
 module.exports = {
   config,
   handleSignal,