Merge pull request #1043 from NativeScript/bektchiev/catch-objc-excep…

…tions

fix(bridge): Catch ObjC Exceptions and throw them to JS

src/NativeScript/CMakeLists.txt

@@ -118,7 +118,7 @@ set(SOURCE_FILES
     Calling/FFICallPrototype.cpp
     Calling/CFunctionWrapper.mm
     Calling/FFIFunctionCallback.cpp
-    Calling/FunctionWrapper.cpp
+    Calling/FunctionWrapper.mm
     GlobalObject.mm
     GlobalObject.moduleLoader.mm
     inspector/CachedResource.mm

src/NativeScript/Calling/FunctionWrapper.cpp → src/NativeScript/Calling/FunctionWrapper.mm

@@ -8,19 +8,37 @@
 
 #include "FFICache.h"
 #include "FFICall.h"
+#include "ObjCTypes.h"
+#include <JavaScriptCore/JSObjectRef.h>
 #include <JavaScriptCore/JSPromiseDeferred.h>
 #include <JavaScriptCore/StrongInlines.h>
 #include <JavaScriptCore/interpreter/FrameTracers.h>
 #include <JavaScriptCore/interpreter/Interpreter.h>
+#include <JavaScriptCore/runtime/Error.h>
 #include <dispatch/dispatch.h>
 #include <malloc/malloc.h>
 
 #include "FunctionWrapper.h"
 #include "Metadata.h"
 
+#import "TNSRuntime.h"
+
 namespace NativeScript {
 using namespace JSC;
 
+JSObject* createErrorFromNSException(TNSRuntime* runtime, ExecState* execState, NSException* exception) {
+    JSObject* error = createError(execState, [[exception reason] UTF8String]);
+
+    JSGlobalContextRef context = runtime.globalContext;
+    JSValueRef wrappedException = [runtime convertObject:exception];
+    JSStringRef nativeExceptionPropertyName = JSStringCreateWithUTF8CString("nativeException");
+    JSObjectSetProperty(context, (JSObjectRef)error, nativeExceptionPropertyName,
+                        wrappedException, kJSPropertyAttributeNone, NULL);
+    JSStringRelease(nativeExceptionPropertyName);
+
+    return error;
+}
+
 const ClassInfo FunctionWrapper::s_info = { "FunctionWrapper", &Base::s_info, nullptr, nullptr, CREATE_METHOD_TABLE(FunctionWrapper) };
 
 void FunctionWrapper::initializeFunctionWrapper(VM& vm, size_t maxParametersCount) {
@@ -71,16 +89,20 @@ EncodedJSValue JSC_HOST_CALL FunctionWrapper::call(ExecState* execState) {
         return JSValue::encode(scope.exception());
     }
 
-    {
-        JSLock::DropAllLocks locksDropper(execState);
-        ffi_call(callee->cif().get(), FFI_FN(invocation.function), invocation._buffer + callee->returnOffset(), reinterpret_cast<void**>(invocation._buffer + callee->argsArrayOffset()));
-    }
+    @try {
+        {
+            JSLock::DropAllLocks locksDropper(execState);
+            ffi_call(callee->cif().get(), FFI_FN(invocation.function), invocation.resultBuffer(), reinterpret_cast<void**>(invocation._buffer + callee->argsArrayOffset()));
+        }
 
-    JSValue result = callee->returnType().read(execState, invocation._buffer + callee->returnOffset(), callee->returnTypeCell().get());
+        JSValue result = callee->returnType().read(execState, invocation._buffer + callee->returnOffset(), callee->returnTypeCell().get());
 
-    callee->postCall(execState, invocation);
+        callee->postCall(execState, invocation);
 
-    return JSValue::encode(result);
+        return JSValue::encode(result);
+    } @catch (NSException* exception) {
+        return throwVMError(execState, scope, createErrorFromNSException([TNSRuntime current], execState, exception));
+    }
 }
 
 JSObject* FunctionWrapper::async(ExecState* execState, JSValue thisValue, const ArgList& arguments) {
@@ -120,37 +142,51 @@ JSObject* FunctionWrapper::async(ExecState* execState, JSValue thisValue, const
     JSPromiseDeferred* deferred = JSPromiseDeferred::create(execState, execState->lexicalGlobalObject());
     auto* releasePool = new ReleasePoolBase::Item(releasePoolHolder.relinquish());
     __block Strong<FunctionWrapper> callee(execState->vm(), this);
+    __block TNSRuntime* runtime = [TNSRuntime current];
 
-    dispatch_async(dispatch_get_global_queue(0, 0), ^{
+    dispatch_async(dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_DEFAULT, 0), ^{
       JSC::VM& vm = fakeExecState->vm();
       auto scope = DECLARE_CATCH_SCOPE(vm);
 
-      ffi_call(call->cif().get(), FFI_FN(invocation->function), invocation->resultBuffer(), reinterpret_cast<void**>(invocation->_buffer + call->argsArrayOffset()));
+      NSException* nsexception = nullptr;
+      @try {
+
+          ffi_call(call->cif().get(), FFI_FN(invocation->function), invocation->resultBuffer(), reinterpret_cast<void**>(invocation->_buffer + call->argsArrayOffset()));
+      } @catch (NSException* ex) {
+          nsexception = ex;
+      }
 
       // Native call is made outside of the VM lock by design.
       // For more information see https://github.com/NativeScript/ios-runtime/issues/215 and it's corresponding PR.
       // This creates a racing condition which might corrupt the internal state of the VM but
       // a fix for it is outside of this PR's scope, so I'm leaving it like it has always been.
-
       JSLockHolder lockHolder(vm);
+
       // we no longer have a valid csaller on the stack, what with being async and all
-      fakeExecState->setCallerFrame(CallFrame::noCaller());
+      fakeExecState->setCallerFrame(fakeExecState->lexicalGlobalObject()->globalExec());
 
       JSValue result;
+      JSValue jsexception = jsNull();
       {
           TopCallFrameSetter frameSetter(vm, fakeExecState);
           result = call->returnType().read(fakeExecState, invocation->_buffer + call->returnOffset(), call->returnTypeCell().get());
 
           call->postCall(fakeExecState, *invocation);
-      }
 
-      if (Exception* exception = scope.exception()) {
+          if (Exception* ex = scope.exception()) {
+              scope.clearException();
+              jsexception = ex->value();
+          } else if (nsexception != nullptr) {
+              jsexception = JSValue(createErrorFromNSException(runtime, fakeExecState, nsexception));
+          }
+      }
+      if (jsexception != jsNull()) {
           scope.clearException();
           CallData rejectCallData;
           CallType rejectCallType = JSC::getCallData(vm, deferred->reject(), rejectCallData);
 
           MarkedArgumentBuffer rejectArguments;
-          rejectArguments.append(exception->value());
+          rejectArguments.append(jsexception);
           JSC::call(fakeExecState->lexicalGlobalObject()->globalExec(), deferred->reject(), rejectCallType, rejectCallData, jsUndefined(), rejectArguments);
       } else {
           CallData resolveCallData;

tests/TestRunner/app/ApiTests.js

@@ -448,6 +448,29 @@ describe(module.id, function () {
                 done();
             });
         });
+
+        if (isSimulator) {
+            // Skip on simulator because libffi breaks exception unwinding on iOS Simulator
+            // see https://github.com/libffi/libffi/issues/418
+            console.warn("warning: Skipping async ObjC exceptions tests on Simulator device!");
+        } else {
+            it("should throw Objective-C exceptions to JavaScript", function (done) {
+                const value = 333;
+                const arr = NSArray.arrayWithObject(value);
+                var promise = arr.objectAtIndex.async(arr, [0])
+                .then(res => {
+                    expect(res).toBe(value);
+                    expect(NSThread.currentThread.isMainThread).toBe(true);
+                })
+                .then(() => arr.objectAtIndex.async(arr, [2]))
+                .catch(error => {
+                    expect(NSThread.currentThread.isMainThread).toBe(true);
+                    expect(error.toString()).toMatch("index 2 beyond bounds");
+                    expect(error.stack).toEqual("objectAtIndex@[native code]\n[native code]");
+                    done();
+                });
+             });
+        }
     });
 
     it("should distinguish between undefined and unavailable variables", function () {

tests/TestRunner/app/Infrastructure/simulator.js

@@ -0,0 +1,9 @@
+function isSimulator() {
+    if (NSProcessInfo.processInfo.isOperatingSystemAtLeastVersion({majorVersion: 9, minorVersion: 0, patchVersion: 0})) {
+        return NSProcessInfo.processInfo.environment.objectForKey("SIMULATOR_DEVICE_NAME") !== null;
+    } else {
+        return UIDevice.currentDevice.name.toLowerCase().indexOf("simulator") > -1;
+    }
+}
+
+global.isSimulator = isSimulator();

tests/TestRunner/app/MethodCallsTests.js

@@ -1085,6 +1085,37 @@ describe(module.id, function () {
         var actual = TNSGetOutput();
         expect(actual).toBe('static setDerivedCategoryProperty: calledstatic derivedCategoryProperty called');
     });
+
+    if (isSimulator) {
+        // Skip on simulator because libffi breaks exception unwinding on iOS Simulator
+        // see https://github.com/libffi/libffi/issues/418
+        console.warn("warning: Skipping ObjC exceptions tests on Simulator device!");
+    } else {
+        it('Throw_ObjC_exceptions_to_JavaScript', function () {
+            try {
+                NSArray.alloc().init().objectAtIndex(3);
+                expect(false).toBeTruthy("Should never be reached because objectAtIndex(3) should throw an Objective-C exception.");
+            } catch(e) {
+                expect(e.stack).toContain("app/MethodCallsTests.js")
+                expect(e.nativeException).toBeDefined();
+                expect(e.nativeException instanceof NSException).toBe(true);
+                expect(e.nativeException.toString()).toBe(e.nativeException.reason);
+                expect(e.nativeException.reason).toContain("index 3 beyond bounds");
+                expect(e.nativeException.name).toBe("NSRangeException");
+                expect(e.nativeException.callStackSymbols.count).toBeGreaterThan(5);
+
+                var nativeCallstack = "";
+                for (var i=0; i < e.nativeException.callStackSymbols.count; i++) {
+                    nativeCallstack += e.nativeException.callStackSymbols[i] + '\n';
+                }
+
+                expect(nativeCallstack).toContain("CoreFoundation");
+                expect(nativeCallstack).toContain("TestRunner");
+                expect(nativeCallstack).toContain("objc_exception_throw");
+                expect(nativeCallstack).toContain("UIApplicationMain");
+            }
+        });
+    }
 
      it('Override: More than one methods with same jsname', function () {
 

tests/TestRunner/app/index.js

@@ -2,6 +2,7 @@
 console.log('Application Start!');
 
 import "./Infrastructure/timers";
+import "./Infrastructure/simulator";
 
 global.UNUSED = function (param) {
 };