Merge pull request #524 from NativeScript/atanasovg/fix-v8-rethrow

Fix re-throwing errors to V8

Author: atanasovg

.gitignore

@@ -6,6 +6,7 @@ dist/
 /test-app/libs
 /test-app/metadata
 /test-app/.idea
+.idea
 
 local.properties
 

runtime/src/main/jni/Module.cpp

@@ -14,6 +14,7 @@
 #include "NativeScriptException.h"
 #include "Util.h"
 #include "SimpleProfiler.h"
+#include "include/v8.h"
 
 #include <sstream>
 #include <assert.h>
@@ -200,12 +201,7 @@ void Module::Load(const string& path)
 	auto globalObject = context->Global();
 	auto require = globalObject->Get(context, ConvertToV8String("require")).ToLocalChecked().As<Function>();
 	Local<Value> args[] = { ConvertToV8String(path) };
-	TryCatch tc;
 	require->Call(context, globalObject, 1, args);
-	if (tc.HasCaught())
-	{
-		throw NativeScriptException(tc, "Fail to load module: " + path);
-	}
 }
 
 Local<Object> Module::LoadImpl(Isolate *isolate, const string& moduleName, const string& baseDir, bool& isData)

runtime/src/main/jni/NativeScriptException.cpp

@@ -3,6 +3,7 @@
 #include "V8GlobalHelpers.h"
 #include "NativeScriptAssert.h"
 #include "V8StringConstants.h"
+#include "include/v8.h"
 #include <sstream>
 
 using namespace std;
@@ -43,6 +44,9 @@ void NativeScriptException::ReThrowToV8()
 	if (m_javascriptException != nullptr)
 	{
 		errObj = Local<Value>::New(isolate, *m_javascriptException);
+		if(errObj->IsObject() && !m_message.empty()) {
+			errObj.As<Object>()->Set(ConvertToV8String("fullMessage"), ConvertToV8String(m_message));
+		}
 	}
 	else if (!m_message.empty())
 	{
@@ -151,30 +155,19 @@ void NativeScriptException::Init(ObjectManager *objectManager)
 // ON V8 UNCAUGHT EXCEPTION
 void NativeScriptException::OnUncaughtError(Local<Message> message, Local<Value> error)
 {
-	auto errorMessage = PrintErrorMessage(message, error);
-
-	Isolate *isolate = Isolate::GetCurrent();
-	HandleScope scope(isolate);
+	string errorMessage;
+	auto v8FullMessage = ConvertToV8String("fullMessage");
 
-	Local<Object> errorObject;
-	if (error->IsObject())
-	{
-		errorObject = error.As<Object>();
-		errorObject->Set(ConvertToV8String("message"), ConvertToV8String(errorMessage));
-	}
-	else
-	{
-		errorObject = Exception::Error(ConvertToV8String(errorMessage)).As<Object>();
+	if(error->IsObject() && error.As<Object>()->Has(v8FullMessage)) {
+		errorMessage = ConvertToString(error.As<Object>()->Get(v8FullMessage).As<String>());
 	}
 
-	CallJsFuncWithErr(errorObject);
+	if(errorMessage.size() == 0) {
+		errorMessage = GetErrorMessage(message, error);
+	}
 
-	// check whether the developer marked the error as "Caught"
-	// As per discussion, it is safer to ALWAYS kill the application due to uncaught error(s)
-	// TODO: We may think for some per-thread specific behavior and allow execution to continue for background thread exceptions
-//		if(!returnValue.IsEmpty() && (returnValue->IsBoolean() || returnValue->IsBooleanObject())){
-//			handled = returnValue->BooleanValue();
-//		}
+	NativeScriptException e(errorMessage);
+	e.ReThrowToJava();
 }
 
 void NativeScriptException::CallJsFuncWithErr(Local<Value> errObj)

runtime/src/main/jni/Runtime.cpp

@@ -248,9 +248,6 @@ void Runtime::CreateJSInstanceNative(JNIEnv *_env, jobject obj, jobject javaObje
 
 	JEnv env(_env);
 
-	// TODO: Do we need a TryCatch here? It is currently not used anywhere
-	TryCatch tc;
-
 	string existingClassName = ArgConverter::jstringToString(className);
 	string jniName = Util::ConvertFromCanonicalToJniName(existingClassName);
 	Local<Object> jsInstance;
@@ -259,9 +256,10 @@ void Runtime::CreateJSInstanceNative(JNIEnv *_env, jobject obj, jobject javaObje
 	auto proxyClassName = m_objectManager->GetClassName(javaObject);
 	DEBUG_WRITE("createJSInstanceNative class %s", proxyClassName.c_str());
 	jsInstance = MetadataNode::CreateExtendedJSWrapper(isolate, m_objectManager, proxyClassName);
+
 	if (jsInstance.IsEmpty())
 	{
-		throw NativeScriptException(string("NativeScript application not initialized correctly. Cannot create extended JS wrapper."));
+		throw NativeScriptException(string("Failed to create JavaScript extend wrapper for class '" + proxyClassName + "'"));
 	}
 
 	implementationObject = MetadataNode::GetImplementationObject(jsInstance);