From 49b2969ef4731ad4a3b63476d80ce31075526613 Mon Sep 17 00:00:00 2001 From: dustinnewman98 Date: Thu, 1 Mar 2018 22:11:55 -0800 Subject: [PATCH] vm: migrate isContext to internal/errors PR-URL: https://github.com/nodejs/node/pull/19268 Refs: https://github.com/nodejs/node/issues/18106 Reviewed-By: Tiancheng "Timothy" Gu Reviewed-By: James M Snell Reviewed-By: Gus Caplan Reviewed-By: Jon Moss Reviewed-By: Joyee Cheung Reviewed-By: Luigi Pinca Reviewed-By: Yuta Hiroto Reviewed-By: Colin Ihrig Reviewed-By: Ruben Bridgewater --- lib/vm.js | 20 +++++++++++++++++-- src/node_contextify.cc | 6 ++---- test/parallel/test-vm-context.js | 9 ++++++--- test/parallel/test-vm-create-context-arg.js | 10 ++++++---- test/parallel/test-vm-is-context.js | 22 +++++++++++++++++---- 5 files changed, 50 insertions(+), 17 deletions(-) diff --git a/lib/vm.js b/lib/vm.js index 5266dbd29fbf8e..ca7b6f33960d2c 100644 --- a/lib/vm.js +++ b/lib/vm.js @@ -26,10 +26,13 @@ const { kParsingContext, makeContext, - isContext, + isContext: _isContext, } = process.binding('contextify'); -const { ERR_INVALID_ARG_TYPE } = require('internal/errors').codes; +const { + ERR_INVALID_ARG_TYPE, + ERR_MISSING_ARGS +} = require('internal/errors').codes; // The binding provides a few useful primitives: // - Script(code, { filename = "evalmachine.anonymous", @@ -119,6 +122,19 @@ function getContextOptions(options) { return {}; } +function isContext(sandbox) { + if (arguments.length < 1) { + throw new ERR_MISSING_ARGS('sandbox'); + } + + if (typeof sandbox !== 'object' && typeof sandbox !== 'function' || + sandbox === null) { + throw new ERR_INVALID_ARG_TYPE('sandbox', 'object', sandbox); + } + + return _isContext(sandbox); +} + let defaultContextNameIndex = 1; function createContext(sandbox, options) { if (sandbox === undefined) { diff --git a/src/node_contextify.cc b/src/node_contextify.cc index a7f46f61269e2b..91df37b37d7d29 100644 --- a/src/node_contextify.cc +++ b/src/node_contextify.cc @@ -276,10 +276,8 @@ void ContextifyContext::MakeContext(const FunctionCallbackInfo& args) { void ContextifyContext::IsContext(const FunctionCallbackInfo& args) { Environment* env = Environment::GetCurrent(args); - if (!args[0]->IsObject()) { - env->ThrowTypeError("sandbox must be an object"); - return; - } + CHECK(args[0]->IsObject()); + Local sandbox = args[0].As(); Maybe result = diff --git a/test/parallel/test-vm-context.js b/test/parallel/test-vm-context.js index 368a40cbd42c03..fd6a66f3927a0e 100644 --- a/test/parallel/test-vm-context.js +++ b/test/parallel/test-vm-context.js @@ -20,7 +20,7 @@ // USE OR OTHER DEALINGS IN THE SOFTWARE. 'use strict'; -require('../common'); +const common = require('../common'); const assert = require('assert'); const vm = require('vm'); @@ -44,9 +44,12 @@ assert.strictEqual(3, context.foo); assert.strictEqual('lala', context.thing); // Issue GH-227: -assert.throws(() => { +common.expectsError(() => { vm.runInNewContext('', null, 'some.js'); -}, /^TypeError: sandbox must be an object$/); +}, { + code: 'ERR_INVALID_ARG_TYPE', + type: TypeError +}); // Issue GH-1140: // Test runInContext signature diff --git a/test/parallel/test-vm-create-context-arg.js b/test/parallel/test-vm-create-context-arg.js index 2bcdc8573fe311..d9e017d0f9173a 100644 --- a/test/parallel/test-vm-create-context-arg.js +++ b/test/parallel/test-vm-create-context-arg.js @@ -20,13 +20,15 @@ // USE OR OTHER DEALINGS IN THE SOFTWARE. 'use strict'; -require('../common'); -const assert = require('assert'); +const common = require('../common'); const vm = require('vm'); -assert.throws(function() { +common.expectsError(() => { vm.createContext('string is not supported'); -}, /^TypeError: sandbox must be an object$/); +}, { + code: 'ERR_INVALID_ARG_TYPE', + type: TypeError +}); // Should not throw. vm.createContext({ a: 1 }); diff --git a/test/parallel/test-vm-is-context.js b/test/parallel/test-vm-is-context.js index 6c3b783c5e9c33..a762622c67a594 100644 --- a/test/parallel/test-vm-is-context.js +++ b/test/parallel/test-vm-is-context.js @@ -20,13 +20,27 @@ // USE OR OTHER DEALINGS IN THE SOFTWARE. 'use strict'; -require('../common'); +const common = require('../common'); const assert = require('assert'); const vm = require('vm'); -assert.throws(function() { - vm.isContext('string is not supported'); -}, /^TypeError: sandbox must be an object$/); +for (const valToTest of [ + 'string', null, undefined, 8.9, Symbol('sym'), true +]) { + common.expectsError(() => { + vm.isContext(valToTest); + }, { + code: 'ERR_INVALID_ARG_TYPE', + type: TypeError + }); +} + +common.expectsError(() => { + vm.isContext(); +}, { + code: 'ERR_MISSING_ARGS', + type: TypeError +}); assert.strictEqual(vm.isContext({}), false); assert.strictEqual(vm.isContext([]), false);