diff --git a/include/limestone/api/limestone_exception.h b/include/limestone/api/limestone_exception.h index 38d8bfd7..8839c503 100644 --- a/include/limestone/api/limestone_exception.h +++ b/include/limestone/api/limestone_exception.h @@ -43,17 +43,16 @@ class limestone_io_exception : public limestone_exception { public: // Constructor that takes an error message and errno as arguments (int) explicit limestone_io_exception(const std::string& message, int error_code) - : limestone_exception(format_message(message, error_code), error_code) {} + : limestone_exception(message, error_code) {} // Constructor that takes an error message and boost::system::error_code as arguments explicit limestone_io_exception(const std::string& message, const boost::system::error_code& error_code) - : limestone_exception(format_message(message, error_code), error_code.value()) {} + : limestone_exception(message, error_code.value()) {} // Helper function to format the error message for int error_code static std::string format_message(const std::string& message, int error_code) { // Retrieve the system error message corresponding to errno std::string errno_str = std::strerror(error_code); - // Format the complete error message return "I/O Error (" + errno_str + "): " + message + " (errno = " + std::to_string(error_code) + ")"; } diff --git a/src/limestone/datastore.cpp b/src/limestone/datastore.cpp index 97f20343..14d17819 100644 --- a/src/limestone/datastore.cpp +++ b/src/limestone/datastore.cpp @@ -385,7 +385,12 @@ void datastore::rotate_epoch_file() { << "." << epoch_id_switched_.load(); std::string new_name = ss.str(); boost::filesystem::path new_file = location_ / new_name; - boost::filesystem::rename(epoch_file_path_, new_file); + boost::system::error_code ec; + boost::filesystem::rename(epoch_file_path_, new_file, ec); + if (ec) { + std::string err_msg = "Failed to rename epoch_file from " + epoch_file_path_.string() + " to " + new_file.string(); + LOG_AND_THROW_IO_EXCEPTION(err_msg, ec); + } add_file(new_file); // create new one diff --git a/src/limestone/limestone_exception_helper.h b/src/limestone/limestone_exception_helper.h index 11cf1b57..1c6f54e4 100644 --- a/src/limestone/limestone_exception_helper.h +++ b/src/limestone/limestone_exception_helper.h @@ -26,32 +26,39 @@ namespace limestone { using limestone::api::limestone_exception; -using limestone::api::limestone_io_exception; +using limestone::api::limestone_io_exception; + + +inline std::string extract_filename(const std::string& path) { + // Use boost::filesystem::path to handle file path and extract filename + boost::filesystem::path p(path); + return p.filename().string(); +} #define THROW_LIMESTONE_EXCEPTION(message) \ { \ - LOG(ERROR) << message; \ - throw limestone_exception(std::string(message) + " (at " + std::string(__FILE__) + ":" + std::to_string(__LINE__) + ")"); \ + LOG_LP(ERROR) << message; \ + throw limestone_exception(std::string(message) + " (at " + extract_filename(__FILE__) + ":" + std::to_string(__LINE__) + ")"); \ } #define THROW_LIMESTONE_IO_EXCEPTION(message, error_code) \ { \ std::string full_message = limestone_io_exception::format_message(message, error_code); \ - LOG(ERROR) << full_message; \ - throw limestone_io_exception(full_message + " (at " + std::string(__FILE__) + ":" + std::to_string(__LINE__) + ")", error_code); \ + LOG_LP(ERROR) << full_message; \ + throw limestone_io_exception(full_message + " (at " + extract_filename(__FILE__) + ":" + std::to_string(__LINE__) + ")", error_code); \ } #define LOG_AND_THROW_EXCEPTION(message) \ { \ - LOG(ERROR) << message; \ - throw limestone_exception(std::string(message) + " (at " + std::string(__FILE__) + ":" + std::to_string(__LINE__) + ")"); \ + LOG_LP(ERROR) << message; \ + throw limestone_exception(std::string(message) + " (at " + extract_filename(__FILE__) + ":" + std::to_string(__LINE__) + ")"); \ } #define LOG_AND_THROW_IO_EXCEPTION(message, error_code) \ { \ std::string full_message = limestone_io_exception::format_message(message, error_code); \ - LOG(ERROR) << full_message; \ - throw limestone_io_exception(full_message + " (at " + std::string(__FILE__) + ":" + std::to_string(__LINE__) + ")", error_code); \ + LOG_LP(ERROR) << full_message; \ + throw limestone_io_exception(full_message + " (at " + extract_filename(__FILE__) + ":" + std::to_string(__LINE__) + ")", error_code); \ } diff --git a/src/limestone/rotation_task.cpp b/src/limestone/rotation_task.cpp index cfa1ba68..1ccdfab9 100644 --- a/src/limestone/rotation_task.cpp +++ b/src/limestone/rotation_task.cpp @@ -61,6 +61,7 @@ rotation_task::rotation_task(datastore& envelope) void rotation_task::rotate() { + try { rotation_result final_result; for (const auto& lc : envelope_.log_channels_) { boost::system::error_code error; @@ -84,6 +85,11 @@ void rotation_task::rotate() { final_result.set_rotation_end_files(envelope_.get_files()); result_promise_.set_value(final_result); + } catch (const std::exception& e) { + auto ex_ptr = std::current_exception(); + std::cerr << "Setting exception in promise" << std::endl; + result_promise_.set_exception(ex_ptr); + } } rotation_result rotation_task::wait_for_result() { diff --git a/test/limestone/compaction/rotation_task_test.cpp b/test/limestone/compaction/rotation_task_test.cpp index a3d2f624..6fa3b01c 100644 --- a/test/limestone/compaction/rotation_task_test.cpp +++ b/test/limestone/compaction/rotation_task_test.cpp @@ -19,6 +19,7 @@ #include #include "test_root.h" #include "rotation_task.h" +#include "limestone/api/limestone_exception.h" namespace limestone::testing { constexpr const char* data_location = "/tmp/rotation_task_test/data_location"; @@ -30,6 +31,7 @@ using limestone::api::log_channel; using limestone::api::rotation_task; using limestone::api::rotation_result; using limestone::api::rotation_task_helper; +using limestone::api::limestone_exception; class rotation_task_test : public ::testing::Test { protected: @@ -139,4 +141,35 @@ TEST_F(rotation_task_test, no_task_execution_when_queue_is_empty) { SUCCEED(); } + +TEST_F(rotation_task_test, task_throws_exception) { + auto task = rotation_task_helper::create_and_enqueue_task(*datastore_); + + // Force an exception to be thrown by removing the directory + if (system("rm -rf /tmp/rotation_task_test") != 0) { + std::cerr << "Cannot remove directory" << std::endl; + } + + // Since the exception is caught in task->rotate(), no exception should be thrown here + task->rotate(); + + // Check that an exception is thrown and verify its details + try { + rotation_result result = task->wait_for_result(); + FAIL() << "Expected limestone_exception to be thrown"; // Fails the test if no exception is thrown + } catch (const limestone_exception& e) { + // Verify the exception details + std::cerr << "Caught exception: " << e.what() << std::endl; + EXPECT_TRUE(std::string(e.what()).rfind("I/O Error (No such file or directory): Failed to rename epoch_file from /tmp/rotation_task_test/data_location/epoch", 0) == 0); + EXPECT_EQ(e.error_code(), ENOENT); + } catch (const std::exception& e) { + // Handle non-limestone_exception std::exception types + std::cerr << "Caught exception: " << e.what() << std::endl; + FAIL() << "Expected limestone_exception but caught a different std: " << e.what(); + } catch (...) { + // Handle unknown exception types + FAIL() << "Expected limestone_exception but caught an unknown exception type."; + } +} + } // namespace limestone::testing diff --git a/test/limestone/log/rotate_test.cpp b/test/limestone/log/rotate_test.cpp index 0545d5f6..c4ce1ebc 100644 --- a/test/limestone/log/rotate_test.cpp +++ b/test/limestone/log/rotate_test.cpp @@ -11,6 +11,10 @@ #include "internal.h" #include "test_root.h" +#include "limestone/api/limestone_exception.h" + +using limestone::api::limestone_exception; +#include "rotation_task.h" #define LOGFORMAT_VER 2 @@ -59,13 +63,24 @@ class rotate_test : public ::testing::Test { // Repeatally call switch_epoch until backup is completed in another thread std::thread switch_epoch_thread([&]() { while (!backup_completed.load()) { - datastore_->switch_epoch(epoch_value++); + datastore_->switch_epoch(epoch_value++); std::this_thread::sleep_for(std::chrono::milliseconds(1)); } }); // Begin backup and wait for completion in main thread - std::unique_ptr bd = datastore_->begin_backup(type); + std::unique_ptr bd; + + try { + // Begin backup and wait for completion in main thread + bd = datastore_->begin_backup(type); + } catch (const std::exception& e) { + // Ensure the thread is joined even in case of an exception + std::cerr << "Exception during backup: " << e.what() << std::endl; + backup_completed.store(true); + switch_epoch_thread.join(); + throw; + } // Set flag to notify backup completion backup_completed.store(true); @@ -80,6 +95,44 @@ class rotate_test : public ::testing::Test { extern void create_file(const boost::filesystem::path& path, std::string_view content); extern std::string data_manifest(int persistent_format_version = 1); + +TEST_F(rotate_test, rotate_fails_with_io_error) { + using namespace limestone::api; + + log_channel& channel = datastore_->create_channel(boost::filesystem::path(location)); + log_channel& unused_channel = datastore_->create_channel(boost::filesystem::path(location)); + datastore_->switch_epoch(42); + channel.begin_session(); + channel.add_entry(42, "k1", "v1", {100, 4}); + channel.end_session(); + datastore_->switch_epoch(43); + + // Force an exception to be thrown by removing the directory + if (system("rm -rf /tmp/rotate_test") != 0) { + std::cerr << "Cannot remove directory" << std::endl; + } + + // Check that an exception is thrown and verify its details + try { + run_backup_with_epoch_switch(backup_type::standard, 44); + FAIL() << "Expected limestone_exception to be thrown"; // Fails the test if no exception is thrown + } catch (const limestone_exception& e) { + // Verify the exception details + std::cerr << "Caught exception: " << e.what() << std::endl; + EXPECT_TRUE(std::string(e.what()).rfind("I/O Error (No such file or directory): Failed to rename epoch_file from /tmp", 0) == 0); + EXPECT_EQ(e.error_code(), ENOENT); + } catch (const std::exception& e) { + // Handle non-limestone_exception std::exception types + std::cerr << "Caught exception: " << e.what() << std::endl; + FAIL() << "Expected limestone_exception but caught a different std: " << e.what(); + } catch (...) { + // Handle unknown exception types + FAIL() << "Expected limestone_exception but caught an unknown exception type."; + } + +} + + TEST_F(rotate_test, log_is_rotated) { // NOLINT using namespace limestone::api;