From 6c6230ba320ab8b0d38e6ad4fb3dc304c8f0cc4a Mon Sep 17 00:00:00 2001
From: Ralph Wessel <ralph@speckle.systems>
Date: Mon, 19 Aug 2024 22:35:30 +0100
Subject: [PATCH] Added try/catch in JSPortal for installing/executing JS
 functions Docs

---
 .../Browser/Bridge/Account/GetAccounts.cpp    |  3 +-
 .../Browser/Bridge/Account/GetAccounts.h      |  8 ++--
 .../Speckle/Interface/Browser/JSPortal.h      | 46 ++++++++++++-------
 3 files changed, 33 insertions(+), 24 deletions(-)

diff --git a/SpeckleConnector/Connector/Interface/Browser/Bridge/Account/GetAccounts.cpp b/SpeckleConnector/Connector/Interface/Browser/Bridge/Account/GetAccounts.cpp
index b460e35..d3a17c0 100644
--- a/SpeckleConnector/Connector/Interface/Browser/Bridge/Account/GetAccounts.cpp
+++ b/SpeckleConnector/Connector/Interface/Browser/Bridge/Account/GetAccounts.cpp
@@ -19,8 +19,7 @@ namespace {
  
 	bridge: The parent bridge object (provides access to bridge methods)
   --------------------------------------------------------------------*/
-GetAccounts::GetAccounts() : JSBridgeMethod{"GetAccounts", [&]() { return getAccounts(); }} {
-} //GetAccounts::GetAccounts
+GetAccounts::GetAccounts() : JSBridgeMethod{"GetAccounts", [&]() { return getAccounts(); }} {}
 
 
 /*--------------------------------------------------------------------
diff --git a/SpeckleConnector/Connector/Interface/Browser/Bridge/Account/GetAccounts.h b/SpeckleConnector/Connector/Interface/Browser/Bridge/Account/GetAccounts.h
index a7e095d..30df4eb 100644
--- a/SpeckleConnector/Connector/Interface/Browser/Bridge/Account/GetAccounts.h
+++ b/SpeckleConnector/Connector/Interface/Browser/Bridge/Account/GetAccounts.h
@@ -21,17 +21,15 @@ namespace connector::interfac::browser::bridge {
 		 @param bridge The parent bridge object (provides access to bridge methods)
 		 */
 		GetAccounts();
-		/*!
-		 Cppy constructor
-		 @param bridge The parent bridge object (provides access to bridge methods)
-		 */
-		GetAccounts(const GetAccounts& source) = default;
 		
 		/*!
 		 Object cloning
 		 @return A clone of this object
 		 */
 		GetAccounts* clonePtr() const override { return new GetAccounts{*this}; }
+
+		// MARK: - Functions (const)
+		
 		/*!
 		 Get an argument instance for the function (used to deserialise/unpack incoming arguments)
 		 @return An argument instance
diff --git a/SpeckleLib/Speckle/Interface/Browser/JSPortal.h b/SpeckleLib/Speckle/Interface/Browser/JSPortal.h
index ecb2ffe..e2506af 100644
--- a/SpeckleLib/Speckle/Interface/Browser/JSPortal.h
+++ b/SpeckleLib/Speckle/Interface/Browser/JSPortal.h
@@ -68,11 +68,14 @@ namespace speckle::interfac::browser {
 	template<typename FunctionBinding>
 	bool JSPortal<FunctionBinding>::execute(const speckle::utility::String& code) const {
 #ifdef ARCHICAD
-		auto engine = getJSEngine();
-		return engine ? engine->ExecuteJS(code) : false;
-#else
-		return false;	//Implement as required
+		try {
+			auto engine = getJSEngine();
+			return engine ? engine->ExecuteJS(code) : false;
+		} catch(...) {
+			///TODO: Need to discuss the best course of action to notify of a failure
+		}
 #endif
+		return false;
 	} //JSPortal<FunctionBinding>::execute
 	
 	
@@ -86,19 +89,28 @@ namespace speckle::interfac::browser {
 	template<typename FunctionBinding>
 	bool JSPortal<FunctionBinding>::install(std::shared_ptr<JSObject<FunctionBinding>> object) {
 #ifdef ARCHICAD
-		auto engine = getJSEngine();
-		if (!engine)
-			return false;
-		JS::Object* acObject = new JS::Object(object->getName());
-		for (auto& function : *object) {
-			acObject->AddItem(new JS::Function(function->getName(), [&] (GS::Ref<JS::Base> args) {
-				return function->execute(args);
-			}));
-		}
-		if (engine->RegisterAsynchJSObject(acObject)) {
-			base::push_back(object);
-			object->setPortal(*this);
-			return true;
+		try {
+			auto engine = getJSEngine();
+			if (!engine)
+				return false;
+			JS::Object* acObject = new JS::Object(object->getName());
+			for (auto& function : *object) {
+				acObject->AddItem(new JS::Function(function->getName(), [&] (GS::Ref<JS::Base> args) {
+					try {
+						return function->execute(args);
+					} catch(...) {
+						///TODO: Need to discuss the best course of action to notify of a failure
+					}
+					return GS::Ref<JS::Base>{};
+				}));
+			}
+			if (engine->RegisterAsynchJSObject(acObject)) {
+				base::push_back(object);
+				object->setPortal(*this);
+				return true;
+			}
+		} catch(...) {
+			///TODO: Need to discuss the best course of action to notify of a failure
 		}
 #endif
 		return false;