src: remove throws in set/getHiddenValue

These are internal only utility functions, CHECK instead of throw PR-URL: #16544 Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Anatoli Papirovski <apapirovski@mac.com> Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
nodejs · Nov 2, 2017 · 3d20190 · 3d20190
1 parent 841e305
commit 3d20190
Show file tree

Hide file tree

Showing 4 changed files with 28 additions and 56 deletions.
diff --git a/lib/internal/util.js b/lib/internal/util.js
@@ -1,13 +1,18 @@
 'use strict';
 
 const errors = require('internal/errors');
-const binding = process.binding('util');
 const { signals } = process.binding('constants').os;
 
-const { createPromise, promiseResolve, promiseReject } = binding;
+const {
+  createPromise,
+  getHiddenValue,
+  promiseResolve,
+  promiseReject,
+  setHiddenValue,
+  arrow_message_private_symbol: kArrowMessagePrivateSymbolIndex,
+  decorated_private_symbol: kDecoratedPrivateSymbolIndex
+} = process.binding('util');
 
-const kArrowMessagePrivateSymbolIndex = binding['arrow_message_private_symbol'];
-const kDecoratedPrivateSymbolIndex = binding['decorated_private_symbol'];
 const noCrypto = !process.versions.openssl;
 
 function isError(e) {
@@ -66,14 +71,14 @@ function deprecate(fn, msg, code) {
 
 function decorateErrorStack(err) {
   if (!(isError(err) && err.stack) ||
-      binding.getHiddenValue(err, kDecoratedPrivateSymbolIndex) === true)
+      getHiddenValue(err, kDecoratedPrivateSymbolIndex) === true)
     return;
 
-  const arrow = binding.getHiddenValue(err, kArrowMessagePrivateSymbolIndex);
+  const arrow = getHiddenValue(err, kArrowMessagePrivateSymbolIndex);
 
   if (arrow) {
     err.stack = arrow + err.stack;
-    binding.setHiddenValue(err, kDecoratedPrivateSymbolIndex, true);
+    setHiddenValue(err, kDecoratedPrivateSymbolIndex, true);
   }
 }
 

diff --git a/lib/repl.js b/lib/repl.js
@@ -289,7 +289,8 @@ function REPLServer(prompt,
     const top = replMap.get(self);
     const pstrace = Error.prepareStackTrace;
     Error.prepareStackTrace = prepareStackTrace(pstrace);
-    internalUtil.decorateErrorStack(e);
+    if (typeof e === 'object')
+      internalUtil.decorateErrorStack(e);
     Error.prepareStackTrace = pstrace;
     const isError = internalUtil.isError(e);
     if (e instanceof SyntaxError && e.stack) {

diff --git a/src/node_util.cc b/src/node_util.cc
@@ -101,11 +101,8 @@ inline Local<Private> IndexToPrivateSymbol(Environment* env, uint32_t index) {
 static void GetHiddenValue(const FunctionCallbackInfo<Value>& args) {
   Environment* env = Environment::GetCurrent(args);
 
-  if (!args[0]->IsObject())
-    return env->ThrowTypeError("obj must be an object");
-
-  if (!args[1]->IsUint32())
-    return env->ThrowTypeError("index must be an uint32");
+  CHECK(args[0]->IsObject());
+  CHECK(args[1]->IsUint32());
 
   Local<Object> obj = args[0].As<Object>();
   auto index = args[1]->Uint32Value(env->context()).FromJust();
@@ -118,11 +115,8 @@ static void GetHiddenValue(const FunctionCallbackInfo<Value>& args) {
 static void SetHiddenValue(const FunctionCallbackInfo<Value>& args) {
   Environment* env = Environment::GetCurrent(args);
 
-  if (!args[0]->IsObject())
-    return env->ThrowTypeError("obj must be an object");
-
-  if (!args[1]->IsUint32())
-    return env->ThrowTypeError("index must be an uint32");
+  CHECK(args[0]->IsObject());
+  CHECK(args[1]->IsUint32());
 
   Local<Object> obj = args[0].As<Object>();
   auto index = args[1]->Uint32Value(env->context()).FromJust();

diff --git a/test/parallel/test-util-internal.js b/test/parallel/test-util-internal.js
@@ -5,50 +5,22 @@ require('../common');
 const assert = require('assert');
 const fixtures = require('../common/fixtures');
 
-const binding = process.binding('util');
-const kArrowMessagePrivateSymbolIndex = binding['arrow_message_private_symbol'];
+const {
+  getHiddenValue,
+  setHiddenValue,
+  arrow_message_private_symbol: kArrowMessagePrivateSymbolIndex
+} = process.binding('util');
 
-function getHiddenValue(obj, index) {
-  return function() {
-    binding.getHiddenValue(obj, index);
-  };
-}
-
-function setHiddenValue(obj, index, val) {
-  return function() {
-    binding.setHiddenValue(obj, index, val);
-  };
-}
-
-const errMessageObj = /obj must be an object/;
-const errMessageIndex = /index must be an uint32/;
-
-assert.throws(getHiddenValue(), errMessageObj);
-assert.throws(getHiddenValue(null, 'foo'), errMessageObj);
-assert.throws(getHiddenValue(undefined, 'foo'), errMessageObj);
-assert.throws(getHiddenValue('bar', 'foo'), errMessageObj);
-assert.throws(getHiddenValue(85, 'foo'), errMessageObj);
-assert.throws(getHiddenValue({}), errMessageIndex);
-assert.throws(getHiddenValue({}, null), errMessageIndex);
-assert.throws(getHiddenValue({}, []), errMessageIndex);
-assert.deepStrictEqual(
-  binding.getHiddenValue({}, kArrowMessagePrivateSymbolIndex),
+assert.strictEqual(
+  getHiddenValue({}, kArrowMessagePrivateSymbolIndex),
   undefined);
 
-assert.throws(setHiddenValue(), errMessageObj);
-assert.throws(setHiddenValue(null, 'foo'), errMessageObj);
-assert.throws(setHiddenValue(undefined, 'foo'), errMessageObj);
-assert.throws(setHiddenValue('bar', 'foo'), errMessageObj);
-assert.throws(setHiddenValue(85, 'foo'), errMessageObj);
-assert.throws(setHiddenValue({}), errMessageIndex);
-assert.throws(setHiddenValue({}, null), errMessageIndex);
-assert.throws(setHiddenValue({}, []), errMessageIndex);
 const obj = {};
 assert.strictEqual(
-  binding.setHiddenValue(obj, kArrowMessagePrivateSymbolIndex, 'bar'),
+  setHiddenValue(obj, kArrowMessagePrivateSymbolIndex, 'bar'),
   true);
 assert.strictEqual(
-  binding.getHiddenValue(obj, kArrowMessagePrivateSymbolIndex),
+  getHiddenValue(obj, kArrowMessagePrivateSymbolIndex),
   'bar');
 
 let arrowMessage;
@@ -57,7 +29,7 @@ try {
   require(fixtures.path('syntax', 'bad_syntax'));
 } catch (err) {
   arrowMessage =
-      binding.getHiddenValue(err, kArrowMessagePrivateSymbolIndex);
+      getHiddenValue(err, kArrowMessagePrivateSymbolIndex);
 }
 
 assert(/bad_syntax\.js:1/.test(arrowMessage));