[TIMOB-25963] Android: Improve KrollRuntime error output

android/runtime/common/src/java/org/appcelerator/kroll/KrollExceptionHandler.java

@@ -34,18 +34,21 @@
 public interface KrollExceptionHandler {
 	public class ExceptionMessage
 	{
-		public String title, message, sourceName, lineSource;
+		public String title, message, sourceName, lineSource, jsStack, javaStack;
 		public int line, lineOffset;
 
 		public ExceptionMessage(final String title, final String message, final String sourceName, final int line,
-								final String lineSource, final int lineOffset)
+								final String lineSource, final int lineOffset, final String jsStack,
+								final String javaStack)
 		{
 			this.title = title;
 			this.message = message;
 			this.sourceName = sourceName;
 			this.lineSource = lineSource;
 			this.line = line;
 			this.lineOffset = lineOffset;
+			this.jsStack = jsStack;
+			this.javaStack = javaStack;
 		}
 	}
 

android/runtime/common/src/java/org/appcelerator/kroll/KrollRuntime.java

@@ -518,25 +518,26 @@ public static void removeExceptionHandler(String key)
 	}
 
 	public static void dispatchException(final String title, final String message, final String sourceName,
-										 final int line, final String lineSource, final int lineOffset)
+										 final int line, final String lineSource, final int lineOffset,
+										 final String jsStack, final String javaStack)
 	{
 		if (instance != null) {
 			HashMap<String, KrollExceptionHandler> handlers = instance.exceptionHandlers;
 			KrollExceptionHandler currentHandler;
+			ExceptionMessage exceptionMessage =
+				new ExceptionMessage(title, message, sourceName, line, lineSource, lineOffset, jsStack, javaStack);
 
 			if (!handlers.isEmpty()) {
 				for (String key : handlers.keySet()) {
 					currentHandler = handlers.get(key);
 					if (currentHandler != null) {
-						currentHandler.handleException(
-							new ExceptionMessage(title, message, sourceName, line, lineSource, lineOffset));
+						currentHandler.handleException(exceptionMessage);
 					}
 				}
 			}
 
 			// Handle exception with defaultExceptionHandler
-			instance.primaryExceptionHandler.handleException(
-				new ExceptionMessage(title, message, sourceName, line, lineSource, lineOffset));
+			instance.primaryExceptionHandler.handleException(exceptionMessage);
 		}
 	}
 

android/runtime/v8/src/native/JNIUtil.cpp

@@ -43,6 +43,7 @@ jclass JNIUtil::setClass = NULL;
 jclass JNIUtil::outOfMemoryError = NULL;
 jclass JNIUtil::nullPointerException = NULL;
 jclass JNIUtil::throwableClass = NULL;
+jclass JNIUtil::stackTraceElementClass = NULL;
 
 jclass JNIUtil::v8ObjectClass = NULL;
 jclass JNIUtil::v8FunctionClass = NULL;
@@ -79,7 +80,11 @@ jmethodID JNIUtil::booleanInitMethod = NULL;
 jmethodID JNIUtil::booleanBooleanValueMethod = NULL;
 jmethodID JNIUtil::longInitMethod = NULL;
 jmethodID JNIUtil::numberDoubleValueMethod = NULL;
+
 jmethodID JNIUtil::throwableGetMessageMethod = NULL;
+jmethodID JNIUtil::throwableGetStackTraceMethod = NULL;
+
+jmethodID JNIUtil::stackTraceElementToStringMethod = NULL;
 
 jfieldID JNIUtil::v8ObjectPtrField = NULL;
 jmethodID JNIUtil::v8ObjectInitMethod = NULL;
@@ -318,6 +323,7 @@ void JNIUtil::initCache()
 	outOfMemoryError = findClass("java/lang/OutOfMemoryError");
 	nullPointerException = findClass("java/lang/NullPointerException");
 	throwableClass = findClass("java/lang/Throwable");
+	stackTraceElementClass = findClass("java/lang/StackTraceElement");
 
 	v8ObjectClass = findClass("org/appcelerator/kroll/runtime/v8/V8Object");
 	v8FunctionClass = findClass("org/appcelerator/kroll/runtime/v8/V8Function");
@@ -355,6 +361,8 @@ void JNIUtil::initCache()
 	longInitMethod = getMethodID(longClass, "<init>", "(J)V", false);
 	numberDoubleValueMethod = getMethodID(numberClass, "doubleValue", "()D", false);
 	throwableGetMessageMethod = getMethodID(throwableClass, "getMessage", "()Ljava/lang/String;", false);
+	throwableGetStackTraceMethod = getMethodID(throwableClass, "getStackTrace", "()[Ljava/lang/StackTraceElement;", false);
+	stackTraceElementToStringMethod = getMethodID(stackTraceElementClass, "toString", "()Ljava/lang/String;", false);
 
 	v8ObjectPtrField = getFieldID(v8ObjectClass, "ptr", "J");
 	v8ObjectInitMethod = getMethodID(v8ObjectClass, "<init>", "(J)V", false);
@@ -393,7 +401,7 @@ void JNIUtil::initCache()
 	krollProxyOnPropertiesChangedMethod = getMethodID(krollProxyClass, "onPropertiesChanged",
 		"([[Ljava/lang/Object;)V", false);
 
-	krollRuntimeDispatchExceptionMethod = getMethodID(krollRuntimeClass, "dispatchException", "(Ljava/lang/String;Ljava/lang/String;Ljava/lang/String;ILjava/lang/String;I)V",true);
+	krollRuntimeDispatchExceptionMethod = getMethodID(krollRuntimeClass, "dispatchException", "(Ljava/lang/String;Ljava/lang/String;Ljava/lang/String;ILjava/lang/String;ILjava/lang/String;Ljava/lang/String;)V",true);
 	krollAssetHelperReadAssetMethod = getMethodID(krollAssetHelperClass, "readAsset", "(Ljava/lang/String;)Ljava/lang/String;", true);
 
 	krollLoggingLogWithDefaultLoggerMethod = getMethodID(krollLoggingClass, "logWithDefaultLogger", "(ILjava/lang/String;)V", true);

android/runtime/v8/src/native/JNIUtil.h

@@ -91,6 +91,7 @@ class JNIUtil
 	static jclass setClass;
 	static jclass outOfMemoryError;
 	static jclass throwableClass;
+	static jclass stackTraceElementClass;
 	static jclass nullPointerException;
 
 	// Titanium classes
@@ -128,6 +129,8 @@ class JNIUtil
 	static jmethodID longInitMethod;
 	static jmethodID numberDoubleValueMethod;
 	static jmethodID throwableGetMessageMethod;
+	static jmethodID throwableGetStackTraceMethod;
+	static jmethodID stackTraceElementToStringMethod;
 
 	// Titanium methods and fields
 	static jfieldID v8ObjectPtrField;

android/runtime/v8/src/native/JSException.cpp

@@ -1,14 +1,18 @@
 /**
  * Appcelerator Titanium Mobile
- * Copyright (c) 2011 by Appcelerator, Inc. All Rights Reserved.
+ * Copyright (c) 2011-2018 by Appcelerator, Inc. All Rights Reserved.
  * Licensed under the terms of the Apache Public License
  * Please see the LICENSE included with this distribution for details.
  */
+#include <cstring>
+#include <sstream>
+
 #include <jni.h>
 #include <v8.h>
 
 #include "JNIUtil.h"
 #include "TypeConverter.h"
+#include "V8Util.h"
 
 #include "JSException.h"
 
@@ -18,35 +22,59 @@ namespace titanium {
 
 Local<Value> JSException::fromJavaException(v8::Isolate* isolate, jthrowable javaException)
 {
-	JNIEnv *env = JNIScope::getEnv();
+	JNIEnv* env = JNIScope::getEnv();
 	if (!env) {
 		return GetJNIEnvironmentError(isolate);
 	}
 
-	env->ExceptionDescribe();
-
 	bool deleteRef = false;
 	if (!javaException) {
 		javaException = env->ExceptionOccurred();
-		env->ExceptionClear();
 		deleteRef = true;
 	}
+	env->ExceptionClear();
 
-	//env->ExceptionDescribe();
-
-	jstring message = (jstring) env->CallObjectMethod(javaException, JNIUtil::throwableGetMessageMethod);
-	if (!message) {
+	jstring javaMessage = (jstring) env->CallObjectMethod(javaException, JNIUtil::throwableGetMessageMethod);
+	if (!javaMessage) {
 		return THROW(isolate, "Java Exception occurred");
 	}
 
-	Local<Value> jsMessage = TypeConverter::javaStringToJsString(isolate, env, message);
-	env->DeleteLocalRef(message);
+	Local<Context> context = isolate->GetCurrentContext();
+
+	// Grab the top-level error message
+	Local<Value> message = TypeConverter::javaStringToJsString(isolate, env, javaMessage);
+	env->DeleteLocalRef(javaMessage);
+	// Create a JS Error holding this message
+	// We use .As<String> here because we know that the return value of TypeConverter::javaStringToJsString
+	// must be a String. Only other variant is Null when the javaMessage is null, which we already checked for above.
+	// We use .As<Object> on Error because an Error is an Object.
+	Local<Object> error = Exception::Error(message.As<String>()).As<Object>();
 
+	// Now loop through the java stack and generate a JS String from the result and assign to Local<String> stack
+	std::stringstream stackStream;
+	jobjectArray frames = (jobjectArray) env->CallObjectMethod(javaException, JNIUtil::throwableGetStackTraceMethod);
+	jsize frames_length = env->GetArrayLength(frames);
+	for (int i = 0; i < (frames_length > MAX_STACK ? MAX_STACK : frames_length); i++) {
+		jobject frame = env->GetObjectArrayElement(frames, i);
+		jstring javaStack = (jstring) env->CallObjectMethod(frame, JNIUtil::stackTraceElementToStringMethod);
+
+		const char* stackPtr = env->GetStringUTFChars(javaStack, NULL);
+		stackStream << std::endl << "    " << stackPtr;
+
+		env->ReleaseStringUTFChars(javaStack, stackPtr);
+		env->DeleteLocalRef(javaStack);
+	}
 	if (deleteRef) {
 		env->DeleteLocalRef(javaException);
 	}
+	stackStream << std::endl;
+
+	// Now explicitly assign our properly generated stacktrace
+	Local<String> javaStack = String::NewFromUtf8(isolate, stackStream.str().c_str());
+	error->Set(context, STRING_NEW(isolate, "nativeStack"), javaStack);
 
-	return isolate->ThrowException(jsMessage->ToString(isolate));
+	// throw it
+	return isolate->ThrowException(error);
 }
 
 }

android/runtime/v8/src/native/JSException.h

@@ -11,6 +11,8 @@
 #include <jni.h>
 #include <v8.h>
 
+#define MAX_STACK 10
+
 #define THROW(isolate, msg) \
 	isolate->ThrowException(v8::String::NewFromUtf8(isolate, msg))
 

android/runtime/v8/src/native/Proxy.cpp

@@ -166,6 +166,12 @@ static void onPropertyChangedForProxy(Isolate* isolate, Local<String> property,
 		env->DeleteLocalRef(javaValue);
 	}
 
+	if (env->ExceptionCheck()) {
+		JSException::fromJavaException(isolate);
+		env->ExceptionClear();
+		return;
+	}
+
 	// Store new property value on JS internal map.
 	setPropertyOnProxy(isolate, property, value, proxyObject);
 }
@@ -206,6 +212,12 @@ void Proxy::getIndexedProperty(uint32_t index, const PropertyCallbackInfo<Value>
 
 	proxy->unreferenceJavaObject(javaProxy);
 
+	if (env->ExceptionCheck()) {
+		JSException::fromJavaException(isolate);
+		env->ExceptionClear();
+		return;
+	}
+
 	Local<Value> result = TypeConverter::javaObjectToJsValue(isolate, env, value);
 	env->DeleteLocalRef(value);
 
@@ -237,6 +249,12 @@ void Proxy::setIndexedProperty(uint32_t index, Local<Value> value, const Propert
 		env->DeleteLocalRef(javaValue);
 	}
 
+	if (env->ExceptionCheck()) {
+		JSException::fromJavaException(isolate);
+		env->ExceptionClear();
+		return;
+	}
+
 	info.GetReturnValue().Set(value);
 }
 
@@ -272,6 +290,12 @@ void Proxy::hasListenersForEventType(const v8::FunctionCallbackInfo<v8::Value>&
 
 	env->DeleteLocalRef(krollObject);
 	env->DeleteLocalRef(javaEventType);
+
+	if (env->ExceptionCheck()) {
+		JSException::fromJavaException(isolate);
+		env->ExceptionClear();
+		return;
+	}
 }
 
 void Proxy::onEventFired(const v8::FunctionCallbackInfo<v8::Value>& args)
@@ -312,6 +336,12 @@ void Proxy::onEventFired(const v8::FunctionCallbackInfo<v8::Value>& args)
 	if (isNew) {
 		env->DeleteLocalRef(javaEventData);
 	}
+
+	if (env->ExceptionCheck()) {
+		JSException::fromJavaException(isolate);
+		env->ExceptionClear();
+		return;
+	}
 }
 
 Local<FunctionTemplate> Proxy::inheritProxyTemplate(Isolate* isolate,
@@ -498,7 +528,10 @@ void Proxy::proxyOnPropertiesChanged(const v8::FunctionCallbackInfo<v8::Value>&
 
 	proxy->unreferenceJavaObject(javaProxy);
 
-	return;
+	if (env->ExceptionCheck()) {
+		JSException::fromJavaException(isolate);
+		env->ExceptionClear();
+	}
 }
 
 void Proxy::dispose(Isolate* isolate)

android/runtime/v8/src/native/ProxyFactory.cpp

@@ -13,6 +13,7 @@
 #include "AndroidUtil.h"
 #include "JavaObject.h"
 #include "JNIUtil.h"
+#include "JSException.h"
 #include "KrollBindings.h"
 #include "Proxy.h"
 #include "TypeConverter.h"
@@ -166,6 +167,12 @@ jobject ProxyFactory::createJavaProxy(jclass javaClass, Local<Object> v8Proxy, c
 	env->DeleteLocalRef(javaArgs);
 	// We don't delete the global jclass reference...
 
+	if (env->ExceptionCheck()) {
+		JSException::fromJavaException(isolate);
+		env->ExceptionClear();
+		return NULL;
+	}
+
 	return javaProxy;
 }
 

android/runtime/v8/src/native/TypeConverter.cpp

@@ -114,7 +114,7 @@ jstring TypeConverter::jsValueToJavaString(v8::Isolate* isolate, v8::Local<v8::V
 
 jstring TypeConverter::jsValueToJavaString(v8::Isolate* isolate, JNIEnv *env, v8::Local<v8::Value> jsValue)
 {
-	if (jsValue->IsNull()) {
+	if (jsValue.IsEmpty() || jsValue->IsNullOrUndefined()) {
 		return NULL;
 	}
 

android/runtime/v8/src/native/V8Runtime.cpp

@@ -346,8 +346,7 @@ JNIEXPORT jboolean JNICALL Java_org_appcelerator_kroll_runtime_v8_V8Runtime_nati
 
 	// TODO Pump the message loop/queues until it's empty?
 	// while (v8::platform::PumpMessageLoop(V8Runtime::platform, V8Runtime:v8_isolate)) continue;
-    // v8::platform::RunIdleTasks(g_platform, isolate,
-    //                            50.0 / base::Time::kMillisecondsPerSecond);
+	// v8::platform::RunIdleTasks(g_platform, isolate, 50.0 / base::Time::kMillisecondsPerSecond);
 
 	// FIXME What is a good value to use here? We're basically giving it 100 ms to run right now
 	double deadline_in_s = V8Runtime::platform->MonotonicallyIncreasingTime() + 0.1;

android/runtime/v8/src/native/V8Runtime.h

@@ -19,6 +19,7 @@ class V8Runtime
 	static Persistent<Context> globalContext;
 	static Persistent<Object> krollGlobalObject;
 	static Persistent<Array> moduleContexts;
+
 	static Isolate* v8_isolate;
 	static Platform* platform;