Skip to content

Improve error reporting #1525

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Oct 23, 2019
Merged

Improve error reporting #1525

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
546cf8f
Improve error reporting
vtrifonov Oct 22, 2019
f8a21f1
Fixed exception message check
vtrifonov Oct 22, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .gitignore
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,3 +22,4 @@ bin/
.classpath
android-runtime.iml
test-app/build-tools/*.log
test-app/analytics/build-statistics.json
30 changes: 15 additions & 15 deletions test-app/app/src/main/assets/app/tests/exceptionHandlingTests.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -302,7 +302,7 @@ describe("Tests exception handling ", function () {
errMsg = e.toString();
}
expect(exceptionCaught).toBe(true);
expect(errMsg).toBe("Error: Unknown error. Cannot get error message.");
expect(errMsg).toBe("Error: com.tns.tests.ExceptionHandlingTest$BadException");
});

it("should successfully catch syntax errors", function () {
Expand All @@ -320,18 +320,18 @@ describe("Tests exception handling ", function () {
expect(errMsg).toContain("File: (file:///data/data/com.tns.testapplication/files/app/tests/syntaxErrors.js:3:4)");
});

// run this test only for API level bigger than 25 as we have handling there
if(android.os.Build.VERSION.SDK_INT > 25 && android.os.Build.CPU_ABI != "x86" && android.os.Build.CPU_ABI != "x86_64") {
it("Should handle SIGABRT and throw a NativeScript exception when incorrectly calling JNI methods", function () {
let myClassInstance = new com.tns.tests.MyTestBaseClass3();
// public void callMeWithAString(java.lang.String[] stringArr, Runnable arbitraryInterface)
try {
myClassInstance.callMeWithAString("stringVal", new java.lang.Runnable({ run: () => {} }))
} catch (e) {
android.util.Log.d("~~~~~", "~~~~~~~~ " + e.toString());

expect(e.toString()).toContain("SIGABRT");
}
});
}
// run this test only for API level bigger than 25 as we have handling there
if(android.os.Build.VERSION.SDK_INT > 25 && android.os.Build.CPU_ABI != "x86" && android.os.Build.CPU_ABI != "x86_64") {
it("Should handle SIGABRT and throw a NativeScript exception when incorrectly calling JNI methods", function () {
let myClassInstance = new com.tns.tests.MyTestBaseClass3();
// public void callMeWithAString(java.lang.String[] stringArr, Runnable arbitraryInterface)
try {
myClassInstance.callMeWithAString("stringVal", new java.lang.Runnable({ run: () => {} }))
} catch (e) {
android.util.Log.d("~~~~~", "~~~~~~~~ " + e.toString());

expect(e.toString()).toContain("SIGABRT");
}
});
}
});
2 changes: 1 addition & 1 deletion test-app/app/src/main/java/com/tns/NativeScriptUncaughtExceptionHandler.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ public void uncaughtException(Thread thread, Throwable ex) {
Runtime runtime = Runtime.getCurrentRuntime();

if (runtime != null) {
runtime.passUncaughtExceptionToJs(ex, ex.getMessage(), Runtime.getJSStackTrace(ex), stackTraceErrorMessage);
runtime.passUncaughtExceptionToJs(ex, ex.getMessage(), stackTraceErrorMessage, Runtime.getJSStackTrace(ex));
}
} catch (Throwable t) {
if (Util.isDebuggableApp(context)) {
Expand Down
30 changes: 27 additions & 3 deletions test-app/runtime/src/main/cpp/NativeScriptException.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -140,6 +140,9 @@ void NativeScriptException::Init() {

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);

NATIVESCRIPTEXCEPTION_GET_MESSAGE_METHOD_ID = env.GetStaticMethodID(NATIVESCRIPTEXCEPTION_CLASS, "getMessage", "(Ljava/lang/Throwable;)Ljava/lang/String;");
assert(NATIVESCRIPTEXCEPTION_GET_MESSAGE_METHOD_ID != nullptr);
}

// ON V8 UNCAUGHT EXCEPTION
Expand Down Expand Up @@ -204,7 +207,8 @@ Local<Value> NativeScriptException::WrapJavaToJsException() {

Local<Value> NativeScriptException::GetJavaExceptionFromEnv(const JniLocalRef& exc, JEnv& env) {
auto errMsg = GetExceptionMessage(env, exc);
DEBUG_WRITE("Error during java interop errorMessage %s", errMsg.c_str());
auto stackTrace = GetExceptionStackTrace(env, exc);
DEBUG_WRITE("Error during java interop errorMessage: %s\n stackTrace:\n %s", errMsg.c_str(), stackTrace.c_str());

auto isolate = Isolate::GetCurrent();
auto objectManager = Runtime::GetObjectManager(isolate);
Expand All @@ -223,6 +227,10 @@ Local<Value> NativeScriptException::GetJavaExceptionFromEnv(const JniLocalRef& e
auto context = isolate->GetCurrentContext();
errObj->Set(context, V8StringConstants::GetNativeException(isolate), nativeExceptionObject);

string jsStackTraceMessage = GetErrorStackTrace(Exception::GetStackTrace(errObj));
errObj->Set(context, V8StringConstants::GetStack(isolate), ArgConverter::ConvertToV8String(isolate, jsStackTraceMessage));
errObj->Set(context, V8StringConstants::GetStackTrace(isolate), ArgConverter::ConvertToV8String(isolate, jsStackTraceMessage + stackTrace));

return errObj;
}

Expand Down Expand Up @@ -362,15 +370,17 @@ string NativeScriptException::GetErrorStackTrace(const Local<StackTrace>& stackT
auto lineNumber = frame->GetLineNumber();
auto column = frame->GetColumn();

ss << "\t" << (i > 0 ? "at " : "") << funcName.c_str() << "(" << srcName.c_str() << ":" << lineNumber << ":" << column << ")" << endl;
auto startString = i == 0 ? "" : "\t";

ss << startString << (i > 0 ? "at " : "") << funcName.c_str() << "(" << srcName.c_str() << ":" << lineNumber << ":" << column << ")" << endl;
}

return ss.str();
}

string NativeScriptException::GetExceptionMessage(JEnv& env, jthrowable exception) {
string errMsg;
JniLocalRef msg(env.CallStaticObjectMethod(NATIVESCRIPTEXCEPTION_CLASS, NATIVESCRIPTEXCEPTION_GET_STACK_TRACE_AS_STRING_METHOD_ID, exception));
JniLocalRef msg(env.CallStaticObjectMethod(NATIVESCRIPTEXCEPTION_CLASS, NATIVESCRIPTEXCEPTION_GET_MESSAGE_METHOD_ID, exception));

const char* msgStr = env.GetStringUTFChars(msg, nullptr);

Expand All @@ -381,9 +391,23 @@ string NativeScriptException::GetExceptionMessage(JEnv& env, jthrowable exceptio
return errMsg;
}

string NativeScriptException::GetExceptionStackTrace(JEnv& env, jthrowable exception) {
string errStackTrace;
JniLocalRef msg(env.CallStaticObjectMethod(NATIVESCRIPTEXCEPTION_CLASS, NATIVESCRIPTEXCEPTION_GET_STACK_TRACE_AS_STRING_METHOD_ID, exception));

const char* msgStr = env.GetStringUTFChars(msg, nullptr);

errStackTrace.append(msgStr);

env.ReleaseStringUTFChars(msg, msgStr);

return errStackTrace;
}

jclass NativeScriptException::RUNTIME_CLASS = nullptr;
jclass NativeScriptException::THROWABLE_CLASS = nullptr;
jclass NativeScriptException::NATIVESCRIPTEXCEPTION_CLASS = nullptr;
jmethodID NativeScriptException::NATIVESCRIPTEXCEPTION_JSVALUE_CTOR_ID = nullptr;
jmethodID NativeScriptException::NATIVESCRIPTEXCEPTION_THROWABLE_CTOR_ID = nullptr;
jmethodID NativeScriptException::NATIVESCRIPTEXCEPTION_GET_MESSAGE_METHOD_ID = nullptr;
jmethodID NativeScriptException::NATIVESCRIPTEXCEPTION_GET_STACK_TRACE_AS_STRING_METHOD_ID = nullptr;
8 changes: 7 additions & 1 deletion test-app/runtime/src/main/cpp/NativeScriptException.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,10 +52,15 @@ class NativeScriptException {
JniLocalRef TryGetJavaThrowableObject(JEnv& env, const v8::Local<v8::Object>& jsObj);

/*
* Gets java exception stack message from jthrowable
* Gets java exception message from jthrowable
*/
std::string GetExceptionMessage(JEnv& env, jthrowable exception);

/*
* Gets java exception stack trace from jthrowable
*/
std::string GetExceptionStackTrace(JEnv& env, jthrowable exception);

/*
* Gets the member m_javaException, wraps it and creates a javascript error object from it
*/
Expand Down Expand Up @@ -92,6 +97,7 @@ class NativeScriptException {
static jclass NATIVESCRIPTEXCEPTION_CLASS;
static jmethodID NATIVESCRIPTEXCEPTION_JSVALUE_CTOR_ID;
static jmethodID NATIVESCRIPTEXCEPTION_THROWABLE_CTOR_ID;
static jmethodID NATIVESCRIPTEXCEPTION_GET_MESSAGE_METHOD_ID;
static jmethodID NATIVESCRIPTEXCEPTION_GET_STACK_TRACE_AS_STRING_METHOD_ID;

static void PrintErrorMessage(const std::string& errorMessage);
Expand Down
12 changes: 8 additions & 4 deletions test-app/runtime/src/main/java/com/tns/NativeScriptException.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,12 +39,11 @@ public String getIncomingStackTrace() {

@RuntimeCallable
public static String getStackTraceAsString(Throwable ex) {
String errMessage;
String errMessage = "";
try {
errMessage = ex.toString();
for (StackTraceElement frame: ex.getStackTrace()) {
errMessage += "\n ";
errMessage += frame;
errMessage += "\t";
errMessage += frame + "\n";
}

Throwable cause = ex.getCause();
Expand All @@ -57,4 +56,9 @@ public static String getStackTraceAsString(Throwable ex) {
}
return errMessage;
}

@RuntimeCallable
public static String getMessage(Throwable ex) {
return ex.toString();
}
}