Merge pull request #495 from NativeScript/slavchev/fix-issue-476

Fix for #476. For some throwable object getStackTrace() method throws…

Author: Mihail Slavchev

runtime/src/main/java/com/tns/NativeScriptException.java

@@ -30,4 +30,30 @@ public NativeScriptException(String detailMessage, long jsValueAddress)
 		super(detailMessage);
 		this.jsValueAddress = jsValueAddress;
 	}
+
+	@RuntimeCallable
+	public static String getStackTraceAsString(Throwable ex)
+	{
+		String errMessage;
+		try
+		{
+			errMessage = ex.toString();
+			for (StackTraceElement frame: ex.getStackTrace())
+			{
+				errMessage += "\n    ";
+				errMessage += frame;
+			}
+
+			Throwable cause = ex.getCause();
+			if (cause != null) {
+                errMessage += "\nCaused by: ";
+                errMessage += getStackTraceAsString(cause);
+			}
+		}
+		catch (Throwable err)
+		{
+			errMessage = "Unknown error. Cannot get error message.";
+		}
+		return errMessage;
+	}
 }

runtime/src/main/jni/NativeScriptException.cpp

@@ -135,21 +135,6 @@ void NativeScriptException::Init(ObjectManager *objectManager)
 	THROWABLE_CLASS = env.FindClass("java/lang/Throwable");
 	assert(THROWABLE_CLASS != nullptr);
 
-	THROWABLE_GET_CAUSE_METHOD_ID = env.GetMethodID(THROWABLE_CLASS, "getCause", "()Ljava/lang/Throwable;");
-	assert(THROWABLE_GET_CAUSE_METHOD_ID != nullptr);
-
-	THROWABLE_GET_STACK_TRACE_METHOD_ID = env.GetMethodID(THROWABLE_CLASS, "getStackTrace", "()[Ljava/lang/StackTraceElement;");
-	assert(THROWABLE_GET_STACK_TRACE_METHOD_ID != nullptr);
-
-	THROWABLE_TO_STRING_METHOD_ID = env.GetMethodID(THROWABLE_CLASS, V8StringConstants::TO_STRING, "()Ljava/lang/String;");
-	assert(THROWABLE_TO_STRING_METHOD_ID != nullptr);
-
-	STACK_TRACE_ELEMENT_CLASS = env.FindClass("java/lang/StackTraceElement");
-	assert(STACK_TRACE_ELEMENT_CLASS != nullptr);
-
-	FRAME_TO_STRING_METHOD_ID = env.GetMethodID(STACK_TRACE_ELEMENT_CLASS, V8StringConstants::TO_STRING, "()Ljava/lang/String;");
-	assert(FRAME_TO_STRING_METHOD_ID != nullptr);
-
 	NATIVESCRIPTEXCEPTION_CLASS = env.FindClass("com/tns/NativeScriptException");
 	assert(NATIVESCRIPTEXCEPTION_CLASS != nullptr);
 
@@ -158,6 +143,9 @@ void NativeScriptException::Init(ObjectManager *objectManager)
 
 	NATIVESCRIPTEXCEPTION_THROWABLE_CTOR_ID = env.GetMethodID(NATIVESCRIPTEXCEPTION_CLASS, "<init>", "(Ljava/lang/String;Ljava/lang/Throwable;)V");
 	assert(NATIVESCRIPTEXCEPTION_THROWABLE_CTOR_ID != nullptr);
+
+	NATIVESCRIPTEXCEPTION_GET_STACK_TRACE_AS_STRING_METHOD_ID = env.GetStaticMethodID(NATIVESCRIPTEXCEPTION_CLASS, "getStackTraceAsString", "(Ljava/lang/Throwable;)Ljava/lang/String;");
+	assert(NATIVESCRIPTEXCEPTION_GET_STACK_TRACE_AS_STRING_METHOD_ID != nullptr);
 }
 
 // ON V8 UNCAUGHT EXCEPTION
@@ -394,62 +382,21 @@ string NativeScriptException::GetErrorStackTrace(const Local<StackTrace>& stackT
 string NativeScriptException::GetExceptionMessage(JEnv& env, jthrowable exception)
 {
 	string errMsg;
-	JniLocalRef frames(env.CallObjectMethod(exception, THROWABLE_GET_STACK_TRACE_METHOD_ID));
-
-	jsize framesLength = env.GetArrayLength(frames);
-
-	if (nullptr != (jobjectArray) frames)
-	{
-		JniLocalRef msg(env.CallObjectMethod(exception, THROWABLE_TO_STRING_METHOD_ID));
-
-		const char* msgStr = env.GetStringUTFChars(msg, nullptr);
-
-		if (!errMsg.empty())
-		{
-			errMsg.append("\nCaused by: ");
-			errMsg.append(msgStr);
-		}
-		else
-		{
-			errMsg = msgStr;
-		}
+	JniLocalRef msg(env.CallStaticObjectMethod(NATIVESCRIPTEXCEPTION_CLASS, NATIVESCRIPTEXCEPTION_GET_STACK_TRACE_AS_STRING_METHOD_ID, exception));
 
-		env.ReleaseStringUTFChars(msg, msgStr);
-	}
+	const char* msgStr = env.GetStringUTFChars(msg, nullptr);
 
-	for (jsize i = 0; i < framesLength; i++)
-	{
-		JniLocalRef frame(env.GetObjectArrayElement(frames, i));
-		JniLocalRef msg(env.CallObjectMethod(frame, FRAME_TO_STRING_METHOD_ID));
-
-		const char* msgStr = env.GetStringUTFChars(msg, nullptr);
+	errMsg.append(msgStr);
 
-		errMsg.append("\n    ");
-		errMsg.append(msgStr);
+	env.ReleaseStringUTFChars(msg, msgStr);
 
-		env.ReleaseStringUTFChars(msg, msgStr);
-	}
-
-	if (nullptr != (jobjectArray) frames)
-	{
-		JniLocalRef cause(env.CallObjectMethod(exception, THROWABLE_GET_CAUSE_METHOD_ID));
-
-		if (nullptr != (jthrowable) cause)
-		{
-			errMsg = GetExceptionMessage(env, cause);
-		}
-	}
 	return errMsg;
 }
 
 ObjectManager* NativeScriptException::objectManager = nullptr;
 jclass NativeScriptException::RUNTIME_CLASS = nullptr;
 jclass NativeScriptException::THROWABLE_CLASS = nullptr;
-jmethodID NativeScriptException::THROWABLE_GET_CAUSE_METHOD_ID = nullptr;
-jmethodID NativeScriptException::THROWABLE_GET_STACK_TRACE_METHOD_ID = nullptr;
-jmethodID NativeScriptException::THROWABLE_TO_STRING_METHOD_ID = nullptr;
-jclass NativeScriptException::STACK_TRACE_ELEMENT_CLASS = nullptr;
-jmethodID NativeScriptException::FRAME_TO_STRING_METHOD_ID = nullptr;
 jclass NativeScriptException::NATIVESCRIPTEXCEPTION_CLASS = nullptr;
 jmethodID NativeScriptException::NATIVESCRIPTEXCEPTION_JSVALUE_CTOR_ID = nullptr;
 jmethodID NativeScriptException::NATIVESCRIPTEXCEPTION_THROWABLE_CTOR_ID = nullptr;
+jmethodID NativeScriptException::NATIVESCRIPTEXCEPTION_GET_STACK_TRACE_AS_STRING_METHOD_ID = nullptr;

runtime/src/main/jni/NativeScriptException.h

@@ -40,14 +40,10 @@ namespace tns
 			static ObjectManager *objectManager;
 			static jclass RUNTIME_CLASS;
 			static jclass THROWABLE_CLASS;
-			static jmethodID THROWABLE_GET_CAUSE_METHOD_ID;
-			static jmethodID THROWABLE_GET_STACK_TRACE_METHOD_ID;
-			static jmethodID THROWABLE_TO_STRING_METHOD_ID;
-			static jclass STACK_TRACE_ELEMENT_CLASS;
-			static jmethodID FRAME_TO_STRING_METHOD_ID;
 			static jclass NATIVESCRIPTEXCEPTION_CLASS;
 			static jmethodID NATIVESCRIPTEXCEPTION_JSVALUE_CTOR_ID;
 			static jmethodID NATIVESCRIPTEXCEPTION_THROWABLE_CTOR_ID;
+			static jmethodID NATIVESCRIPTEXCEPTION_GET_STACK_TRACE_AS_STRING_METHOD_ID;
 	};
 }
 

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

@@ -280,7 +280,6 @@ describe("Tests exception handling ", function () {
 		}
 
 		expect(exceptionCaught).toBe(true);
-		
 	});
 
 	it("should not wrap the thrown exception into NativeScriptException", function () {
@@ -311,4 +310,17 @@ describe("Tests exception handling ", function () {
 		expect(e2.getCause().getClass()).toBe(java.io.IOException.class);
 	});
 
+	it("should successfully catch Java exception when getStackTrace fails", function () {
+	    var test = new com.tns.tests.ExceptionHandlingTest();
+	    var exceptionCaught = false;
+	    var errMsg;
+	    try {
+	        test.throwException();
+	    } catch (e) {
+	        exceptionCaught = true;
+	        errMsg = e.toString();
+	    }
+	    expect(exceptionCaught).toBe(true);
+	    expect(errMsg).toBe("Error: Unknown error. Cannot get error message.");
+	});
 });

test-app/app/src/main/assets/metadata/treeNodeStream.dat

@@ -7,6 +7,16 @@
 
 public class ExceptionHandlingTest
 {
+	private static class BadException extends Exception
+	{
+		public BadException() {}
+
+		@Override
+		public StackTraceElement[] getStackTrace() {
+			throw new RuntimeException();
+		}
+	}
+
 	public void triggerEvent1(String s, int n) throws Exception
 	{
 		this.onEvent1(s, n);
@@ -51,4 +61,8 @@ public void getExceptionRec(String s, int n)
 	{
 		this.onGetFile(s, n);
 	}
+
+    public void throwException() throws BadException {
+        throw new BadException();
+    }
 }