report: refactor triggerReport()

This commit fixes the triggerReport() argument validation. The existing test is also updated, as it was not passing the Error object to triggerReport(). PR-URL: #26268 Reviewed-By: Richard Lau <[email protected]> Reviewed-By: Ruben Bridgewater <[email protected]> Reviewed-By: James M Snell <[email protected]>
nodejs · Mar 1, 2019 · 8e2cc5e · 8e2cc5e
1 parent b0c310d
commit 8e2cc5e
Show file tree

Hide file tree

Showing 3 changed files with 17 additions and 20 deletions.
diff --git a/lib/internal/process/report.js b/lib/internal/process/report.js
@@ -75,20 +75,18 @@ const report = {
   },
   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);
-    }
-    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')
+
+    if (typeof file === 'object' && file !== null) {
+      err = file;
+      file = undefined;
+    } else if (file !== undefined && typeof file !== 'string') {
       throw new ERR_INVALID_ARG_TYPE('file', 'String', file);
+    } else if (err === undefined) {
+      err = new ERR_SYNTHETIC();
+    } else if (err === null || typeof err !== 'object') {
+      throw new ERR_INVALID_ARG_TYPE('err', 'Object', err);
+    }
+
     return nr.triggerReport(file, err.stack);
   },
   getReport(err) {

diff --git a/src/node_report_module.cc b/src/node_report_module.cc
@@ -48,12 +48,11 @@ void TriggerReport(const FunctionCallbackInfo<Value>& info) {
   std::string filename;
   Local<String> stackstr;
 
-  if (info.Length() == 1) {
-    stackstr = info[0].As<String>();
-  } else {
+  CHECK_EQ(info.Length(), 2);
+  stackstr = info[1].As<String>();
+
+  if (info[0]->IsString())
     filename = *String::Utf8Value(isolate, info[0]);
-    stackstr = info[1].As<String>();
-  }
 
   filename = TriggerNodeReport(
       isolate, env, "JavaScript API", __func__, filename, stackstr);

diff --git a/test/node-report/test-api-pass-error.js b/test/node-report/test-api-pass-error.js
@@ -7,8 +7,8 @@ const assert = require('assert');
 if (process.argv[2] === 'child') {
   try {
     throw new Error('Testing error handling');
-  } catch {
-    process.report.triggerReport();
+  } catch (err) {
+    process.report.triggerReport(err);
   }
 } else {
   const helper = require('../common/report.js');