Remove a not neede try/catch when evaluating JSFunction values so we can improve error reporting by not hiding the information we get form the JS crash

pedrovgs · pedrovgs · commit ff490e9532eb · 2022-03-30T17:10:52.000+02:00
diff --git a/IntegrationTests/TestSuites/Sources/PrimaryTests/UnitTestUtils.swift b/IntegrationTests/TestSuites/Sources/PrimaryTests/UnitTestUtils.swift
@@ -103,6 +103,12 @@ func expectThrow<T>(_ body: @autoclosure () throws -> T, file: StaticString = #f
     throw MessageError("Expect to throw an exception", file: file, line: line, column: column)
 }
 
+func wrapUnsafeThrowableFunction(_ body: @escaping () -> Void, file: StaticString = #file, line: UInt = #line, column: UInt = #column) throws -> Error {
+    JSObject.global.callThrowingClosure.function!(JSClosure { _ in 
+            body() 
+            return .undefined
+    })
+}
 func expectNotNil<T>(_ value: T?, file: StaticString = #file, line: UInt = #line, column: UInt = #column) throws {
     switch value {
     case .some: return
diff --git a/IntegrationTests/TestSuites/Sources/PrimaryTests/main.swift b/IntegrationTests/TestSuites/Sources/PrimaryTests/main.swift
@@ -705,27 +705,21 @@ try test("Exception") {
     let prop_9: JSValue = globalObject1.prop_9
 
     // MARK: Throwing method calls
-    let error1 = try expectThrow(try prop_9.object!.throwing.func1!())
+    let error1 = try wrapUnsafeThrowableFunction { _ = prop_9.object!.func1!() }
     try expectEqual(error1 is JSValue, true)
     let errorObject = JSError(from: error1 as! JSValue)
     try expectNotNil(errorObject)
 
-    let error2 = try expectThrow(try prop_9.object!.throwing.func2!())
+    let error2 = try wrapUnsafeThrowableFunction { _ = prop_9.object!.func2!() }
     try expectEqual(error2 is JSValue, true)
     let errorString = try expectString(error2 as! JSValue)
     try expectEqual(errorString, "String Error")
 
-    let error3 = try expectThrow(try prop_9.object!.throwing.func3!())
+    let error3 = try wrapUnsafeThrowableFunction { _ = prop_9.object!.func3!() }
     try expectEqual(error3 is JSValue, true)
     let errorNumber = try expectNumber(error3 as! JSValue)
     try expectEqual(errorNumber, 3.0)
 
-    // MARK: Simple function calls
-    let error4 = try expectThrow(try prop_9.func1.function!.throws())
-    try expectEqual(error4 is JSValue, true)
-    let errorObject2 = JSError(from: error4 as! JSValue)
-    try expectNotNil(errorObject2)
-
     // MARK: Throwing constructor call
     let Animal = JSObject.global.Animal.function!
     _ = try Animal.throws.new("Tama", 3, true)
diff --git a/IntegrationTests/bin/primary-tests.js b/IntegrationTests/bin/primary-tests.js
@@ -82,6 +82,14 @@ global.Animal = function (name, age, isCat) {
     }
 };
 
+global.callThrowingClosure = (c) => { 
+    try {
+        c() 
+    } catch (error) {
+        return error
+    }
+}
+
 const { startWasiTask } = require("../lib");
 
 startWasiTask("./dist/PrimaryTests.wasm").catch((err) => {
diff --git a/Runtime/src/index.ts b/Runtime/src/index.ts
@@ -226,6 +226,29 @@ export class SwiftRuntime {
                 this.memory
             );
         },
+        swjs_call_function_unsafe: (
+            ref: ref,
+            argv: pointer,
+            argc: number,
+            kind_ptr: pointer,
+            payload1_ptr: pointer,
+            payload2_ptr: pointer
+        ) => {
+            const func = this.memory.getObject(ref);
+            const result = Reflect.apply(
+                func,
+                undefined,
+                JSValue.decodeArray(argv, argc, this.memory)
+            );
+            JSValue.write(
+                result,
+                kind_ptr,
+                payload1_ptr,
+                payload2_ptr,
+                false,
+                this.memory
+            );
+        },
 
         swjs_call_function_with_this: (
             obj_ref: ref,
@@ -265,6 +288,32 @@ export class SwiftRuntime {
                 this.memory
             );
         },
+
+        swjs_call_function_with_this_unsafe: (
+            obj_ref: ref,
+            func_ref: ref,
+            argv: pointer,
+            argc: number,
+            kind_ptr: pointer,
+            payload1_ptr: pointer,
+            payload2_ptr: pointer
+        ) => {
+            const obj = this.memory.getObject(obj_ref);
+            const func = this.memory.getObject(func_ref);
+            const result = Reflect.apply(
+                func,
+                obj,
+                JSValue.decodeArray(argv, argc, this.memory)
+            );
+            JSValue.write(
+                result,
+                kind_ptr,
+                payload1_ptr,
+                payload2_ptr,
+                false,
+                this.memory
+            );
+        },
         swjs_call_new: (ref: ref, argv: pointer, argc: number) => {
             const constructor = this.memory.getObject(ref);
             const instance = Reflect.construct(
diff --git a/Runtime/src/types.ts b/Runtime/src/types.ts
@@ -58,6 +58,14 @@ export interface ImportedFunctions {
         payload1_ptr: pointer,
         payload2_ptr: pointer
     ): void;
+    swjs_call_function_unsafe(
+        ref: number,
+        argv: pointer,
+        argc: number,
+        kind_ptr: pointer,
+        payload1_ptr: pointer,
+        payload2_ptr: pointer
+    ): void;
     swjs_call_function_with_this(
         obj_ref: ref,
         func_ref: ref,
@@ -67,6 +75,15 @@ export interface ImportedFunctions {
         payload1_ptr: pointer,
         payload2_ptr: pointer
     ): void;
+    swjs_call_function_with_this_unsafe(
+        obj_ref: ref,
+        func_ref: ref,
+        argv: pointer,
+        argc: number,
+        kind_ptr: pointer,
+        payload1_ptr: pointer,
+        payload2_ptr: pointer
+    ): void;
     swjs_call_new(ref: number, argv: pointer, argc: number): number;
     swjs_call_throwing_new(
         ref: number,
diff --git a/Sources/JavaScriptKit/FundamentalObjects/JSFunction.swift b/Sources/JavaScriptKit/FundamentalObjects/JSFunction.swift
@@ -93,11 +93,11 @@ internal func invokeJSFunction(_ jsFunc: JSFunction, arguments: [ConvertibleToJS
             var payload1 = JavaScriptPayload1()
             var payload2 = JavaScriptPayload2()
             if let thisId = this?.id {
-                _call_function_with_this(thisId,
+                _call_function_with_this_unsafe(thisId,
                                          jsFunc.id, argv, Int32(argc),
                                          &kindAndFlags, &payload1, &payload2)
             } else {
-                _call_function(
+                _call_function_unsafe(
                     jsFunc.id, argv, Int32(argc),
                     &kindAndFlags, &payload1, &payload2
                 )
diff --git a/Sources/JavaScriptKit/XcodeSupport.swift b/Sources/JavaScriptKit/XcodeSupport.swift
@@ -53,6 +53,13 @@ import _CJavaScriptKit
         _: UnsafeMutablePointer<JavaScriptPayload1>!,
         _: UnsafeMutablePointer<JavaScriptPayload2>!
     ) { fatalError() }
+    func _call_function_unsafe(
+        _: JavaScriptObjectRef,
+        _: UnsafePointer<RawJSValue>!, _: Int32,
+        _: UnsafeMutablePointer<JavaScriptValueKindAndFlags>!,
+        _: UnsafeMutablePointer<JavaScriptPayload1>!,
+        _: UnsafeMutablePointer<JavaScriptPayload2>!
+    ) { fatalError() }
     func _call_function_with_this(
         _: JavaScriptObjectRef,
         _: JavaScriptObjectRef,
@@ -61,6 +68,14 @@ import _CJavaScriptKit
         _: UnsafeMutablePointer<JavaScriptPayload1>!,
         _: UnsafeMutablePointer<JavaScriptPayload2>!
     ) { fatalError() }
+    func _call_function_with_this_unsafe(
+        _: JavaScriptObjectRef,
+        _: JavaScriptObjectRef,
+        _: UnsafePointer<RawJSValue>!, _: Int32,
+        _: UnsafeMutablePointer<JavaScriptValueKindAndFlags>!,
+        _: UnsafeMutablePointer<JavaScriptPayload1>!,
+        _: UnsafeMutablePointer<JavaScriptPayload2>!
+    ) { fatalError() }
     func _call_new(
         _: JavaScriptObjectRef,
         _: UnsafePointer<RawJSValue>!, _: Int32
diff --git a/Sources/_CJavaScriptKit/include/_CJavaScriptKit.h b/Sources/_CJavaScriptKit/include/_CJavaScriptKit.h
@@ -170,6 +170,21 @@ extern void _call_function(const JavaScriptObjectRef ref, const RawJSValue *argv
                            JavaScriptPayload1 *result_payload1,
                            JavaScriptPayload2 *result_payload2);
 
+/// `_call_function` calls JavaScript function with given arguments list without capturing any exception
+///
+/// @param ref The target JavaScript function to call.
+/// @param argv A list of `RawJSValue` arguments to apply.
+/// @param argc The length of `argv``.
+/// @param result_kind A result pointer of JavaScript value kind of returned result or thrown exception.
+/// @param result_payload1 A result pointer of first payload of JavaScript value of returned result or thrown exception.
+/// @param result_payload2 A result pointer of second payload of JavaScript value of returned result or thrown exception.
+__attribute__((__import_module__("javascript_kit"),
+               __import_name__("swjs_call_function_unsafe")))
+extern void _call_function_unsafe(const JavaScriptObjectRef ref, const RawJSValue *argv,
+                           const int argc, JavaScriptValueKindAndFlags *result_kind,
+                           JavaScriptPayload1 *result_payload1,
+                           JavaScriptPayload2 *result_payload2);
+
 /// `_call_function_with_this` calls JavaScript function with given arguments list and given `_this`.
 ///
 /// @param _this The value of `this` provided for the call to `func_ref`.
@@ -188,6 +203,24 @@ extern void _call_function_with_this(const JavaScriptObjectRef _this,
                                      JavaScriptPayload1 *result_payload1,
                                      JavaScriptPayload2 *result_payload2);
 
+/// `_call_function_with_this` calls JavaScript function with given arguments list and given `_this` without capturing any exception.
+///
+/// @param _this The value of `this` provided for the call to `func_ref`.
+/// @param func_ref The target JavaScript function to call.
+/// @param argv A list of `RawJSValue` arguments to apply.
+/// @param argc The length of `argv``.
+/// @param result_kind A result pointer of JavaScript value kind of returned result or thrown exception.
+/// @param result_payload1 A result pointer of first payload of JavaScript value of returned result or thrown exception.
+/// @param result_payload2 A result pointer of second payload of JavaScript value of returned result or thrown exception.
+__attribute__((__import_module__("javascript_kit"),
+               __import_name__("swjs_call_function_with_this_unsafe")))
+extern void _call_function_with_this_unsafe(const JavaScriptObjectRef _this,
+                                     const JavaScriptObjectRef func_ref,
+                                     const RawJSValue *argv, const int argc,
+                                     JavaScriptValueKindAndFlags *result_kind,
+                                     JavaScriptPayload1 *result_payload1,
+                                     JavaScriptPayload2 *result_payload2);
+
 /// `_call_new` calls JavaScript object constructor with given arguments list.
 ///
 /// @param ref The target JavaScript constructor to call.
