Skip to content

Commit 5c86811

Browse files
vtrifonovvtrifonov
authored
Merge pull request #1525 from NativeScript/trifonov/error-reporting-fixes
Improve error reporting
2 parents ff679d6 + f8a21f1 commit 5c86811

File tree

6 files changed

+59
-24
lines changed

6 files changed

+59
-24
lines changed

‎.gitignore‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,3 +22,4 @@ bin/
2222
.classpath
2323
android-runtime.iml
2424
test-app/build-tools/*.log
25+
test-app/analytics/build-statistics.json

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

Lines changed: 15 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -302,7 +302,7 @@ describe("Tests exception handling ", function () {
302302
errMsg = e.toString();
303303
}
304304
expect(exceptionCaught).toBe(true);
305-
expect(errMsg).toBe("Error: Unknown error. Cannot get error message.");
305+
expect(errMsg).toBe("Error: com.tns.tests.ExceptionHandlingTest$BadException");
306306
});
307307

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

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

‎test-app/app/src/main/java/com/tns/NativeScriptUncaughtExceptionHandler.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@ public void uncaughtException(Thread thread, Throwable ex) {
3131
Runtime runtime = Runtime.getCurrentRuntime();
3232

3333
if (runtime != null) {
34-
runtime.passUncaughtExceptionToJs(ex, ex.getMessage(), Runtime.getJSStackTrace(ex), stackTraceErrorMessage);
34+
runtime.passUncaughtExceptionToJs(ex, ex.getMessage(), stackTraceErrorMessage, Runtime.getJSStackTrace(ex));
3535
}
3636
} catch (Throwable t) {
3737
if (Util.isDebuggableApp(context)) {

‎test-app/runtime/src/main/cpp/NativeScriptException.cpp‎

Lines changed: 27 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -140,6 +140,9 @@ void NativeScriptException::Init() {
140140

141141
NATIVESCRIPTEXCEPTION_GET_STACK_TRACE_AS_STRING_METHOD_ID = env.GetStaticMethodID(NATIVESCRIPTEXCEPTION_CLASS, "getStackTraceAsString", "(Ljava/lang/Throwable;)Ljava/lang/String;");
142142
assert(NATIVESCRIPTEXCEPTION_GET_STACK_TRACE_AS_STRING_METHOD_ID != nullptr);
143+
144+
NATIVESCRIPTEXCEPTION_GET_MESSAGE_METHOD_ID = env.GetStaticMethodID(NATIVESCRIPTEXCEPTION_CLASS, "getMessage", "(Ljava/lang/Throwable;)Ljava/lang/String;");
145+
assert(NATIVESCRIPTEXCEPTION_GET_MESSAGE_METHOD_ID != nullptr);
143146
}
144147

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

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

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

230+
string jsStackTraceMessage = GetErrorStackTrace(Exception::GetStackTrace(errObj));
231+
errObj->Set(context, V8StringConstants::GetStack(isolate), ArgConverter::ConvertToV8String(isolate, jsStackTraceMessage));
232+
errObj->Set(context, V8StringConstants::GetStackTrace(isolate), ArgConverter::ConvertToV8String(isolate, jsStackTraceMessage + stackTrace));
233+
226234
return errObj;
227235
}
228236

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

365-
ss << "\t" << (i > 0 ? "at " : "") << funcName.c_str() << "(" << srcName.c_str() << ":" << lineNumber << ":" << column << ")" << endl;
373+
auto startString = i == 0 ? "" : "\t";
374+
375+
ss << startString << (i > 0 ? "at " : "") << funcName.c_str() << "(" << srcName.c_str() << ":" << lineNumber << ":" << column << ")" << endl;
366376
}
367377

368378
return ss.str();
369379
}
370380

371381
string NativeScriptException::GetExceptionMessage(JEnv& env, jthrowable exception) {
372382
string errMsg;
373-
JniLocalRef msg(env.CallStaticObjectMethod(NATIVESCRIPTEXCEPTION_CLASS, NATIVESCRIPTEXCEPTION_GET_STACK_TRACE_AS_STRING_METHOD_ID, exception));
383+
JniLocalRef msg(env.CallStaticObjectMethod(NATIVESCRIPTEXCEPTION_CLASS, NATIVESCRIPTEXCEPTION_GET_MESSAGE_METHOD_ID, exception));
374384

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

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

394+
string NativeScriptException::GetExceptionStackTrace(JEnv& env, jthrowable exception) {
395+
string errStackTrace;
396+
JniLocalRef msg(env.CallStaticObjectMethod(NATIVESCRIPTEXCEPTION_CLASS, NATIVESCRIPTEXCEPTION_GET_STACK_TRACE_AS_STRING_METHOD_ID, exception));
397+
398+
const char* msgStr = env.GetStringUTFChars(msg, nullptr);
399+
400+
errStackTrace.append(msgStr);
401+
402+
env.ReleaseStringUTFChars(msg, msgStr);
403+
404+
return errStackTrace;
405+
}
406+
384407
jclass NativeScriptException::RUNTIME_CLASS = nullptr;
385408
jclass NativeScriptException::THROWABLE_CLASS = nullptr;
386409
jclass NativeScriptException::NATIVESCRIPTEXCEPTION_CLASS = nullptr;
387410
jmethodID NativeScriptException::NATIVESCRIPTEXCEPTION_JSVALUE_CTOR_ID = nullptr;
388411
jmethodID NativeScriptException::NATIVESCRIPTEXCEPTION_THROWABLE_CTOR_ID = nullptr;
412+
jmethodID NativeScriptException::NATIVESCRIPTEXCEPTION_GET_MESSAGE_METHOD_ID = nullptr;
389413
jmethodID NativeScriptException::NATIVESCRIPTEXCEPTION_GET_STACK_TRACE_AS_STRING_METHOD_ID = nullptr;

‎test-app/runtime/src/main/cpp/NativeScriptException.h‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -52,10 +52,15 @@ class NativeScriptException {
5252
JniLocalRef TryGetJavaThrowableObject(JEnv& env, const v8::Local<v8::Object>& jsObj);
5353

5454
/*
55-
* Gets java exception stack message from jthrowable
55+
* Gets java exception message from jthrowable
5656
*/
5757
std::string GetExceptionMessage(JEnv& env, jthrowable exception);
5858

59+
/*
60+
* Gets java exception stack trace from jthrowable
61+
*/
62+
std::string GetExceptionStackTrace(JEnv& env, jthrowable exception);
63+
5964
/*
6065
* Gets the member m_javaException, wraps it and creates a javascript error object from it
6166
*/
@@ -92,6 +97,7 @@ class NativeScriptException {
9297
static jclass NATIVESCRIPTEXCEPTION_CLASS;
9398
static jmethodID NATIVESCRIPTEXCEPTION_JSVALUE_CTOR_ID;
9499
static jmethodID NATIVESCRIPTEXCEPTION_THROWABLE_CTOR_ID;
100+
static jmethodID NATIVESCRIPTEXCEPTION_GET_MESSAGE_METHOD_ID;
95101
static jmethodID NATIVESCRIPTEXCEPTION_GET_STACK_TRACE_AS_STRING_METHOD_ID;
96102

97103
static void PrintErrorMessage(const std::string& errorMessage);

‎test-app/runtime/src/main/java/com/tns/NativeScriptException.java‎

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -39,12 +39,11 @@ public String getIncomingStackTrace() {
3939

4040
@RuntimeCallable
4141
public static String getStackTraceAsString(Throwable ex) {
42-
String errMessage;
42+
String errMessage = "";
4343
try {
44-
errMessage = ex.toString();
4544
for (StackTraceElement frame: ex.getStackTrace()) {
46-
errMessage += "\n ";
47-
errMessage += frame;
45+
errMessage += "\t";
46+
errMessage += frame + "\n";
4847
}
4948

5049
Throwable cause = ex.getCause();
@@ -57,4 +56,9 @@ public static String getStackTraceAsString(Throwable ex) {
5756
}
5857
return errMessage;
5958
}
59+
60+
@RuntimeCallable
61+
public static String getMessage(Throwable ex) {
62+
return ex.toString();
63+
}
6064
}

0 commit comments

Comments
 (0)