vm: migrate isContext to internal/errors

PR-URL: #19268 Refs: #18106 Reviewed-By: Tiancheng "Timothy" Gu <timothygu99@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Gus Caplan <me@gus.host> Reviewed-By: Jon Moss <me@jonathanmoss.me> Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Yuta Hiroto <hello@hiroppy.me> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
nodejs · Mar 22, 2018 · 49b2969 · 49b2969
1 parent 4e1f090
commit 49b2969
Show file tree

Hide file tree

Showing 5 changed files with 50 additions and 17 deletions.
diff --git 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
@@ -276,10 +276,8 @@ void ContextifyContext::MakeContext(const FunctionCallbackInfo<Value>& args) {
 void ContextifyContext::IsContext(const FunctionCallbackInfo<Value>& args) {
   Environment* env = Environment::GetCurrent(args);
 
-  if (!args[0]->IsObject()) {
-    env->ThrowTypeError("sandbox must be an object");
-    return;
-  }
+  CHECK(args[0]->IsObject());
+
   Local<Object> sandbox = args[0].As<Object>();
 
   Maybe<bool> result =

diff --git 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
@@ -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
@@ -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);