Skip to content

Commit 351301e

Browse files
ammarahm-edammarahm-ed
committed
runtime: avoid throwing uncaught exceptions in java
1 parent 357fc11 commit 351301e

File tree

5 files changed

+102
-66
lines changed

5 files changed

+102
-66
lines changed

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

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -85,6 +85,7 @@ Runtime::Runtime(JNIEnv *jEnv, jobject runtime, int id)
8585
m_objectManager = new ObjectManager(m_runtime);
8686
m_loopTimer = new MessageLoopTimer();
8787
id_to_runtime_cache.Insert(id, this);
88+
pendingError = nullptr;
8889

8990
js_method_cache = new JSMethodCache(this);
9091

@@ -604,7 +605,7 @@ jobject Runtime::ConvertJsValueToJavaObject(JEnv &jEnv, napi_value value, int cl
604605
void
605606
Runtime::PassExceptionToJsNative(JNIEnv *jEnv, jobject obj, jthrowable exception, jstring message,
606607
jstring fullStackTrace, jstring jsStackTrace,
607-
jboolean isDiscarded) {
608+
jboolean isDiscarded, jboolean isPendingError) {
608609
napi_env napiEnv = env;
609610

610611
std::string errMsg = ArgConverter::jstringToString(message);
@@ -642,7 +643,9 @@ Runtime::PassExceptionToJsNative(JNIEnv *jEnv, jobject obj, jthrowable exception
642643

643644
// Pass err to JS
644645
NativeScriptException::CallJsFuncWithErr(env, errObj, isDiscarded);
645-
646+
if (isPendingError) {
647+
pendingError = napi_util::make_ref(env, errObj);
648+
}
646649
}
647650

648651
void

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

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -112,7 +112,7 @@ namespace tns {
112112

113113
void
114114
PassExceptionToJsNative(JNIEnv *env, jobject obj, jthrowable exception, jstring message,
115-
jstring fullStackTrace, jstring jsStackTrace, jboolean isDiscarded);
115+
jstring fullStackTrace, jstring jsStackTrace, jboolean isDiscarded, jboolean isPendingError);
116116

117117
void PassUncaughtExceptionFromWorkerToMainHandler(napi_value message, napi_value stackTrace,
118118
napi_value filename, int lineno);
@@ -123,7 +123,24 @@ namespace tns {
123123

124124
bool is_destroying = false;
125125

126+
napi_value getPendingError() {
127+
if (!pendingError) return nullptr;
128+
napi_value error = napi_util::get_ref_value(env, pendingError);
129+
napi_delete_reference(env, pendingError);
130+
pendingError = nullptr;
131+
return error;
132+
}
133+
134+
void clearPendingError() {
135+
if (!pendingError) return;
136+
napi_delete_reference(env, pendingError);
137+
pendingError = nullptr;
138+
}
139+
126140
private:
141+
napi_ref pendingError;
142+
143+
127144
Runtime(JNIEnv *env, jobject runtime, int id);
128145

129146
static napi_value GlobalAccessorCallback(napi_env env, napi_callback_info info);

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -241,14 +241,14 @@ extern "C" JNIEXPORT void Java_com_tns_Runtime_unlock(JNIEnv* env, jobject obj,
241241
}
242242
}
243243

244-
extern "C" JNIEXPORT void Java_com_tns_Runtime_passExceptionToJsNative(JNIEnv* jEnv, jobject obj, jint runtimeId, jthrowable exception, jstring message, jstring fullStackTrace, jstring jsStackTrace, jboolean isDiscarded) {
244+
extern "C" JNIEXPORT void Java_com_tns_Runtime_passExceptionToJsNative(JNIEnv* jEnv, jobject obj, jint runtimeId, jthrowable exception, jstring message, jstring fullStackTrace, jstring jsStackTrace, jboolean isDiscarded, jboolean isPendingError) {
245245
auto runtime = TryGetRuntime(runtimeId);
246246
if (runtime == nullptr) return;
247247

248248
NapiScope scope(runtime->GetNapiEnv());
249249

250250
try {
251-
runtime->PassExceptionToJsNative(jEnv, obj, exception, message, fullStackTrace, jsStackTrace, isDiscarded);
251+
runtime->PassExceptionToJsNative(jEnv, obj, exception, message, fullStackTrace, jsStackTrace, isDiscarded, isPendingError);
252252
} catch (NativeScriptException& e) {
253253
e.ReThrowToJava(runtime->GetNapiEnv());
254254
} catch (std::exception e) {

‎test-app/runtime/src/main/cpp/runtime/metadata/MetadataNode.cpp‎

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1986,13 +1986,21 @@ napi_value MetadataNode::MethodCallback(napi_env env, napi_callback_info info) {
19861986
}
19871987
}
19881988

1989+
19891990
if (argc == 0 && methodName == PROP_KEY_VALUEOF) {
19901991
return jsThis;
19911992
} else {
1993+
Runtime::GetRuntime(env)->clearPendingError();
19921994
bool isFromInterface = initialCallbackData->node->IsNodeTypeInterface();
1993-
return CallbackHandlers::CallJavaMethod(env, jsThis, *className, methodName, entry,
1995+
napi_value result = CallbackHandlers::CallJavaMethod(env, jsThis, *className, methodName, entry,
19941996
isFromInterface, first.isStatic, info,
19951997
argc, argv.data());
1998+
napi_value error;
1999+
error = Runtime::GetRuntime(env)->getPendingError();
2000+
if (error) {
2001+
throw NativeScriptException(env, error);
2002+
}
2003+
return result;
19962004
}
19972005

19982006
} catch (NativeScriptException &e) {

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

Lines changed: 68 additions & 60 deletions
Original file line numberDiff line numberDiff line change
@@ -37,8 +37,6 @@
3737
import java.util.concurrent.RunnableFuture;
3838
import java.util.concurrent.atomic.AtomicInteger;
3939

40-
import dalvik.annotation.optimization.FastNative;
41-
4240
public class Runtime {
4341
private native void initNativeScript(int runtimeId, String filesPath, String nativeLibDir, boolean verboseLoggingEnabled, boolean isDebuggable, String packageName,
4442
Object[] v8Options, String callingDir, int maxLogcatObjectSize, boolean forceLog);
@@ -61,7 +59,7 @@ private native void initNativeScript(int runtimeId, String filesPath, String nat
6159

6260
private native void unlock(int runtimeId);
6361

64-
private native void passExceptionToJsNative(int runtimeId, Throwable ex, String message, String fullStackTrace, String jsStackTrace, boolean isDiscarded);
62+
private native void passExceptionToJsNative(int runtimeId, Throwable ex, String message, String fullStackTrace, String jsStackTrace, boolean isDiscarded, boolean isPendingError);
6563

6664
private static native int getCurrentRuntimeId();
6765

@@ -82,21 +80,21 @@ private native void initNativeScript(int runtimeId, String filesPath, String nat
8280
private static native void ResetDateTimeConfigurationCache(int runtimeId);
8381

8482
void passUncaughtExceptionToJs(Throwable ex, String message, String fullStackTrace, String jsStackTrace) {
85-
passExceptionToJsNative(getRuntimeId(), ex, message, fullStackTrace, jsStackTrace, false);
83+
passExceptionToJsNative(getRuntimeId(), ex, message, fullStackTrace, jsStackTrace, false, false);
8684
}
8785

88-
void passDiscardedExceptionToJs(Throwable ex, String prefix) {
86+
void passExceptionToJS(Throwable ex, boolean isPendingError, boolean isDiscarded) {
8987
//String message = prefix + ex.getMessage();
9088
// we'd better not prefix the error with something like - Error on "main" thread for reportSupressedException
9189
// as it doesn't seem very useful for the users
92-
passExceptionToJsNative(getRuntimeId(), ex, ex.getMessage(), Runtime.getStackTraceErrorMessage(ex), Runtime.getJSStackTrace(ex), true);
90+
passExceptionToJsNative(getRuntimeId(), ex, ex.getMessage(), Runtime.getStackTraceErrorMessage(ex), Runtime.getJSStackTrace(ex), isDiscarded, isPendingError);
9391
}
9492

9593
public static void passSuppressedExceptionToJs(Throwable ex, String methodName) {
9694
com.tns.Runtime runtime = com.tns.Runtime.getCurrentRuntime();
9795
if (runtime != null) {
9896
String errorMessage = "Error on \"" + Thread.currentThread().getName() + "\" thread for " + methodName + "\n";
99-
runtime.passDiscardedExceptionToJs(ex, "");
97+
runtime.passExceptionToJS(ex, false , false);
10098
}
10199
}
102100

@@ -668,78 +666,91 @@ private void init(Logger logger, String appName, String nativeLibDir, File rootD
668666

669667
@RuntimeCallable
670668
public void enableVerboseLogging() {
671-
672669
logger.setEnabled(true);
673670
ProxyGenerator.IsLogEnabled = true;
674671
}
675672

676673

677674
@RuntimeCallable
678675
public void disableVerboseLogging() {
679-
// logger.setEnabled(false);
680-
// ProxyGenerator.IsLogEnabled = false;
676+
logger.setEnabled(false);
677+
ProxyGenerator.IsLogEnabled = false;
681678
}
682679

683-
public void run() throws NativeScriptException {
680+
public void run() {
684681
ManualInstrumentation.Frame frame = ManualInstrumentation.start("Runtime.run");
685682
try {
686683
String mainModule = Module.bootstrapApp();
687684
runModule(new File(mainModule));
685+
} catch (NativeScriptException e){
686+
passExceptionToJS(e, false, false);
688687
} finally {
689688
frame.close();
690689
}
691690
}
692691

693-
public void runModule(File jsFile) throws NativeScriptException {
694-
String filePath = jsFile.getPath();
695-
runModule(getRuntimeId(), filePath);
692+
public void runModule(File jsFile) {
693+
try {
694+
String filePath = jsFile.getPath();
695+
runModule(getRuntimeId(), filePath);
696+
} catch (NativeScriptException ex) {
697+
passExceptionToJS(ex, false, false);
698+
}
696699
}
697700

698-
public Object runScript(File jsFile) throws NativeScriptException {
699-
return this.runScript(jsFile, true);
701+
public Object runScript(File jsFile) {
702+
try {
703+
return this.runScript(jsFile, true);
704+
} catch (NativeScriptException ex) {
705+
passExceptionToJS(ex, false, false);
706+
return null;
707+
}
700708
}
701709

702-
public Object runScript(File jsFile, final boolean waitForResultOnMainThread) throws NativeScriptException {
710+
public Object runScript(File jsFile, final boolean waitForResultOnMainThread) {
703711
Object result = null;
712+
try {
713+
if (jsFile.exists() && jsFile.isFile()) {
714+
final String filePath = jsFile.getAbsolutePath();
704715

705-
if (jsFile.exists() && jsFile.isFile()) {
706-
final String filePath = jsFile.getAbsolutePath();
707-
708-
boolean isWorkThread = threadScheduler.getThread().equals(Thread.currentThread());
716+
boolean isWorkThread = threadScheduler.getThread().equals(Thread.currentThread());
709717

710-
if (isWorkThread) {
711-
result = runScript(getRuntimeId(), filePath);
712-
} else {
713-
final Object[] arr = new Object[2];
714-
715-
Runnable r = new Runnable() {
716-
@Override
717-
public void run() {
718-
synchronized (this) {
719-
try {
720-
arr[0] = runScript(getRuntimeId(), filePath);
721-
} finally {
722-
this.notify();
723-
arr[1] = Boolean.TRUE;
718+
if (isWorkThread) {
719+
result = runScript(getRuntimeId(), filePath);
720+
} else {
721+
final Object[] arr = new Object[2];
722+
723+
Runnable r = new Runnable() {
724+
@Override
725+
public void run() {
726+
synchronized (this) {
727+
try {
728+
arr[0] = runScript(getRuntimeId(), filePath);
729+
} finally {
730+
this.notify();
731+
arr[1] = Boolean.TRUE;
732+
}
724733
}
725734
}
726-
}
727-
};
735+
};
728736

729-
boolean success = threadScheduler.post(r);
737+
boolean success = threadScheduler.post(r);
730738

731-
if (success) {
732-
synchronized (r) {
733-
try {
734-
if (arr[1] == null && waitForResultOnMainThread) {
735-
r.wait();
739+
if (success) {
740+
synchronized (r) {
741+
try {
742+
if (arr[1] == null && waitForResultOnMainThread) {
743+
r.wait();
744+
}
745+
} catch (InterruptedException e) {
746+
result = e;
736747
}
737-
} catch (InterruptedException e) {
738-
result = e;
739748
}
740749
}
741750
}
742751
}
752+
} catch (NativeScriptException ex) {
753+
passExceptionToJS(ex, false, false);
743754
}
744755

745756
return result;
@@ -1116,9 +1127,6 @@ private Object getJavaObjectByID(int javaObjectID) throws Exception {
11161127
}
11171128
}
11181129

1119-
// Log.d(DEFAULT_LOG_TAG,
1120-
// "Platform.getJavaObjectByID found strong object with id:" +
1121-
// javaObjectID);
11221130
return instance;
11231131
}
11241132

@@ -1344,13 +1352,13 @@ private Object dispatchCallJSMethodNative(final int javaObjectID, Class<?> claz,
13441352
try {
13451353
ret = callJSMethodNative(getRuntimeId(), javaObjectID, claz, methodName, returnType, isConstructor, packagedArgs);
13461354
} catch (NativeScriptException e) {
1347-
if (discardUncaughtJsExceptions) {
1355+
// if (discardUncaughtJsExceptions) {
13481356
String errorMessage = "Error on \"" + Thread.currentThread().getName() + "\" thread for callJSMethodNative\n";
1349-
android.util.Log.w("Warning", "NativeScript discarding uncaught JS exception!");
1350-
passDiscardedExceptionToJs(e, errorMessage);
1351-
} else {
1352-
throw e;
1353-
}
1357+
// android.util.Log.w("Warning", "NativeScript discarding uncaught JS exception!");
1358+
passExceptionToJS(e, true, true);
1359+
// } else {
1360+
// throw e;
1361+
// }
13541362
}
13551363
} else {
13561364
final Object[] arr = new Object[2];
@@ -1365,13 +1373,13 @@ public void run() {
13651373
final Object[] packagedArgs = packageArgs(tmpArgs);
13661374
arr[0] = callJSMethodNative(getRuntimeId(), javaObjectID, claz, methodName, returnType, isCtor, packagedArgs);
13671375
} catch (NativeScriptException e) {
1368-
if (discardUncaughtJsExceptions) {
1376+
// if (discardUncaughtJsExceptions) {
13691377
String errorMessage = "Error on \"" + Thread.currentThread().getName() + "\" thread for callJSMethodNative\n";
1370-
passDiscardedExceptionToJs(e, errorMessage);
1371-
android.util.Log.w("Warning", "NativeScript discarding uncaught JS exception!");
1372-
} else {
1373-
throw e;
1374-
}
1378+
passExceptionToJS(e, true, false);
1379+
// android.util.Log.w("Warning", "NativeScript discarding uncaught JS exception!");
1380+
// } else {
1381+
// throw e;
1382+
// }
13751383
} finally {
13761384
this.notify();
13771385
arr[1] = Boolean.TRUE;

0 commit comments

Comments
 (0)