[proxies] Improve error messages.

This CL makes proxy-related error messages more accurate and verbose. (Exception: those used in deprecated functions in v8natives.js.) Some of the old error messages were simply wrong. On the side, fix ShouldThrow semantics of JSProxy::SetPrototype and JSProxy::DefineOwnProperty. R=cbruni@chromium.org, jkummerow@chromium.org BUG=v8:1543 LOG=n Review URL: https://codereview.chromium.org/1527583002 Cr-Commit-Position: refs/heads/master@{#32836}
2015-12-14 05:44:38 -08:00 · 2015-12-14 05:44:38 -08:00 · e960636571
commit e960636571
parent fd781bcc84
8 changed files with 185 additions and 125 deletions
--- a/src/builtins.cc
+++ b/src/builtins.cc
@ -1829,8 +1829,8 @@ namespace {
 MaybeHandle<JSProxy> ProxyCreate(Isolate* isolate, Handle<Object> target,
                                 Handle<Object> handler) {
  if (!target->IsJSReceiver()) {
-    THROW_NEW_ERROR(
-        isolate, NewTypeError(MessageTemplate::kProxyTargetNonObject), JSProxy);
+    THROW_NEW_ERROR(isolate, NewTypeError(MessageTemplate::kProxyNonObject),
+                    JSProxy);
  }
  if (target->IsJSProxy() && JSProxy::cast(*target)->IsRevoked()) {
    THROW_NEW_ERROR(isolate,
@ -1838,8 +1838,7 @@ MaybeHandle<JSProxy> ProxyCreate(Isolate* isolate, Handle<Object> target,
                    JSProxy);
  }
  if (!handler->IsJSReceiver()) {
-    THROW_NEW_ERROR(isolate,
-                    NewTypeError(MessageTemplate::kProxyHandlerNonObject),
+    THROW_NEW_ERROR(isolate, NewTypeError(MessageTemplate::kProxyNonObject),
                    JSProxy);
  }
  if (handler->IsJSProxy() && JSProxy::cast(*handler)->IsRevoked()) {
--- a/src/js/proxy.js
+++ b/src/js/proxy.js
@ -36,8 +36,7 @@ function ProxyEnumerate(trap, handler, target) {
  var trap_result = %_Call(trap, handler, target);
  // 8. If Type(trapResult) is not Object, throw a TypeError exception.
  if (!IS_SPEC_OBJECT(trap_result)) {
-    throw MakeTypeError(kProxyTrapReturnedNonObject, handler, trap_result,
-                        "enumerate");
+    throw MakeTypeError(kProxyEnumerateNonObject);
  }
  // 9. Return trapResult.
  var result = [];
@ -46,8 +45,7 @@ function ProxyEnumerate(trap, handler, target) {
    // Not yet spec'ed as of 2015-11-25, but will be spec'ed soon:
    // If the iterator returns a non-string value, throw a TypeError.
    if (!IS_STRING(key)) {
-      throw MakeTypeError(kProxyTrapReturned, handler, "non-String",
-                          "enumerate");
+      throw MakeTypeError(kProxyEnumerateNonString);
    }
    result.push(key);
  }
--- a/src/js/v8natives.js
+++ b/src/js/v8natives.js
@ -475,11 +475,11 @@ function GetTrap(handler, name, defaultTrap) {
  var trap = handler[name];
  if (IS_UNDEFINED(trap)) {
    if (IS_UNDEFINED(defaultTrap)) {
-      throw MakeTypeError(kProxyHandlerTrapMissing, handler, name);
+      throw MakeTypeError(kIllegalInvocation);
    }
    trap = defaultTrap;
  } else if (!IS_CALLABLE(trap)) {
-    throw MakeTypeError(kProxyHandlerTrapMustBeCallable, handler, name);
+    throw MakeTypeError(kIllegalInvocation);
  }
  return trap;
 }
@ -510,8 +510,7 @@ function GetOwnPropertyJS(obj, v) {
    if (IS_UNDEFINED(descriptor)) return descriptor;
    var desc = ToCompletePropertyDescriptor(descriptor);
    if (!desc.isConfigurable()) {
-      throw MakeTypeError(kProxyPropNotConfigurable,
-                          handler, p, "getOwnPropertyDescriptor");
+      throw MakeTypeError(kIllegalInvocation);
    }
    return desc;
  }
@ -543,8 +542,7 @@ function DefineProxyProperty(obj, p, attributes, should_throw) {
  var result = CallTrap2(handler, "defineProperty", UNDEFINED, p, attributes);
  if (!result) {
    if (should_throw) {
-      throw MakeTypeError(kProxyTrapReturned,
-                          handler, "false", "defineProperty");
+      throw MakeTypeError(kIllegalInvocation);
    } else {
      return false;
    }
@ -782,7 +780,7 @@ function ObjectGetPrototypeOf(obj) {
  return %_GetPrototype(TO_OBJECT(obj));
 }

-// ES6 section 19.1.2.19.
+// ES6 section 19.1.2.18.
 function ObjectSetPrototypeOf(obj, proto) {
  CHECK_OBJECT_COERCIBLE(obj, "Object.setPrototypeOf");

--- a/src/messages.h
+++ b/src/messages.h
@ -185,46 +185,90 @@ class CallSite {
  T(ProtoObjectOrNull, "Object prototype may only be an Object or null: %")    \
  T(PrototypeParentNotAnObject,                                                \
    "Class extends value does not have valid prototype property %")            \
-  T(ProxyDeletePropertyViolatesInvariant,                                      \
-    "Trap 'deleteProperty' returned true but property '%' is not configurable" \
-    " in the proxy target")                                                    \
-  T(ProxyHandlerNonObject, "Cannot create proxy with non-object as handler")   \
-  T(ProxyHandlerTrapMissing, "Proxy handler % has no '%' trap")                \
-  T(ProxyHandlerTrapMustBeCallable,                                            \
-    "Proxy handler % has non-callable '%' trap")                               \
-  T(ProxySetPrototypeFailed,                                                   \
-    "Proxy handler returned false when setting prototype '%'")                 \
-  T(ProxyNonObjectPropNames, "Trap '%' returned non-object %")                 \
-  T(ProxyPreventExtensionsViolatesInvariant,                                   \
-    "Trap 'preventExtensions' returned true but the proxy's target is "        \
-    "extensible")                                                              \
-  T(ProxyPropNotConfigurable,                                                  \
-    "Proxy handler % returned non-configurable descriptor for property '%' "   \
-    "from '%' trap")                                                           \
-  T(ProxyRepeatedPropName, "Trap '%' returned repeated property name '%'")     \
+  T(ProxyConstructNonObject,                                                   \
+    "'construct' on proxy: trap returned non-object ('%')")                    \
+  T(ProxyDefinePropertyNonConfigurable,                                        \
+    "'defineProperty' on proxy: trap returned truish for defining "            \
+    "non-configurable property '%' which is either non-existant or "           \
+    "configurable in the proxy target")                                        \
+  T(ProxyDefinePropertyNonExtensible,                                          \
+    "'defineProperty' on proxy: trap returned truish for adding property '%' " \
+    " to the non-extensible proxy target")                                     \
+  T(ProxyDefinePropertyIncompatible,                                           \
+    "'defineProperty' on proxy: trap returned truish for adding property '%' " \
+    " that is incompatible with the existing property in the proxy target")    \
+  T(ProxyDeletePropertyNonConfigurable,                                        \
+    "'deleteProperty' on proxy: trap returned truish for property '%' which "  \
+    "is non-configurable in the proxy target")                                 \
+  T(ProxyEnumerateNonObject, "'enumerate' on proxy: trap returned non-object") \
+  T(ProxyEnumerateNonString,                                                   \
+    "'enumerate' on proxy: trap result includes non-string")                   \
+  T(ProxyGetNonConfigurableData,                                               \
+    "'get' on proxy: property '%' is a read-only and "                         \
+    "non-configurable data property on the proxy target but the proxy "        \
+    "did not return its actual value (expected '%' but got '%')")              \
+  T(ProxyGetNonConfigurableAccessor,                                           \
+    "'get' on proxy: property '%' is a non-configurable accessor "             \
+    "property on the proxy target and does not have a getter function, but "   \
+    "the trap did not return 'undefined' (got '%')")                           \
+  T(ProxyGetOwnPropertyDescriptorIncompatible,                                 \
+    "'getOwnPropertyDescriptor' on proxy: trap returned descriptor for "       \
+    "property '%' that is incompatible with the existing property in the "     \
+    "proxy target")                                                            \
+  T(ProxyGetOwnPropertyDescriptorInvalid,                                      \
+    "'getOwnPropertyDescriptor' on proxy: trap returned neither object nor "   \
+    "undefined for property '%'")                                              \
+  T(ProxyGetOwnPropertyDescriptorNonConfigurable,                              \
+    "'getOwnPropertyDescriptor' on proxy: trap reported non-configurability "  \
+    "for property '%' which is either non-existant or configurable in the "    \
+    "proxy target")                                                            \
+  T(ProxyGetOwnPropertyDescriptorNonExtensible,                                \
+    "'getOwnPropertyDescriptor' on proxy: trap returned undefined for "        \
+    "property '%' which exists in the non-extensible proxy target")            \
+  T(ProxyGetOwnPropertyDescriptorUndefined,                                    \
+    "'getOwnPropertyDescriptor' on proxy: trap returned undefined for "        \
+    "property '%' which is non-configurable in the proxy target")              \
+  T(ProxyGetPrototypeOfInvalid,                                                \
+    "'getPrototypeOf' on proxy: trap returned neither object nor null")        \
+  T(ProxyGetPrototypeOfNonExtensible,                                          \
+    "'getPrototypeOf' on proxy: proxy target is non-extensible but the "       \
+    "trap did not return its actual prototype")                                \
  T(ProxyHandlerOrTargetRevoked,                                               \
-    "Cannot create proxy with a revoked proxy as handler or target")           \
+    "Cannot create proxy with a revoked proxy as target or handler")           \
+  T(ProxyHasNonConfigurable,                                                   \
+    "'has' on proxy: trap returned falsish for property '%' which exists in "  \
+    "the proxy target as non-configurable")                                    \
+  T(ProxyHasNonExtensible,                                                     \
+    "'has' on proxy: trap returned falsish for property '%' but the proxy "    \
+    "target is not extensible")                                                \
+  T(ProxyIsExtensibleInconsistent,                                             \
+    "'isExtensible' on proxy: trap result does not reflect extensibility of "  \
+    "proxy target (which is '%')")                                             \
+  T(ProxyNonObject,                                                            \
+    "Cannot create proxy with a non-object as target or handler")              \
+  T(ProxyOwnKeysMissing,                                                       \
+    "'ownKeys' on proxy: trap result did not include '%'")                     \
+  T(ProxyOwnKeysNonExtensible,                                                 \
+    "'ownKeys' on proxy: trap returned extra keys but proxy target is "        \
+    "non-extensible")                                                          \
+  T(ProxyPreventExtensionsExtensible,                                          \
+    "'preventExtensions' on proxy: trap returned truish but the proxy target " \
+    "is extensible")                                                           \
  T(ProxyRevoked, "Cannot perform '%' on a proxy that has been revoked")       \
-  T(ProxyTargetNotExtensible, "Proxy target is not extensible")                \
-  T(ProxyTargetNonObject, "Proxy target is non-object")                        \
-  T(ProxyTargetPropNotConfigurable,                                            \
-    "Proxy target property '%' is not configurable")                           \
-  T(ProxyTrapConstructMustReturnObject,                                        \
-    "Construct trap must return Object, but got ''%s'.")                       \
-  T(ProxyTrapFunctionExpected,                                                 \
-    "Proxy.createFunction called with non-function for '%' trap")              \
-  T(ProxyTrapOwnKeysResultMustInclude,                                         \
-    "Result of trap 'ownKeys' must include '%'.")                              \
-  T(ProxyTrapReturned, "Proxy handler % returned % from '%' trap")             \
-  T(ProxyTrapReturnedFalseish,                                                 \
-    "Proxy handler % returned false value '%' from '%' trap")                  \
-  T(ProxyTrapReturnedNonObject,                                                \
-    "Proxy handler % returned % from '%' trap instead of an Object")           \
-  T(ProxyTrapDescriptorNonConfigurable,                                        \
-    "Proxy %s trap returned non-configurable PropertyDescriptor for property " \
-    "'%' even though the property on the Proxy target is configurable")        \
-  T(ProxyTrapViolatesInvariant,                                                \
-    "Result of trap '%' is inconsistent with proxy's target")                  \
+  T(ProxySetFrozenData,                                                        \
+    "'set' on proxy: trap returned truish for property '%' which exists in "   \
+    "the proxy target as a non-configurable and non-writable data property "   \
+    "with a different value")                                                  \
+  T(ProxySetFrozenAccessor,                                                    \
+    "'set' on proxy: trap returned truish for property '%' which exists in "   \
+    "the proxy target as a non-configurable and non-writable accessor "        \
+    "property without a setter")                                               \
+  T(ProxySetPrototypeOfNonExtensible,                                          \
+    "'setPrototypeOf' on proxy: trap returned truish for setting a new "       \
+    "prototype on the non-extensible proxy target")                            \
+  T(ProxyTrapReturnedFalsish, "'%' on proxy: trap returned falsish")           \
+  T(ProxyTrapReturnedFalsishFor,                                               \
+    "'%' on proxy: trap returned falsish for property '%'")                    \
  T(RedefineDisallowed, "Cannot redefine property: %")                         \
  T(RedefineExternalArray,                                                     \
    "Cannot redefine a property of an object with external array elements")    \
--- a/src/objects.cc
+++ b/src/objects.cc
@ -799,18 +799,24 @@ MaybeHandle<Object> JSProxy::GetProperty(Isolate* isolate,
                        !target_desc.configurable() &&
                        !target_desc.writable() &&
                        !trap_result->SameValue(*target_desc.value());
+    if (inconsistent) {
+      THROW_NEW_ERROR(
+          isolate, NewTypeError(MessageTemplate::kProxyGetNonConfigurableData,
+                                name, target_desc.value(), trap_result),
+          Object);
+    }
    // 10.b. If IsAccessorDescriptor(targetDesc) and targetDesc.[[Configurable]]
    //       is false and targetDesc.[[Get]] is undefined, then
    // 10.b.i. If trapResult is not undefined, throw a TypeError exception.
-    inconsistent =
-        inconsistent ||
-        (PropertyDescriptor::IsAccessorDescriptor(&target_desc) &&
-         !target_desc.configurable() && target_desc.get()->IsUndefined() &&
-         !trap_result->IsUndefined());
+    inconsistent = PropertyDescriptor::IsAccessorDescriptor(&target_desc) &&
+                   !target_desc.configurable() &&
+                   target_desc.get()->IsUndefined() &&
+                   !trap_result->IsUndefined();
    if (inconsistent) {
      THROW_NEW_ERROR(
          isolate,
-          NewTypeError(MessageTemplate::kProxyTrapViolatesInvariant, trap_name),
+          NewTypeError(MessageTemplate::kProxyGetNonConfigurableAccessor, name,
+                       trap_result),
          Object);
    }
  }
@ -987,8 +993,7 @@ MaybeHandle<Object> JSProxy::GetPrototype(Handle<JSProxy> proxy) {
  // 8. If Type(handlerProto) is neither Object nor Null, throw a TypeError.
  if (!(handler_proto->IsJSReceiver() || handler_proto->IsNull())) {
    THROW_NEW_ERROR(isolate,
-                    NewTypeError(MessageTemplate::kProxyHandlerTrapMissing,
-                                 handler, trap_name),
+                    NewTypeError(MessageTemplate::kProxyGetPrototypeOfInvalid),
                    Object);
  }
  // 9. Let extensibleTarget be ? IsExtensible(target).
@ -1002,10 +1007,10 @@ MaybeHandle<Object> JSProxy::GetPrototype(Handle<JSProxy> proxy) {
                             Object::GetPrototype(isolate, target), Object);
  // 12. If SameValue(handlerProto, targetProto) is false, throw a TypeError.
  if (!handler_proto->SameValue(*target_proto)) {
-    THROW_NEW_ERROR(isolate,
-                    NewTypeError(MessageTemplate::kProxyHandlerTrapMissing,
-                                 handler, trap_name),
-                    Object);
+    THROW_NEW_ERROR(
+        isolate,
+        NewTypeError(MessageTemplate::kProxyGetPrototypeOfNonExtensible),
+        Object);
  }
  // 13. Return handlerProto.
  return handler_proto;
@ -4663,7 +4668,7 @@ Maybe<bool> JSProxy::HasProperty(Isolate* isolate, Handle<JSProxy> proxy,
      //       exception.
      if (!target_desc.configurable()) {
        isolate->Throw(*isolate->factory()->NewTypeError(
-            MessageTemplate::kProxyTargetPropNotConfigurable, name));
+            MessageTemplate::kProxyHasNonConfigurable, name));
        return Nothing<bool>();
      }
      // 9b ii. Let extensibleTarget be ? IsExtensible(target).
@ -4672,7 +4677,7 @@ Maybe<bool> JSProxy::HasProperty(Isolate* isolate, Handle<JSProxy> proxy,
      // 9b iii. If extensibleTarget is false, throw a TypeError exception.
      if (!extensible_target.FromJust()) {
        isolate->Throw(*isolate->factory()->NewTypeError(
-            MessageTemplate::kProxyTargetNotExtensible));
+            MessageTemplate::kProxyHasNonExtensible, name));
        return Nothing<bool>();
      }
    }
@ -4717,8 +4722,8 @@ Maybe<bool> JSProxy::SetProperty(Handle<JSProxy> proxy, Handle<Name> name,
      Nothing<bool>());
  if (!trap_result->BooleanValue()) {
    RETURN_FAILURE(isolate, should_throw,
-                   NewTypeError(MessageTemplate::kProxyTrapReturnedFalseish,
-                                handler, trap_result, trap_name));
+                   NewTypeError(MessageTemplate::kProxyTrapReturnedFalsishFor,
+                                trap_name, name));
  }

  // Enforce the invariant.
@ -4727,15 +4732,21 @@ Maybe<bool> JSProxy::SetProperty(Handle<JSProxy> proxy, Handle<Name> name,
      JSReceiver::GetOwnPropertyDescriptor(isolate, target, name, &target_desc);
  MAYBE_RETURN(owned, Nothing<bool>());
  if (owned.FromJust()) {
-    bool inconsistent =
-        (PropertyDescriptor::IsDataDescriptor(&target_desc) &&
-         !target_desc.configurable() && !target_desc.writable() &&
-         !value->SameValue(*target_desc.value())) ||
-        (PropertyDescriptor::IsAccessorDescriptor(&target_desc) &&
-         !target_desc.configurable() && target_desc.set()->IsUndefined());
+    bool inconsistent = PropertyDescriptor::IsDataDescriptor(&target_desc) &&
+                        !target_desc.configurable() &&
+                        !target_desc.writable() &&
+                        !value->SameValue(*target_desc.value());
    if (inconsistent) {
      isolate->Throw(*isolate->factory()->NewTypeError(
-          MessageTemplate::kProxyTrapViolatesInvariant, trap_name));
+          MessageTemplate::kProxySetFrozenData, name));
+      return Nothing<bool>();
+    }
+    inconsistent = PropertyDescriptor::IsAccessorDescriptor(&target_desc) &&
+                   !target_desc.configurable() &&
+                   target_desc.set()->IsUndefined();
+    if (inconsistent) {
+      isolate->Throw(*isolate->factory()->NewTypeError(
+          MessageTemplate::kProxySetFrozenAccessor, name));
      return Nothing<bool>();
    }
  }
@ -4775,8 +4786,8 @@ Maybe<bool> JSProxy::DeletePropertyOrElement(Handle<JSProxy> proxy,
      Nothing<bool>());
  if (!trap_result->BooleanValue()) {
    RETURN_FAILURE(isolate, should_throw,
-                   NewTypeError(MessageTemplate::kProxyTrapReturnedFalseish,
-                                handler, trap_result, trap_name));
+                   NewTypeError(MessageTemplate::kProxyTrapReturnedFalsishFor,
+                                trap_name, name));
  }

  // Enforce the invariant.
@ -4786,7 +4797,7 @@ Maybe<bool> JSProxy::DeletePropertyOrElement(Handle<JSProxy> proxy,
  MAYBE_RETURN(owned, Nothing<bool>());
  if (owned.FromJust() && !target_desc.configurable()) {
    isolate->Throw(*factory->NewTypeError(
-        MessageTemplate::kProxyDeletePropertyViolatesInvariant, name));
+        MessageTemplate::kProxyDeletePropertyNonConfigurable, name));
    return Nothing<bool>();
  }
  return Just(true);
@ -6255,11 +6266,12 @@ Maybe<bool> JSReceiver::OrdinaryDefineOwnProperty(LookupIterator* it,
 // static
 Maybe<bool> JSReceiver::IsCompatiblePropertyDescriptor(
    Isolate* isolate, bool extensible, PropertyDescriptor* desc,
-    PropertyDescriptor* current, Handle<Name> property_name) {
+    PropertyDescriptor* current, Handle<Name> property_name,
+    ShouldThrow should_throw) {
  // 1. Return ValidateAndApplyPropertyDescriptor(undefined, undefined,
  //    Extensible, Desc, Current).
  return ValidateAndApplyPropertyDescriptor(
-      isolate, NULL, extensible, desc, current, THROW_ON_ERROR, property_name);
+      isolate, NULL, extensible, desc, current, should_throw, property_name);
 }


@ -6829,8 +6841,8 @@ Maybe<bool> JSProxy::DefineOwnProperty(Isolate* isolate, Handle<JSProxy> proxy,
  // 10. If booleanTrapResult is false, return false.
  if (!trap_result_obj->BooleanValue()) {
    RETURN_FAILURE(isolate, should_throw,
-                   NewTypeError(MessageTemplate::kProxyTrapReturnedFalseish,
-                                handler, trap_result_obj, trap_name));
+                   NewTypeError(MessageTemplate::kProxyTrapReturnedFalsishFor,
+                                trap_name, property_name));
  }
  // 11. Let targetDesc be ? target.[[GetOwnProperty]](P).
  PropertyDescriptor target_desc;
@ -6851,30 +6863,33 @@ Maybe<bool> JSProxy::DefineOwnProperty(Isolate* isolate, Handle<JSProxy> proxy,
    // 15a. If extensibleTarget is false, throw a TypeError exception.
    if (!extensible_target) {
      isolate->Throw(*isolate->factory()->NewTypeError(
-          MessageTemplate::kProxyTargetNotExtensible));
+          MessageTemplate::kProxyDefinePropertyNonExtensible, property_name));
      return Nothing<bool>();
    }
    // 15b. If settingConfigFalse is true, throw a TypeError exception.
    if (setting_config_false) {
-      // TODO(jkummerow): Better error message?
      isolate->Throw(*isolate->factory()->NewTypeError(
-          MessageTemplate::kRedefineDisallowed, key));
+          MessageTemplate::kProxyDefinePropertyNonConfigurable, property_name));
      return Nothing<bool>();
    }
  } else {
    // 16. Else targetDesc is not undefined,
    // 16a. If IsCompatiblePropertyDescriptor(extensibleTarget, Desc,
    //      targetDesc) is false, throw a TypeError exception.
-    Maybe<bool> valid = IsCompatiblePropertyDescriptor(
-        isolate, extensible_target, desc, &target_desc, property_name);
+    Maybe<bool> valid =
+        IsCompatiblePropertyDescriptor(isolate, extensible_target, desc,
+                                       &target_desc, property_name, DONT_THROW);
    MAYBE_RETURN(valid, Nothing<bool>());
-    DCHECK(valid.FromJust());
+    if (!valid.FromJust()) {
+      isolate->Throw(*isolate->factory()->NewTypeError(
+          MessageTemplate::kProxyDefinePropertyIncompatible, property_name));
+      return Nothing<bool>();
+    }
    // 16b. If settingConfigFalse is true and targetDesc.[[Configurable]] is
    //      true, throw a TypeError exception.
    if (setting_config_false && target_desc.configurable()) {
-      // TODO(jkummerow): Better error message?
      isolate->Throw(*isolate->factory()->NewTypeError(
-          MessageTemplate::kRedefineDisallowed, key));
+          MessageTemplate::kProxyDefinePropertyNonConfigurable, property_name));
      return Nothing<bool>();
    }
  }
@ -6997,8 +7012,7 @@ Maybe<bool> JSProxy::GetOwnPropertyDescriptor(Isolate* isolate,
  //    TypeError exception.
  if (!trap_result_obj->IsJSReceiver() && !trap_result_obj->IsUndefined()) {
    isolate->Throw(*isolate->factory()->NewTypeError(
-        MessageTemplate::kProxyTrapReturnedFalseish, handler, trap_result_obj,
-        trap_name));
+        MessageTemplate::kProxyGetOwnPropertyDescriptorInvalid, name));
    return Nothing<bool>();
  }
  // 10. Let targetDesc be ? target.[[GetOwnProperty]](P).
@ -7014,7 +7028,7 @@ Maybe<bool> JSProxy::GetOwnPropertyDescriptor(Isolate* isolate,
    //      exception.
    if (!target_desc.configurable()) {
      isolate->Throw(*isolate->factory()->NewTypeError(
-          MessageTemplate::kProxyTargetPropNotConfigurable, name));
+          MessageTemplate::kProxyGetOwnPropertyDescriptorUndefined, name));
      return Nothing<bool>();
    }
    // 11c. Let extensibleTarget be ? IsExtensible(target).
@ -7024,7 +7038,7 @@ Maybe<bool> JSProxy::GetOwnPropertyDescriptor(Isolate* isolate,
    // 11e. If extensibleTarget is false, throw a TypeError exception.
    if (!extensible_target.FromJust()) {
      isolate->Throw(*isolate->factory()->NewTypeError(
-          MessageTemplate::kProxyTargetNotExtensible));
+          MessageTemplate::kProxyGetOwnPropertyDescriptorNonExtensible, name));
      return Nothing<bool>();
    }
    // 11f. Return undefined.
@ -7043,18 +7057,23 @@ Maybe<bool> JSProxy::GetOwnPropertyDescriptor(Isolate* isolate,
  PropertyDescriptor::CompletePropertyDescriptor(isolate, desc);
  // 15. Let valid be IsCompatiblePropertyDescriptor (extensibleTarget,
  //     resultDesc, targetDesc).
-  Maybe<bool> valid = IsCompatiblePropertyDescriptor(
-      isolate, extensible_target.FromJust(), desc, &target_desc, name);
-  // 16. If valid is false, throw a TypeError exception.
+  Maybe<bool> valid =
+      IsCompatiblePropertyDescriptor(isolate, extensible_target.FromJust(),
+                                     desc, &target_desc, name, DONT_THROW);
  MAYBE_RETURN(valid, Nothing<bool>());
-  DCHECK(valid.FromJust());
+  // 16. If valid is false, throw a TypeError exception.
+  if (!valid.FromJust()) {
+    isolate->Throw(*isolate->factory()->NewTypeError(
+        MessageTemplate::kProxyGetOwnPropertyDescriptorIncompatible, name));
+    return Nothing<bool>();
+  }
  // 17. If resultDesc.[[Configurable]] is false, then
  if (!desc->configurable()) {
    // 17a. If targetDesc is undefined or targetDesc.[[Configurable]] is true:
    if (target_desc.is_empty() || target_desc.configurable()) {
      // 17a i. Throw a TypeError exception.
      isolate->Throw(*isolate->factory()->NewTypeError(
-          MessageTemplate::kProxyTrapDescriptorNonConfigurable, trap_name,
+          MessageTemplate::kProxyGetOwnPropertyDescriptorNonConfigurable,
          name));
      return Nothing<bool>();
    }
@ -7337,9 +7356,9 @@ Maybe<bool> JSProxy::PreventExtensions(Handle<JSProxy> proxy,
      Execution::Call(isolate, trap, handler, arraysize(args), args),
      Nothing<bool>());
  if (!trap_result->BooleanValue()) {
-    RETURN_FAILURE(isolate, should_throw,
-                   NewTypeError(MessageTemplate::kProxyTrapReturned, handler,
-                                trap_result, trap_name));
+    RETURN_FAILURE(
+        isolate, should_throw,
+        NewTypeError(MessageTemplate::kProxyTrapReturnedFalsish, trap_name));
  }

  // Enforce the invariant.
@ -7347,7 +7366,7 @@ Maybe<bool> JSProxy::PreventExtensions(Handle<JSProxy> proxy,
  MAYBE_RETURN(target_result, Nothing<bool>());
  if (target_result.FromJust()) {
    isolate->Throw(*factory->NewTypeError(
-        MessageTemplate::kProxyPreventExtensionsViolatesInvariant));
+        MessageTemplate::kProxyPreventExtensionsExtensible));
    return Nothing<bool>();
  }
  return Just(true);
@ -7450,8 +7469,9 @@ Maybe<bool> JSProxy::IsExtensible(Handle<JSProxy> proxy) {
  Maybe<bool> target_result = JSReceiver::IsExtensible(target);
  MAYBE_RETURN(target_result, Nothing<bool>());
  if (target_result.FromJust() != trap_result->BooleanValue()) {
-    isolate->Throw(*factory->NewTypeError(
-        MessageTemplate::kProxyTrapViolatesInvariant, trap_name));
+    isolate->Throw(
+        *factory->NewTypeError(MessageTemplate::kProxyIsExtensibleInconsistent,
+                               factory->ToBoolean(target_result.FromJust())));
    return Nothing<bool>();
  }
  return target_result;
@ -8578,8 +8598,7 @@ Maybe<bool> JSProxy::OwnPropertyKeys(Isolate* isolate,
    int* found = unchecked_result_keys.Find(key);
    if (found == nullptr || *found == kGone) {
      isolate->Throw(*isolate->factory()->NewTypeError(
-          MessageTemplate::kProxyTrapOwnKeysResultMustInclude,
-          handle(key, isolate)));
+          MessageTemplate::kProxyOwnKeysMissing, handle(key, isolate)));
      return Nothing<bool>();
    }
    // 17b. Remove key from uncheckedResultKeys.
@ -8599,8 +8618,7 @@ Maybe<bool> JSProxy::OwnPropertyKeys(Isolate* isolate,
    int* found = unchecked_result_keys.Find(key);
    if (found == nullptr || *found == kGone) {
      isolate->Throw(*isolate->factory()->NewTypeError(
-          MessageTemplate::kProxyTrapOwnKeysResultMustInclude,
-          handle(key, isolate)));
+          MessageTemplate::kProxyOwnKeysMissing, handle(key, isolate)));
      return Nothing<bool>();
    }
    // 19b. Remove key from uncheckedResultKeys.
@ -8611,7 +8629,7 @@ Maybe<bool> JSProxy::OwnPropertyKeys(Isolate* isolate,
  if (unchecked_result_keys_size != 0) {
    DCHECK_GT(unchecked_result_keys_size, 0);
    isolate->Throw(*isolate->factory()->NewTypeError(
-        MessageTemplate::kProxyTargetNotExtensible));
+        MessageTemplate::kProxyOwnKeysNonExtensible));
    return Nothing<bool>();
  }
  // 21. Return trapResult.
@ -15153,7 +15171,12 @@ Maybe<bool> JSProxy::SetPrototype(Handle<JSProxy> proxy, Handle<Object> value,
  Maybe<bool> is_extensible = JSReceiver::IsExtensible(target);
  if (is_extensible.IsNothing()) return Nothing<bool>();
  // 10. If extensibleTarget is true, return booleanTrapResult.
-  if (is_extensible.FromJust()) return Just(bool_trap_result);
+  if (is_extensible.FromJust()) {
+    if (bool_trap_result) return Just(true);
+    RETURN_FAILURE(
+        isolate, should_throw,
+        NewTypeError(MessageTemplate::kProxyTrapReturnedFalsish, trap_name));
+  }
  // 11. Let targetProto be ? target.[[GetPrototypeOf]]().
  Handle<Object> target_proto;
  ASSIGN_RETURN_ON_EXCEPTION_VALUE(isolate, target_proto,
@ -15163,11 +15186,14 @@ Maybe<bool> JSProxy::SetPrototype(Handle<JSProxy> proxy, Handle<Object> value,
  // throw a TypeError exception.
  if (bool_trap_result && !value->SameValue(*target_proto)) {
    isolate->Throw(*isolate->factory()->NewTypeError(
-        MessageTemplate::kProxyTrapViolatesInvariant, trap_name));
+        MessageTemplate::kProxySetPrototypeOfNonExtensible));
    return Nothing<bool>();
  }
  // 13. Return booleanTrapResult.
-  return Just(bool_trap_result);
+  if (bool_trap_result) return Just(true);
+  RETURN_FAILURE(
+      isolate, should_throw,
+      NewTypeError(MessageTemplate::kProxyTrapReturnedFalsish, trap_name));
 }


--- a/src/objects.h
+++ b/src/objects.h
@ -1847,7 +1847,8 @@ class JSReceiver: public HeapObject {
  // ES6 9.1.6.2
  MUST_USE_RESULT static Maybe<bool> IsCompatiblePropertyDescriptor(
      Isolate* isolate, bool extensible, PropertyDescriptor* desc,
-      PropertyDescriptor* current, Handle<Name> property_name);
+      PropertyDescriptor* current, Handle<Name> property_name,
+      ShouldThrow should_throw);
  // ES6 9.1.6.3
  // |it| can be NULL in cases where the ES spec passes |undefined| as the
  // receiver. Exactly one of |it| and |property_name| must be provided.
--- a/src/runtime/runtime-object.cc
+++ b/src/runtime/runtime-object.cc
@ -183,14 +183,9 @@ RUNTIME_FUNCTION(Runtime_SetPrototype) {
  DCHECK(args.length() == 2);
  CONVERT_ARG_HANDLE_CHECKED(JSReceiver, obj, 0);
  CONVERT_ARG_HANDLE_CHECKED(Object, prototype, 1);
-  Maybe<bool> status =
-      JSReceiver::SetPrototype(obj, prototype, true, Object::THROW_ON_ERROR);
-  if (status.IsNothing()) return isolate->heap()->exception();
-  if (!status.FromJust()) {
-    THROW_NEW_ERROR_RETURN_FAILURE(
-        isolate,
-        NewTypeError(MessageTemplate::kProxySetPrototypeFailed, prototype));
-  }
+  MAYBE_RETURN(
+      JSReceiver::SetPrototype(obj, prototype, true, Object::THROW_ON_ERROR),
+      isolate->heap()->exception());
  return *obj;
 }

--- a/src/runtime/runtime-proxy.cc
+++ b/src/runtime/runtime-proxy.cc
@ -134,8 +134,7 @@ RUNTIME_FUNCTION(Runtime_JSProxyConstruct) {
  if (!new_object->IsJSReceiver()) {
    THROW_NEW_ERROR_RETURN_FAILURE(
        isolate,
-        NewTypeError(MessageTemplate::kProxyTrapConstructMustReturnObject,
-                     new_object));
+        NewTypeError(MessageTemplate::kProxyConstructNonObject, new_object));
  }
  // 10. Return newObj.
  return *new_object;