From 616fa4a0068af5f597d637301e7414c733eee024 Mon Sep 17 00:00:00 2001
From: phosit <phosit@autistici.org>
Date: Wed, 18 Jun 2025 07:22:10 +0200
Subject: [PATCH] Handle C++ exceptions in Engine functions

`JSNatives` passed to SpiderMonkey must not throw exceptions. Most
callbacks are wrapped in `ScriptFunction::ToJSNative`.
This commit adds exception handling to `ScriptFunction::ToJSNative` so
that exceptions thrown in the wrapped callbacks are catched and rethrown
as JavaScript `Error`s.
---
 .../_test.scriptinterface/exception/catch.js  |  8 +++++
 source/scriptinterface/FunctionWrapper.h      | 32 +++++++++++--------
 .../tests/test_FunctionWrapper.h              | 23 +++++++++++++
 3 files changed, 50 insertions(+), 13 deletions(-)
 create mode 100644 binaries/data/mods/_test.scriptinterface/exception/catch.js

diff --git a/binaries/data/mods/_test.scriptinterface/exception/catch.js b/binaries/data/mods/_test.scriptinterface/exception/catch.js
new file mode 100644
index 0000000000..ef716ea765
--- /dev/null
+++ b/binaries/data/mods/_test.scriptinterface/exception/catch.js
@@ -0,0 +1,8 @@
+try
+{
+	Engine.callback();
+}
+catch (e)
+{
+	log(e.message);
+}
diff --git a/source/scriptinterface/FunctionWrapper.h b/source/scriptinterface/FunctionWrapper.h
index b79fdf736c..7fc82ce9ac 100644
--- a/source/scriptinterface/FunctionWrapper.h
+++ b/source/scriptinterface/FunctionWrapper.h
@@ -324,20 +324,26 @@ public:
 		if (!wentOk)
 			return false;
 
-		/**
-		 * TODO: error handling isn't standard, and since this can call any C++ function,
-		 * there's no simple obvious way to deal with it.
-		 * For now we check for pending JS exceptions, but it would probably be nicer
-		 * to standardise on something, or perhaps provide an "errorHandler" here.
-		 */
-		if constexpr (std::is_same_v<void, typename args_info<decltype(callable)>::return_type>)
-			call<callable>(obj, outs);
-		else if constexpr (std::is_same_v<JS::Value, typename args_info<decltype(callable)>::return_type>)
-			args.rval().set(call<callable>(obj, outs));
-		else
-			Script::ToJSVal(rq, args.rval(), call<callable>(obj, outs));
+		try
+		{
+			if constexpr (std::is_same_v<void, typename args_info<decltype(callable)>::return_type>)
+				call<callable>(obj, outs);
+			else if constexpr (std::is_same_v<JS::Value, typename args_info<decltype(callable)>::return_type>)
+				args.rval().set(call<callable>(obj, outs));
+			else
+				Script::ToJSVal(rq, args.rval(), call<callable>(obj, outs));
 
-		return !ScriptException::IsPending(rq);
+			return !ScriptException::IsPending(rq);
+		}
+		catch (const std::exception& e)
+		{
+			ScriptException::Raise(rq, "%s", e.what());
+		}
+		catch (...)
+		{
+			ScriptException::Raise(rq, "Unknown error occured in an Engine callback.");
+		}
+		return false;
 	}
 
 	/**
diff --git a/source/scriptinterface/tests/test_FunctionWrapper.h b/source/scriptinterface/tests/test_FunctionWrapper.h
index 659098fb6d..a54fe49932 100644
--- a/source/scriptinterface/tests/test_FunctionWrapper.h
+++ b/source/scriptinterface/tests/test_FunctionWrapper.h
@@ -18,6 +18,7 @@
 #include "lib/self_test.h"
 
 #include "scriptinterface/FunctionWrapper.h"
+#include "scriptinterface/ModuleLoader.h"
 #include "scriptinterface/ScriptContext.h"
 #include "scriptinterface/ScriptInterface.h"
 
@@ -130,4 +131,26 @@ public:
 
 		TS_ASSERT(!ScriptFunction::CallVoid(rq, nativeScope, name));
 	}
+
+	void test_exception()
+	{
+		g_VFS = CreateVfs();
+		TS_ASSERT_OK(g_VFS->Mount(L"", DataDir() / "mods" / "_test.scriptinterface" / "exception" / "",
+			VFS_MOUNT_MUST_EXIST));
+
+		ScriptInterface script{"Engine", "Test", g_ScriptContext, [](const VfsPath&){
+			return true;
+		}};
+		const ScriptRequest rq{script};
+
+		auto _ = ScriptFunction::Register(rq, "callback", [&](){
+			throw std::runtime_error{"Testerror"};
+		});
+
+		TestLogger logger;
+		std::ignore = script.GetModuleLoader().LoadModule(rq, "catch.js");
+		TS_ASSERT_STR_CONTAINS(logger.GetOutput(), "Testerror");
+
+		g_VFS.reset();
+	}
 };