VM aborts when error thrown when in property setter

[aherlihy]

• Version: v12.14.1
• Platform: Recreated on Catalina 10.15.4 and Windows 10 x64
• Subsystem:

What steps will reproduce the bug?

const repl = require('repl');
const r = repl.start('> ');
Object.defineProperty(r.context, 'db', {
  set: (val) => {
    throw new Error('test error');
  }
});

How often does it reproduce? Is there a required condition?

Every time

What is the expected behavior?

The error that is thrown should be reported in the default eval function, and it should be catchable.

What do you see instead?

FATAL ERROR: v8::FromJust Maybe value is Nothing.
 1: 0x10007f231 node::Abort() [/Users/anna/.nvm/versions/node/v12.14.1/bin/node]
 2: 0x10007f3b5 node::OnFatalError(char const*, char const*) [/Users/anna/.nvm/versions/node/v12.14.1/bin/node]
 3: 0x100178f00 v8::V8::FromJustIsNothing() [/Users/anna/.nvm/versions/node/v12.14.1/bin/node]
 4: 0x100399618 v8::internal::PropertyCallbackArguments::CallNamedSetter(v8::internal::Handle<v8::internal::InterceptorInfo>, v8::internal::Handle<v8::internal::Name>, v8::internal::Handle<v8::internal::Object>) [/Users/anna/.nvm/versions/node/v12.14.1/bin/node]
 5: 0x1004aaa32 v8::internal::(anonymous namespace)::SetPropertyWithInterceptorInternal(v8::internal::LookupIterator*, v8::internal::Handle<v8::internal::InterceptorInfo>, v8::Maybe<v8::internal::ShouldThrow>, v8::internal::Handle<v8::internal::Object>) [/Users/anna/.nvm/versions/node/v12.14.1/bin/node]
 6: 0x1004ef1ab v8::internal::Object::SetPropertyInternal(v8::internal::LookupIterator*, v8::internal::Handle<v8::internal::Object>, v8::Maybe<v8::internal::ShouldThrow>, v8::internal::StoreOrigin, bool*) [/Users/anna/.nvm/versions/node/v12.14.1/bin/node]
 7: 0x1004eefe8 v8::internal::Object::SetProperty(v8::internal::LookupIterator*, v8::internal::Handle<v8::internal::Object>, v8::internal::StoreOrigin, v8::Maybe<v8::internal::ShouldThrow>) [/Users/anna/.nvm/versions/node/v12.14.1/bin/node]
 8: 0x10038ce16 v8::internal::StoreIC::Store(v8::internal::Handle<v8::internal::Object>, v8::internal::Handle<v8::internal::Name>, v8::internal::Handle<v8::internal::Object>, v8::internal::StoreOrigin) [/Users/anna/.nvm/versions/node/v12.14.1/bin/node]
 9: 0x10038c725 v8::internal::StoreGlobalIC::Store(v8::internal::Handle<v8::internal::Name>, v8::internal::Handle<v8::internal::Object>) [/Users/anna/.nvm/versions/node/v12.14.1/bin/node]
10: 0x100392436 v8::internal::Runtime_StoreGlobalICNoFeedback_Miss(int, unsigned long*, v8::internal::Isolate*) [/Users/anna/.nvm/versions/node/v12.14.1/bin/node]
11: 0x1009311f9 Builtins_CEntry_Return1_DontSaveFPRegs_ArgvOnStack_NoBuiltinExit [/Users/anna/.nvm/versions/node/v12.14.1/bin/node]
12: 0x100985e16 Builtins_StaGlobalHandler [/Users/anna/.nvm/versions/node/v12.14.1/bin/node]
Abort trap: 6

Additional information

We need to be able to throw an error if a user tries to assign a context value to a disallowed type.

[addaleax]

Minimal reproduction:

const vm = require('vm');
const ctx = vm.createContext();

Object.defineProperty(ctx, 'db', {
  set: (val) => {
    throw new Error('test error');
  }
});

vm.runInContext('db = 42', ctx);

[jasnell]

Also confirmed to be a problem in 14.3 and master (not only 12.x)

[lrlna]

If #33808 lands, would it be possible to backport the fix over to a 12.x? Or would it only go in the latest?

[devsnek]

@lrlna it should be able to be backported

[lrlna]

@devsnek yay, thanks!