Merge branch 'plamen5kov/nativescript_exception_fix' into release

Author: Plamen5kov

runtime/src/main/jni/NativeScriptException.cpp

@@ -155,17 +155,7 @@ void NativeScriptException::Init(ObjectManager *objectManager)
 // ON V8 UNCAUGHT EXCEPTION
 void NativeScriptException::OnUncaughtError(Local<Message> message, Local<Value> error)
 {
-	auto isolate = Isolate::GetCurrent();
-	string errorMessage;
-	auto v8FullMessage = ArgConverter::ConvertToV8String(isolate, "fullMessage");
-
-	if(error->IsObject() && error.As<Object>()->Has(v8FullMessage)) {
-		errorMessage = ArgConverter::ConvertToString(error.As<Object>()->Get(v8FullMessage).As<String>());
-	}
-
-	if(errorMessage.size() == 0) {
-		errorMessage = GetErrorMessage(message, error);
-	}
+	string errorMessage = GetErrorMessage(message, error);
 
 	NativeScriptException e(errorMessage);
 	e.ReThrowToJava();
@@ -252,17 +242,18 @@ string NativeScriptException::GetFullMessage(const TryCatch& tc, bool isExceptio
 {
 	auto ex = tc.Exception();
 
-	string message;
+	string jsExeptionMessage;
+
 	if (!isExceptionEmpty && !isMessageEmpty)
 	{
-		message = PrintErrorMessage(tc.Message(), ex);
+		jsExeptionMessage = GetErrorMessage(tc.Message(), ex);
 	}
 
 	stringstream ss;
-	ss << endl << prependMessage << message;
+	ss << endl << prependMessage << jsExeptionMessage;
 	string loggedMessage = ss.str();
 
-	DEBUG_WRITE("Error: %s", loggedMessage.c_str());
+	PrintErrorMessage(loggedMessage);
 
 	if (!tc.CanContinue())
 	{
@@ -306,10 +297,7 @@ JniLocalRef NativeScriptException::TryGetJavaThrowableObject(JEnv& env, const Lo
 	return javaThrowableObject;
 }
 
-string NativeScriptException::PrintErrorMessage(const Local<Message>& message, const Local<Value>& error)
-{
-	string errorMessage = GetErrorMessage(message, error);
-
+void NativeScriptException::PrintErrorMessage(const string &errorMessage) {
 	// split the message by new lines to workaround the LogCat's maximum characters in a single message
 	stringstream ss(errorMessage);
 	string line;
@@ -318,22 +306,34 @@ string NativeScriptException::PrintErrorMessage(const Local<Message>& message, c
 		// TODO: Log in the V8's Console as well?
 		DEBUG_WRITE("%s", line.c_str());
 	}
-
-	return errorMessage;
 }
 
-string NativeScriptException::GetErrorMessage(const Local<Message>& message, const Local<Value>& error)
+string NativeScriptException::GetErrorMessage(const Local<Message>& message, Local<Value>& error)
 {
-	stringstream ss;
+	//get whole error message from previous stack
+	string errMessage;
+	auto v8FullMessage = ArgConverter::ConvertToV8String(Isolate::GetCurrent(), "fullMessage");
+	if(error->IsObject() && error.As<Object>()->Has(v8FullMessage)) {
+		errMessage = ArgConverter::ConvertToString(error.As<Object>()->Get(v8FullMessage).As<String>());
+	}
 
+	//get current message
 	auto str = error->ToDetailString();
 	if (str.IsEmpty())
 	{
 		str = String::NewFromUtf8(Isolate::GetCurrent(), "");
 	}
 	String::Utf8Value utfError(str);
-	ss << endl << endl << *utfError << endl;
+
+	//get script name
 	auto scriptResName = message->GetScriptResourceName();
+
+	//get stack trace
+	string stackTraceMessage = GetErrorStackTrace(message->GetStackTrace());
+
+	stringstream ss;
+	ss << endl << errMessage;
+	ss << endl << *utfError << endl;
 	if (!scriptResName.IsEmpty() && scriptResName->IsString())
 	{
 		ss << "File: \"" << ArgConverter::ConvertToString(scriptResName.As<String>());
@@ -342,10 +342,7 @@ string NativeScriptException::GetErrorMessage(const Local<Message>& message, con
 	{
 		ss << "File: \"<unknown>";
 	}
-
 	ss << ", line: " << message->GetLineNumber() << ", column: " << message->GetStartColumn() << endl << endl;
-
-	string stackTraceMessage = GetErrorStackTrace(message->GetStackTrace());
 	ss << "StackTrace: " << endl << stackTraceMessage << endl;
 
 	return ss.str();

runtime/src/main/jni/NativeScriptException.h

@@ -11,26 +11,70 @@ namespace tns
 	class NativeScriptException
 	{
 		public:
+			/*
+			 * Generates a NativeScriptException with java error from environment
+			 */
 			NativeScriptException(JEnv& env);
+
+			/*
+			 * Generates a NativeScriptException with given message
+			 */
 			NativeScriptException(const std::string& message);
+
+			/*
+			 * Generates a NativeScriptException with javascript error from TryCatch and a prepend message if any
+			 */
 			NativeScriptException(v8::TryCatch& tc, const std::string& message = "");
 
 			void ReThrowToV8();
 			void ReThrowToJava();
 
 			static void Init(ObjectManager *objectManager);
+
+			/*
+			 * This handler is attached to v8 to handle uncaught javascript exceptions.
+			 */
 			static void OnUncaughtError(v8::Local<v8::Message> message, v8::Local<v8::Value> error);
+
+			/*
+			 * Calls the global "__onUncaughtError" if such is provided
+			 */
 			static void CallJsFuncWithErr(v8::Local<v8::Value> errObj);
 
 		private:
+			/*
+			 * Try to get native exception or NativeScriptException from js object
+			 */
 			JniLocalRef TryGetJavaThrowableObject(JEnv& env, const v8::Local<v8::Object>& jsObj);
+
+			/*
+			 * Gets java exception stack message from jthrowable
+			 */
 			std::string GetExceptionMessage(JEnv& env, jthrowable exception);
+
+			/*
+			 * Gets the member m_javaException, wraps it and creates a javascript error object from it
+			 */
 			v8::Local<v8::Value> WrapJavaToJsException();
+
+			/*
+			 * Gets all the information from a java exception and puts it in a javascript errror object
+			 */
 			v8::Local<v8::Value> GetJavaExceptionFromEnv(const JniLocalRef& exc, JEnv& env);
-			static std::string PrintErrorMessage(const v8::Local<v8::Message>& message, const v8::Local<v8::Value>& error);
-			static std::string GetErrorMessage(const v8::Local<v8::Message>& message, const v8::Local<v8::Value>& error);
+
+			/*
+			 * Gets all the information from a js message and an js error object and puts it in a string
+			 */
+			static std::string GetErrorMessage(const v8::Local<v8::Message>& message, v8::Local<v8::Value>& error);
+
+			/*
+			 * Generates string stack trace from js StackTrace
+			 */
 			static std::string GetErrorStackTrace(const v8::Local<v8::StackTrace>& stackTrace);
 
+			/*
+			 *	Adds a prepend message to the normal message process
+			 */
 			std::string GetFullMessage(const v8::TryCatch& tc, bool isExceptionEmpty, bool isMessageEmpty, const std::string& prependMessage = "");
 
 			v8::Persistent<v8::Value> *m_javascriptException;
@@ -44,6 +88,8 @@ namespace tns
 			static jmethodID NATIVESCRIPTEXCEPTION_JSVALUE_CTOR_ID;
 			static jmethodID NATIVESCRIPTEXCEPTION_THROWABLE_CTOR_ID;
 			static jmethodID NATIVESCRIPTEXCEPTION_GET_STACK_TRACE_AS_STRING_METHOD_ID;
+
+			static void PrintErrorMessage(const std::string &errorMessage);
 	};
 }
 

test-app/app/src/main/assets/app/tests/exceptionHandlingTests.js

@@ -323,4 +323,23 @@ describe("Tests exception handling ", function () {
 	    expect(exceptionCaught).toBe(true);
 	    expect(errMsg).toBe("Error: Unknown error. Cannot get error message.");
 	});
+
+	it("should successfully catch syntax errors", function () {
+
+	    var exceptionCaught = false;
+	    var errMsg;
+	    try {
+	        require("./syntaxErrors.js");
+	    } catch (e) {
+	        exceptionCaught = true;
+	        errMsg = e.fullMessage
+	    }
+	    expect(exceptionCaught).toBe(true);
+	    expect(errMsg).toContain("Cannot compile /data/data/com.tns.android_runtime_testapp/files/app/tests/syntaxErrors.js");
+	    expect(errMsg).toContain("SyntaxError: Block-scoped declarations (let, const, function, class) not yet supported outside strict mode");
+	    expect(errMsg).toContain("File: \"/data/data/com.tns.android_runtime_testapp/files/app/tests/syntaxErrors.js, line: 4, column: 0");
+
+
+
+	});
 });

test-app/app/src/main/assets/app/tests/syntaxErrors.js

@@ -0,0 +1,4 @@
+//this file is required
+
+var b = 5;
+let a = 5;