chakrashim: Implementing v8::Message

Using the new JSRT method JsGetAndClearExceptionWithMetadata we can implement
more v8::Message functionality which lets us give better messages for
unhandled exceptions.

deps/chakrashim/include/v8.h

@@ -303,6 +303,7 @@ class Local {
   friend class HandleScope;
   friend class Integer;
   friend class Map;
+  friend class Message;
   friend class Number;
   friend class NumberObject;
   friend class Object;
@@ -2736,8 +2737,10 @@ class V8_EXPORT TryCatch {
   void SetNonUser() { user = false; }
   void GetAndClearException();
   void CheckReportExternalException();
+  JsValueRef EnsureException() const;
 
-  JsValueRef error;
+
+  JsValueRef metadata;
   TryCatch* prev;
   bool rethrow;
   bool user;

deps/chakrashim/src/jsrtcachedpropertyidref.inc

@@ -119,6 +119,8 @@ DEF(subtype)
 DEF(thisObject)
 DEF(type)
 DEF(uncaught)
+DEF(url)
+
 
 DEFSYMBOL(self)
 DEFSYMBOL(__external__)

deps/chakrashim/src/v8message.cc

@@ -28,8 +28,15 @@ Local<String> Message::Get() const {
 }
 
 MaybeLocal<String> Message::GetSourceLine(Local<Context> context) const {
-  // CHAKRA-TODO: Figure out how to transmit this info...?
-  return Local<String>();
+  JsPropertyIdRef source = jsrt::IsolateShim::GetCurrent()
+      ->GetCachedPropertyIdRef(jsrt::CachedPropertyIdRef::source);
+
+  JsValueRef result;
+  if (JsGetProperty((JsValueRef)this, source, &result) != JsNoError) {
+    return Local<String>();
+  }
+
+  return Local<String>::New(result);
 }
 
 Local<String> Message::GetSourceLine() const {
@@ -41,37 +48,76 @@ ScriptOrigin Message::GetScriptOrigin() const {
   return ScriptOrigin(Local<String>());
 }
 
-Handle<Value> Message::GetScriptResourceName() const {
-  // CHAKRA-TODO: Figure out how to transmit this info...?
-  return Handle<Value>();
-}
-
 Local<StackTrace> Message::GetStackTrace() const {
   // CHAKRA-TODO: Figure out what to do here
   return Local<StackTrace>();
 }
 
+Handle<Value> Message::GetScriptResourceName() const {
+  JsPropertyIdRef url = jsrt::IsolateShim::GetCurrent()
+      ->GetCachedPropertyIdRef(jsrt::CachedPropertyIdRef::url);
+
+  JsValueRef result;
+  if (JsGetProperty((JsValueRef)this, url, &result) != JsNoError) {
+    return Local<Value>();
+  }
+  return Handle<Value>::New(result);
+}
+
 Maybe<int> Message::GetLineNumber(Local<Context> context) const {
-  // CHAKRA-TODO: Figure out how to transmit this info...?
-  return Nothing<int>();
+  JsPropertyIdRef lineProp = jsrt::IsolateShim::GetCurrent()
+      ->GetCachedPropertyIdRef(jsrt::CachedPropertyIdRef::line);
+
+  JsValueRef result;
+  if (JsGetProperty((JsValueRef)this, lineProp, &result) != JsNoError) {
+    return Nothing<int>();
+  }
+  int line;
+  if (jsrt::ValueToIntLikely(result, &line) != JsNoError) {
+    return Nothing<int>();
+  }
+  return Just<int>(line + 1);
 }
 
 int Message::GetLineNumber() const {
   return FromMaybe(GetLineNumber(Local<Context>()));
 }
 
 Maybe<int> Message::GetStartColumn(Local<Context> context) const {
-  // CHAKRA-TODO: Figure out how to transmit this info...?
-  return Nothing<int>();
+  JsPropertyIdRef columnProp = jsrt::IsolateShim::GetCurrent()
+      ->GetCachedPropertyIdRef(jsrt::CachedPropertyIdRef::column);
+
+  JsValueRef result;
+  if (JsGetProperty((JsValueRef)this, columnProp, &result) != JsNoError) {
+    return Nothing<int>();
+  }
+  int column;
+  if (jsrt::ValueToIntLikely(result, &column) != JsNoError) {
+    return Nothing<int>();
+  }
+  return Just<int>(column);
 }
 
 int Message::GetStartColumn() const {
   return FromMaybe(GetStartColumn(Local<Context>()));
 }
 
 Maybe<int> Message::GetEndColumn(Local<Context> context) const {
-  // CHAKRA-TODO: Figure out how to transmit this info...?
-  return Nothing<int>();
+  int column = GetStartColumn();
+  JsPropertyIdRef lengthProp = jsrt::IsolateShim::GetCurrent()
+      ->GetCachedPropertyIdRef(jsrt::CachedPropertyIdRef::length);
+
+
+  JsValueRef result;
+  if (JsGetProperty((JsValueRef)this, lengthProp, &result) != JsNoError) {
+    return Nothing<int>();
+  }
+  int length;
+  if (jsrt::ValueToIntLikely(result, &length) != JsNoError) {
+    return Nothing<int>();
+  }
+
+  return Just<int>(column + length + 1);
 }
 
 int Message::GetEndColumn() const {

deps/chakrashim/src/v8trycatch.cc

@@ -24,7 +24,7 @@
 namespace v8 {
 
 TryCatch::TryCatch(Isolate* isolate)
-    : error(JS_INVALID_REFERENCE),
+    : metadata(JS_INVALID_REFERENCE),
       rethrow(false),
       user(true),
       verbose(false) {
@@ -42,11 +42,11 @@ TryCatch::~TryCatch() {
 }
 
 bool TryCatch::HasCaught() const {
-  if (error == JS_INVALID_REFERENCE) {
+  if (metadata == JS_INVALID_REFERENCE) {
     const_cast<TryCatch*>(this)->GetAndClearException();
   }
 
-  if (error != JS_INVALID_REFERENCE) {
+  if (metadata != JS_INVALID_REFERENCE) {
     return true;
   }
   bool hasException;
@@ -78,22 +78,19 @@ void TryCatch::GetAndClearException() {
   }
 
   if (hasException) {
-    JsValueRef exceptionRef;
-    errorCode = JsGetAndClearException(&exceptionRef);
+    JsValueRef metadataRef;
+    errorCode = JsGetAndClearExceptionWithMetadata(&metadataRef);
     // We came here through JsHasException, so script shouldn't be in disabled
     // state.
     CHAKRA_ASSERT(errorCode != JsErrorInDisabledState);
     if (errorCode == JsNoError) {
-      error = exceptionRef;
+      metadata = metadataRef;
     }
   }
 }
 
 Handle<Value> TryCatch::ReThrow() {
-  if (error == JS_INVALID_REFERENCE) {
-    GetAndClearException();
-  }
-
+  JsValueRef error = this->EnsureException();
   if (error == JS_INVALID_REFERENCE) {
     return Local<Value>();
   }
@@ -107,10 +104,7 @@ Handle<Value> TryCatch::ReThrow() {
 }
 
 Local<Value> TryCatch::Exception() const {
-  if (error == JS_INVALID_REFERENCE) {
-    const_cast<TryCatch*>(this)->GetAndClearException();
-  }
-
+  JsValueRef error = this->EnsureException();
   if (error == JS_INVALID_REFERENCE) {
     return Local<Value>();
   }
@@ -119,19 +113,13 @@ Local<Value> TryCatch::Exception() const {
 }
 
 MaybeLocal<Value> TryCatch::StackTrace(Local<Context> context) const {
-  if (error == JS_INVALID_REFERENCE) {
-    const_cast<TryCatch*>(this)->GetAndClearException();
-  }
-
+  JsValueRef error = this->EnsureException();
   if (error == JS_INVALID_REFERENCE) {
     return Local<Value>();
   }
 
   JsPropertyIdRef stack = jsrt::IsolateShim::GetCurrent()
-    ->GetCachedPropertyIdRef(jsrt::CachedPropertyIdRef::stack);
-  if (stack == JS_INVALID_REFERENCE) {
-    return Local<Value>();
-  }
+      ->GetCachedPropertyIdRef(jsrt::CachedPropertyIdRef::stack);
 
   JsValueRef trace;
   if (JsGetProperty(error, stack, &trace) != JsNoError) {
@@ -146,10 +134,14 @@ Local<Value> TryCatch::StackTrace() const {
 }
 
 Local<v8::Message> TryCatch::Message() const {
-  // return an empty ref for now, so no nulls/empty messages will be printed
-  // should be changed once we understand how to retreive the info for each
-  // errror message
-  return Local<v8::Message>();
+  if (metadata == JS_INVALID_REFERENCE) {
+    const_cast<TryCatch*>(this)->GetAndClearException();
+  }
+
+  if (metadata == JS_INVALID_REFERENCE) {
+    return Local<v8::Message>();
+  }
+  return Local<v8::Message>::New(metadata);
 }
 
 void TryCatch::SetVerbose(bool value) {
@@ -181,4 +173,24 @@ void TryCatch::CheckReportExternalException() {
   }
 }
 
+JsValueRef TryCatch::EnsureException() const {
+  if (metadata == JS_INVALID_REFERENCE) {
+    const_cast<TryCatch*>(this)->GetAndClearException();
+  }
+
+  if (metadata == JS_INVALID_REFERENCE) {
+    return JS_INVALID_REFERENCE;
+  }
+
+  JsPropertyIdRef err = jsrt::IsolateShim::GetCurrent()
+      ->GetCachedPropertyIdRef(jsrt::CachedPropertyIdRef::exception);
+
+  JsValueRef exception;
+  if (JsGetProperty(metadata, err, &exception) != JsNoError) {
+    return JS_INVALID_REFERENCE;
+  }
+
+  return exception;
+}
+
 }  // namespace v8

test/message/core_line_numbers.chakracore.out

@@ -1,5 +1,9 @@
+punycode.js:*
+	throw new RangeError(errors[type]);
+	^
+
 RangeError: Invalid input
-   at error (punycode.js:42:*)
+   at error (punycode.js:*:*)
    at decode (punycode.js:*:*)
    at Anonymous function (*test*message*core_line_numbers.js:*:*)
    at Module.prototype._compile (module.js:*:*)

test/message/error_exit.chakracore.out

@@ -1,4 +1,8 @@
 Exiting with code=1
+assert.js:*
+  throw new errors.AssertionError({
+  ^
+
 AssertionError [ERR_ASSERTION]: 1 === 2
    at Anonymous function (*test*message*error_exit.js:*:*)
    at Module.prototype._compile (module.js:*:*)

test/message/eval_messages.chakracore.out

@@ -1,4 +1,8 @@
 [eval]
+[eval]:1
+'use strict'; with(this){__filename}
+              ^^^^^
+
 SyntaxError: 'with' statements are not allowed in strict mode
    at createScript (vm.js:*:*)
    at runInThisContext (vm.js:*:*)
@@ -9,34 +13,54 @@ SyntaxError: 'with' statements are not allowed in strict mode
    at Anonymous function (bootstrap_node.js:*:*)
 42
 42
+[eval]:1
+throw new Error("hello")
+^
+
 Error: hello
-   at Global code ([eval]:1:1)
-   at Script.prototype.runInThisContext (vm.js:*:*)      
+   at Global code ([eval]:*:*)
+   at Script.prototype.runInThisContext (vm.js:*:*)
    at runInThisContext (vm.js:*:*)
    at Anonymous function ([eval]-wrapper:*:*)
    at Module.prototype._compile (module.js:*:*)
    at evalScript (bootstrap_node.js:*:*)
    at startup (bootstrap_node.js:*:*)
    at Anonymous function (bootstrap_node.js:*:*)
+[eval]:1
+'use strict'; throw new Error("hello")
+              ^
+
 Error: hello
-   at Global code ([eval]:1:15)
-   at Script.prototype.runInThisContext (vm.js:*:*)   
+   at Global code ([eval]:*:*)
+   at Script.prototype.runInThisContext (vm.js:*:*)
    at runInThisContext (vm.js:*:*)
    at Anonymous function ([eval]-wrapper:*:*)
    at Module.prototype._compile (module.js:*:*)
    at evalScript (bootstrap_node.js:*:*)
    at startup (bootstrap_node.js:*:*)
    at Anonymous function (bootstrap_node.js:*:*)
 100
+[eval]:1
+'use strict'; var x = 100; y = x;
+                           ^
+
 ReferenceError: Variable undefined in strict mode
-   at Global code ([eval]:1:28)
-   at Script.prototype.runInThisContext (vm.js:*:*)      
+   at Global code ([eval]:*:*)
+   at Script.prototype.runInThisContext (vm.js:*:*)
    at runInThisContext (vm.js:*:*)
    at Anonymous function ([eval]-wrapper:*:*)
    at Module.prototype._compile (module.js:*:*)
    at evalScript (bootstrap_node.js:*:*)
    at startup (bootstrap_node.js:*:*)
    at Anonymous function (bootstrap_node.js:*:*)
+
+vm.js:*
+    return realRunInThisContext.call(this, options);
+    ^
 10
+
+vm.js:*
+    return realRunInThisContext.call(this, options);
+    ^
 10
-done
+done

test/message/nexttick_throw.chakracore.out

@@ -1,7 +1,11 @@
-ReferenceError: 'undefined_reference_error_maker' is not defined
-   at Anonymous function (*test*message*nexttick_throw.js:*:*)
-   at _combinedTickCallback (internal/process/next_tick.js:*:*)
-   at _tickCallback (internal/process/next_tick.js:*:*)
-   at Module.runMain (module.js:*:*)
-   at startup (bootstrap_node.js:*:*)
-   at Anonymous function (bootstrap_node.js:*:*)
+
+*test*message*nexttick_throw.js:*
+*undefined_reference_error_maker;
+*^
+ReferenceError: *undefined_reference_error_maker*
+*at *test*message*nexttick_throw.js:*:*
+*at _combinedTickCallback (internal/process/next_tick.js:*:*)
+*at *_tickCallback (internal/process/next_tick.js:*:*)
+*at Module.runMain (module.js:*:*)
+*at startup (bootstrap_node.js:*:*)
+*at *bootstrap_node.js:*:*

test/message/nexttick_throw.v8.out

@@ -1,11 +1,12 @@
 
 *test*message*nexttick_throw.js:*
-        undefined_reference_error_maker;
-        ^
-ReferenceError: undefined_reference_error_maker is not defined
-    at *test*message*nexttick_throw.js:*:*
-    at _combinedTickCallback (internal/process/next_tick.js:*:*)
-    at process._tickCallback (internal/process/next_tick.js:*:*)
-    at Function.Module.runMain (module.js:*:*)
-    at startup (bootstrap_node.js:*:*)
-    at bootstrap_node.js:*:*
+*undefined_reference_error_maker;
+*^
+ReferenceError: *undefined_reference_error_maker*
+*at *test*message*nexttick_throw.js:*:*
+*at _combinedTickCallback (internal/process/next_tick.js:*:*)
+*at *_tickCallback (internal/process/next_tick.js:*:*)
+*at Module.runMain (module.js:*:*)
+*at run (bootstrap_node.js:*:*)
+*at startup (bootstrap_node.js:*:*)
+*at *bootstrap_node.js:*:*