From 3fe7b880aed9f9b02712e36ddfe9be53a212bbe4 Mon Sep 17 00:00:00 2001 From: phosit Date: Sat, 13 Jun 2026 16:30:08 +0200 Subject: [PATCH] Don't crash on compile error of hotloads Hotload functions aren't expected to throw an error. Instead return the error to the function who is waiting for the module. --- source/scriptinterface/ModuleLoader.cpp | 26 +++++++++----- source/scriptinterface/tests/test_Module.h | 40 +++++++++++++--------- 2 files changed, 40 insertions(+), 26 deletions(-) diff --git a/source/scriptinterface/ModuleLoader.cpp b/source/scriptinterface/ModuleLoader.cpp index 9e93ec9911..f9baeba370 100644 --- a/source/scriptinterface/ModuleLoader.cpp +++ b/source/scriptinterface/ModuleLoader.cpp @@ -1,4 +1,4 @@ -/* Copyright (C) 2025 Wildfire Games. +/* Copyright (C) 2026 Wildfire Games. * This file is part of 0 A.D. * * 0 A.D. is free software: you can redistribute it and/or modify @@ -339,16 +339,24 @@ ModuleLoader::Future::Future(const ScriptRequest& rq, ModuleLoader& loader, Resu // - Accessing values which are not yet exported results in an error. These errors might implicitly be // dropped. - JS::RootedObject mod{rq.cx, CompileModule(rq, loader.m_AllowModule, loader.m_Registry, modulePath, - result)}; - JS::RootedObject promise{rq.cx, Evaluate(rq, mod)}; - Evaluating& evaluatingStatus{std::get(m_Status)}; - evaluatingStatus.moduleNamespace = JS::GetModuleNamespace(rq.cx, mod); + try + { + JS::RootedObject mod{rq.cx, CompileModule(rq, loader.m_AllowModule, loader.m_Registry, modulePath, + result)}; + JS::RootedObject promise{rq.cx, Evaluate(rq, mod)}; + Evaluating& evaluatingStatus{std::get(m_Status)}; + evaluatingStatus.moduleNamespace = JS::GetModuleNamespace(rq.cx, mod); - SetReservedSlot(JS::PrivateValue(static_cast(&m_Status))); + SetReservedSlot(JS::PrivateValue(static_cast(&m_Status))); - if (!JS::AddPromiseReactions(rq.cx, promise, evaluatingStatus.fulfill, evaluatingStatus.reject)) - throw std::runtime_error{"Failed adding promise reaction."}; + if (!JS::AddPromiseReactions(rq.cx, promise, evaluatingStatus.fulfill, evaluatingStatus.reject)) + throw std::runtime_error{"Failed adding promise reaction."}; + } + catch(...) + { + if (std::holds_alternative(m_Status)) + m_Status = Rejected{std::current_exception()}; + } } ModuleLoader::Future::Future(Future&& other) noexcept: diff --git a/source/scriptinterface/tests/test_Module.h b/source/scriptinterface/tests/test_Module.h index e3b0b1b3fb..4421bde058 100644 --- a/source/scriptinterface/tests/test_Module.h +++ b/source/scriptinterface/tests/test_Module.h @@ -1,4 +1,4 @@ -/* Copyright (C) 2025 Wildfire Games. +/* Copyright (C) 2026 Wildfire Games. * This file is part of 0 A.D. * * 0 A.D. is free software: you can redistribute it and/or modify @@ -172,8 +172,9 @@ public: const ScriptRequest rq{script}; TestLogger logger; - TS_ASSERT_THROWS(std::ignore = script.GetModuleLoader().LoadModule(rq, - "import_inside_function.js"), const std::invalid_argument&); + auto result = script.GetModuleLoader().LoadModule(rq, "import_inside_function.js"); + auto& front = *result.begin(); + TS_ASSERT_THROWS(std::ignore = front.Get(), const std::invalid_argument&); const std::string log{logger.GetOutput()}; TS_ASSERT_STR_CONTAINS(log, "import_inside_function.js line 3"); TS_ASSERT_STR_CONTAINS(log, "import declarations may only appear at top level of a module"); @@ -185,8 +186,9 @@ public: const ScriptRequest rq{script}; const TestLogger _; - TS_ASSERT_THROWS(std::ignore = script.GetModuleLoader().LoadModule(rq, "nonexistent.js"), - const std::runtime_error&); + auto result= script.GetModuleLoader().LoadModule(rq, "nonexistent.js"); + auto& front = *result.begin(); + TS_ASSERT_THROWS(std::ignore = front.Get(), const std::runtime_error&); } void test_EvaluateOnce() @@ -407,10 +409,10 @@ public: const ScriptRequest rq{script}; TestLogger logger; - TS_ASSERT_THROWS(std::ignore = script.GetModuleLoader().LoadModule(rq, - "export_default/invalid.js"), const std::invalid_argument&); - const std::string log{logger.GetOutput()}; - TS_ASSERT_STR_CONTAINS(log, "export_default/invalid.js line 1"); + auto result = script.GetModuleLoader().LoadModule(rq, "export_default/invalid.js"); + auto& front = *result.begin(); + TS_ASSERT_THROWS(std::ignore = front.Get(), const std::invalid_argument&); + TS_ASSERT_STR_CONTAINS(logger.GetOutput(), "export_default/invalid.js line 1"); } void test_ExportDefaultDoesNotWorkAround() @@ -720,8 +722,9 @@ public: ScriptInterface script{"Test", "Test", g_ScriptContext}; const ScriptRequest rq{script}; - TS_ASSERT_THROWS_EQUALS(std::ignore = script.GetModuleLoader().LoadModule(rq, - "empty.js"), const std::runtime_error& e, e.what(), + auto result = script.GetModuleLoader().LoadModule(rq, "empty.js"); + auto& front = *result.begin(); + TS_ASSERT_THROWS_EQUALS(std::ignore = front.Get(), const std::runtime_error& e, e.what(), "Importing file \"empty.js\" is disallowed."); } @@ -730,8 +733,9 @@ public: ScriptInterface script{"Test", "Test", g_ScriptContext, DisallowedfilePredicate}; const ScriptRequest rq{script}; - TS_ASSERT_THROWS_EQUALS(std::ignore = script.GetModuleLoader().LoadModule(rq, - "restriction/disallowedfile.js"), const std::runtime_error& e, e.what(), + auto result = script.GetModuleLoader().LoadModule(rq, "restriction/disallowedfile.js"); + auto& front = *result.begin(); + TS_ASSERT_THROWS_EQUALS(std::ignore = front.Get(), const std::runtime_error& e, e.what(), "Importing file \"restriction/disallowedfile.js\" is disallowed."); } @@ -740,8 +744,9 @@ public: ScriptInterface script{"Test", "Test", g_ScriptContext, DisallowedfilePredicate}; const ScriptRequest rq{script}; TestLogger logger; - TS_ASSERT_THROWS_EQUALS(std::ignore = script.GetModuleLoader().LoadModule(rq, - "restriction/import.js"), const std::invalid_argument& e, e.what(), + auto result = script.GetModuleLoader().LoadModule(rq, "restriction/import.js"); + auto& front = *result.begin(); + TS_ASSERT_THROWS_EQUALS(std::ignore = front.Get(), const std::invalid_argument& e, e.what(), "Unable to link module."); TS_ASSERT_STR_CONTAINS(logger.GetOutput(), "Importing file \"restriction/disallowedfile.js\" is disallowed."); @@ -752,8 +757,9 @@ public: ScriptInterface script{"Test", "Test", g_ScriptContext, DisallowedfilePredicate}; const ScriptRequest rq{script}; TestLogger logger; - TS_ASSERT_THROWS_EQUALS(std::ignore = script.GetModuleLoader().LoadModule(rq, - "restriction/fancy_import.js"), const std::invalid_argument& e, e.what(), + auto result = script.GetModuleLoader().LoadModule(rq, "restriction/fancy_import.js"); + auto& front = *result.begin(); + TS_ASSERT_THROWS_EQUALS(std::ignore = front.Get(), const std::invalid_argument& e, e.what(), "Unable to link module."); TS_ASSERT_STR_CONTAINS(logger.GetOutput(), "Importing file \"restriction/disallowedfile.js\" is disallowed.");