From 7925adc499a96b0762743e394cb380de5eca09dc Mon Sep 17 00:00:00 2001 From: James M Snell Date: Thu, 26 Oct 2017 21:34:15 -0700 Subject: [PATCH 1/2] src: remove throws in set/getHiddenValue These are internal only utility functions, CHECK instead of throw --- lib/internal/util.js | 19 +++++++----- lib/repl.js | 3 +- src/node_util.cc | 14 +++------ test/parallel/test-util-internal.js | 46 ++++++----------------------- 4 files changed, 27 insertions(+), 55 deletions(-) diff --git a/lib/internal/util.js b/lib/internal/util.js index 4d1fcb3a608890..bcee867a8aba6e 100644 --- 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 index d4b95bf3a67f45..ef69d37382a2eb 100644 --- a/lib/repl.js +++ b/lib/repl.js @@ -287,7 +287,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 index cf26eca692e6ed..647b22087c1142 100644 --- a/src/node_util.cc +++ b/src/node_util.cc @@ -101,11 +101,8 @@ inline Local IndexToPrivateSymbol(Environment* env, uint32_t index) { static void GetHiddenValue(const FunctionCallbackInfo& 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 obj = args[0].As(); auto index = args[1]->Uint32Value(env->context()).FromJust(); @@ -118,11 +115,8 @@ static void GetHiddenValue(const FunctionCallbackInfo& args) { static void SetHiddenValue(const FunctionCallbackInfo& 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 obj = args[0].As(); auto index = args[1]->Uint32Value(env->context()).FromJust(); diff --git a/test/parallel/test-util-internal.js b/test/parallel/test-util-internal.js index 63e782f15de1e8..38022f79bb4f38 100644 --- 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), + 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)); From ca5620eb2f2b00a0ce83a69e4089ad171ab66a4e Mon Sep 17 00:00:00 2001 From: James M Snell Date: Sun, 29 Oct 2017 17:38:14 -0700 Subject: [PATCH 2/2] [Squash] nit --- test/parallel/test-util-internal.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/parallel/test-util-internal.js b/test/parallel/test-util-internal.js index 38022f79bb4f38..5fda104baf19f3 100644 --- a/test/parallel/test-util-internal.js +++ b/test/parallel/test-util-internal.js @@ -11,7 +11,7 @@ const { arrow_message_private_symbol: kArrowMessagePrivateSymbolIndex } = process.binding('util'); -assert.deepStrictEqual( +assert.strictEqual( getHiddenValue({}, kArrowMessagePrivateSymbolIndex), undefined);