From e90dffd518521f2aecfebd96b37d53c23aabb141 Mon Sep 17 00:00:00 2001 From: Oliver Hamlet Date: Thu, 15 May 2025 20:27:52 +0100 Subject: [PATCH] Replace how esplugin errors are thrown Throw std::runtime_error for all esplugin errors except ESP_ERROR_PLUGIN_METADATA_NOT_FOUND, which causes a PluginNotLoadedError to be thrown, since LOOT needs to be able to distinguish that error. --- CMakeLists.txt | 3 +- docs/api/reference.rst | 11 ++--- include/loot/api.h | 2 +- ...categories.h => plugin_not_loaded_error.h} | 19 ++++---- src/api/exception/error_categories.cpp | 46 ------------------- src/api/plugin.cpp | 22 ++++----- src/tests/api/interface/game_interface_test.h | 18 ++------ src/tests/api/internals/plugin_test.h | 19 +++----- 8 files changed, 36 insertions(+), 104 deletions(-) rename include/loot/exception/{error_categories.h => plugin_not_loaded_error.h} (67%) delete mode 100644 src/api/exception/error_categories.cpp diff --git a/CMakeLists.txt b/CMakeLists.txt index 76797bf8..21e7f793 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -200,7 +200,6 @@ set(LIBLOOT_SRC_API_CPP_FILES "${CMAKE_SOURCE_DIR}/src/api/api_database.cpp" "${CMAKE_SOURCE_DIR}/src/api/bsa.cpp" "${CMAKE_SOURCE_DIR}/src/api/exception/cyclic_interaction_error.cpp" - "${CMAKE_SOURCE_DIR}/src/api/exception/error_categories.cpp" "${CMAKE_SOURCE_DIR}/src/api/exception/undefined_group_error.cpp" "${CMAKE_SOURCE_DIR}/src/api/metadata/condition_evaluator.cpp" "${CMAKE_SOURCE_DIR}/src/api/metadata/conditional_metadata.cpp" @@ -231,8 +230,8 @@ set(LIBLOOT_INCLUDE_H_FILES "${CMAKE_SOURCE_DIR}/include/loot/api.h" "${CMAKE_SOURCE_DIR}/include/loot/api_decorator.h" "${CMAKE_SOURCE_DIR}/include/loot/database_interface.h" - "${CMAKE_SOURCE_DIR}/include/loot/exception/error_categories.h" "${CMAKE_SOURCE_DIR}/include/loot/exception/cyclic_interaction_error.h" + "${CMAKE_SOURCE_DIR}/include/loot/exception/plugin_not_loaded_error.h" "${CMAKE_SOURCE_DIR}/include/loot/exception/undefined_group_error.h" "${CMAKE_SOURCE_DIR}/include/loot/enum/edge_type.h" "${CMAKE_SOURCE_DIR}/include/loot/enum/game_type.h" diff --git a/docs/api/reference.rst b/docs/api/reference.rst index fa110b1d..fbeef358 100644 --- a/docs/api/reference.rst +++ b/docs/api/reference.rst @@ -95,13 +95,8 @@ Exceptions .. doxygenclass:: loot::CyclicInteractionError :members: -.. doxygenclass:: loot::UndefinedGroupError +.. doxygenclass:: loot::PluginNotLoadedError :members: -Error Categories -================ - -LOOT uses error category objects to identify errors with codes that originate in -lower-level libraries. - -.. doxygenfunction:: loot::esplugin_category +.. doxygenclass:: loot::UndefinedGroupError + :members: diff --git a/include/loot/api.h b/include/loot/api.h index abdd10eb..a08320b1 100644 --- a/include/loot/api.h +++ b/include/loot/api.h @@ -35,7 +35,7 @@ #include "loot/enum/game_type.h" #include "loot/enum/log_level.h" #include "loot/exception/cyclic_interaction_error.h" -#include "loot/exception/error_categories.h" +#include "loot/exception/plugin_not_loaded_error.h" #include "loot/exception/undefined_group_error.h" #include "loot/game_interface.h" #include "loot/loot_version.h" diff --git a/include/loot/exception/error_categories.h b/include/loot/exception/plugin_not_loaded_error.h similarity index 67% rename from include/loot/exception/error_categories.h rename to include/loot/exception/plugin_not_loaded_error.h index d5c005c8..1743f2c2 100644 --- a/include/loot/exception/error_categories.h +++ b/include/loot/exception/plugin_not_loaded_error.h @@ -22,21 +22,20 @@ . */ -#ifndef LOOT_ERROR_CATEGORIES -#define LOOT_ERROR_CATEGORIES +#ifndef LOOT_EXCEPTION_PLUGIN_NOT_LOADED_ERROR +#define LOOT_EXCEPTION_PLUGIN_NOT_LOADED_ERROR -#include - -#include "loot/api_decorator.h" +#include namespace loot { /** - * @brief Get the error category that can be used to identify system_error - * exceptions that are due to esplugin errors. - * @returns A reference to the static object of unspecified runtime type, - * derived from std::error_category. + * @brief An exception class thrown if a plugin that must be loaded hasn't been + * loaded. */ -LOOT_API const std::error_category& esplugin_category(); +class PluginNotLoadedError : public std::runtime_error { +public: + using std::runtime_error::runtime_error; +}; } #endif diff --git a/src/api/exception/error_categories.cpp b/src/api/exception/error_categories.cpp deleted file mode 100644 index c6035f07..00000000 --- a/src/api/exception/error_categories.cpp +++ /dev/null @@ -1,46 +0,0 @@ -/* LOOT - - A load order optimisation tool for Oblivion, Skyrim, Fallout 3 and - Fallout: New Vegas. - - Copyright (C) 2012-2016 WrinklyNinja - - This file is part of LOOT. - - LOOT is free software: you can redistribute - it and/or modify it under the terms of the GNU General Public License - as published by the Free Software Foundation, either version 3 of - the License, or (at your option) any later version. - - LOOT is distributed in the hope that it will - be useful, but WITHOUT ANY WARRANTY; without even the implied warranty of - MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the - GNU General Public License for more details. - - You should have received a copy of the GNU General Public License - along with LOOT. If not, see - . - */ - -#include "loot/exception/error_categories.h" - -#include - -namespace loot { -namespace detail { -class esplugin_category : public std::error_category { - const char* name() const noexcept override { return "esplugin"; } - - std::string message(int) const override { return "esplugin error"; } - - bool equivalent(const std::error_code& code, int) const noexcept override { - return code.category().name() == name(); - } -}; -} - -LOOT_API const std::error_category& esplugin_category() { - static detail::esplugin_category instance; - return instance; -} -} diff --git a/src/api/plugin.cpp b/src/api/plugin.cpp index 48056dd2..2aae36b7 100644 --- a/src/api/plugin.cpp +++ b/src/api/plugin.cpp @@ -34,7 +34,7 @@ #include "api/helpers/crc.h" #include "api/helpers/logging.h" #include "api/helpers/text.h" -#include "loot/exception/error_categories.h" +#include "loot/exception/plugin_not_loaded_error.h" namespace { using loot::BSA_FILE_EXTENSION; @@ -170,14 +170,14 @@ void HandleEspluginError(unsigned int returnCode, std::string_view operation) { } auto err = fmt::format( - "esplugin failed to {}. Error code: {}", operation, returnCode); + "Failed to {}. esplugin error code: {}", operation, returnCode); - const char* e = nullptr; - esp_get_error_message(&e); - if (e == nullptr) { + const char* message = nullptr; + esp_get_error_message(&message); + if (message == nullptr) { err += ". Details could not be fetched."; } else { - err += ". Details: " + std::string(e); + err += ". Details: " + std::string(message); } auto logger = loot::getLogger(); @@ -185,7 +185,11 @@ void HandleEspluginError(unsigned int returnCode, std::string_view operation) { logger->error(err); } - throw std::system_error(returnCode, loot::esplugin_category(), err); + if (returnCode == ESP_ERROR_PLUGIN_METADATA_NOT_FOUND) { + throw loot::PluginNotLoadedError(err); + } + + throw std::runtime_error(err); } template @@ -268,10 +272,6 @@ Plugin::Plugin(const GameType gameType, tags_ = ExtractBashTags(description); version_ = ExtractVersion(description); } catch (const std::system_error& e) { - if (e.code().category() == esplugin_category()) { - throw; - } - if (logger) { logger->error("Cannot read plugin file \"{}\". Details: {}", pluginPath.u8string(), diff --git a/src/tests/api/interface/game_interface_test.h b/src/tests/api/interface/game_interface_test.h index 6e4abe91..0a1173d0 100644 --- a/src/tests/api/interface/game_interface_test.h +++ b/src/tests/api/interface/game_interface_test.h @@ -371,13 +371,8 @@ TEST_P( if (GetParam() == GameType::tes3 || GetParam() == GameType::openmw || GetParam() == GameType::starfield) { - try { - handle_->LoadPlugins({blankMasterDependentEsm}, false); - FAIL(); - } catch (const std::system_error& e) { - EXPECT_EQ(ESP_ERROR_PLUGIN_METADATA_NOT_FOUND, e.code().value()); - EXPECT_EQ(esplugin_category(), e.code().category()); - } + EXPECT_THROW(handle_->LoadPlugins({blankMasterDependentEsm}, false), + PluginNotLoadedError); } else { handle_->LoadPlugins({blankMasterDependentEsm}, false); @@ -390,13 +385,8 @@ TEST_P( loadPluginsShouldThrowIfAPluginHasAMasterThatIsNotInTheInputAndIsNotAlreadyLoadedAndGameIsMorrowindOrStarfield) { if (GetParam() == GameType::tes3 || GetParam() == GameType::openmw || GetParam() == GameType::starfield) { - try { - handle_->LoadPlugins({blankMasterDependentEsm}, false); - FAIL(); - } catch (const std::system_error& e) { - EXPECT_EQ(ESP_ERROR_PLUGIN_METADATA_NOT_FOUND, e.code().value()); - EXPECT_EQ(esplugin_category(), e.code().category()); - } + EXPECT_THROW(handle_->LoadPlugins({blankMasterDependentEsm}, false), + PluginNotLoadedError); } else { handle_->LoadPlugins({blankMasterDependentEsm}, false); diff --git a/src/tests/api/internals/plugin_test.h b/src/tests/api/internals/plugin_test.h index b9151cba..a248d66f 100644 --- a/src/tests/api/internals/plugin_test.h +++ b/src/tests/api/internals/plugin_test.h @@ -27,7 +27,7 @@ along with LOOT. If not, see #include "api/game/game.h" #include "api/plugin.h" -#include "loot/exception/error_categories.h" +#include "loot/exception/plugin_not_loaded_error.h" #include "tests/common_game_test_fixture.h" namespace loot { @@ -328,7 +328,7 @@ TEST_P(PluginTest, loadingWholePluginShouldSucceedForOpenMWPlugins) { } else { EXPECT_THROW( Plugin(game_.GetType(), game_.GetCache(), dataPath / omwscripts, false), - std::system_error); + std::runtime_error); } } @@ -354,16 +354,11 @@ TEST_P( } TEST_P(PluginTest, loadingAPluginThatDoesNotExistShouldThrow) { - try { - Plugin(game_.GetType(), - game_.GetCache(), - game_.DataPath() / "Blank\\.esp", - true); - FAIL(); - } catch (const std::system_error& e) { - EXPECT_EQ(ESP_ERROR_FILE_NOT_FOUND, e.code().value()); - EXPECT_EQ(esplugin_category(), e.code().category()); - } + EXPECT_THROW(Plugin(game_.GetType(), + game_.GetCache(), + game_.DataPath() / "Blank\\.esp", + true), + std::runtime_error); } TEST_P(PluginTest, isValidShouldReturnTrueForAValidPlugin) {