From 6dc0fd6f9a6229f30aa8aaf8a9097f8acda17628 Mon Sep 17 00:00:00 2001
From: Benjamin Winger <bmw@disroot.org>
Date: Mon, 15 Apr 2024 15:09:54 -0400
Subject: [PATCH] Catch IO exceptions when flushing WAL and log them

Unhandled exceptions in the Database destructor can't be easily handled in the
rust API, and even in the C++ API it may cause terminate to be called, which is not very
user-friendly and will bypass other error reporting mechanisms.
---
 src/common/file_system/local_file_system.cpp | 4 +++-
 src/storage/wal/wal.cpp                      | 8 +++++++-
 tools/rust_api/src/database.rs               | 5 ++---
 3 files changed, 12 insertions(+), 5 deletions(-)

diff --git a/src/common/file_system/local_file_system.cpp b/src/common/file_system/local_file_system.cpp
index 786fefbad6..c2a9521ebb 100644
--- a/src/common/file_system/local_file_system.cpp
+++ b/src/common/file_system/local_file_system.cpp
@@ -292,7 +292,9 @@ void LocalFileSystem::syncFile(const FileInfo& fileInfo) const {
 #if defined(_WIN32)
     // Note that `FlushFileBuffers` returns 0 when fail, while `fsync` returns 0 when succeed.
     if (FlushFileBuffers((HANDLE)localFileInfo.handle) == 0) {
-        throw IOException(stringFormat("Failed to sync file {}.", fileInfo.path));
+        auto error = GetLastError();
+        throw IOException(stringFormat("Failed to sync file {}. Error {}: {}", fileInfo.path, error,
+            std::system_category().message(error)));
     }
 #else
     if (fsync(localFileInfo.fd) != 0) {
diff --git a/src/storage/wal/wal.cpp b/src/storage/wal/wal.cpp
index 4b025836c4..a6a649ab53 100644
--- a/src/storage/wal/wal.cpp
+++ b/src/storage/wal/wal.cpp
@@ -2,11 +2,13 @@
 
 #include <fcntl.h>
 
+#include "common/exception/io.h"
 #include "common/file_system/file_info.h"
 #include "common/file_system/virtual_file_system.h"
 #include "common/serializer/buffered_file.h"
 #include "common/serializer/deserializer.h"
 #include "common/serializer/serializer.h"
+#include "spdlog/spdlog.h"
 #include "storage/storage_utils.h"
 
 using namespace kuzu::common;
@@ -31,7 +33,11 @@ WAL::WAL(const std::string& directory, bool readOnly, BufferManager& bufferManag
 }
 
 WAL::~WAL() {
-    flushAllPages();
+    try {
+        flushAllPages();
+    } catch (IOException& e) {
+        spdlog::error("Failed to flush WAL: {}", e.what());
+    }
 }
 
 page_idx_t WAL::logPageUpdateRecord(DBFileID dbFileID, page_idx_t pageIdxInOriginalFile) {
diff --git a/tools/rust_api/src/database.rs b/tools/rust_api/src/database.rs
index 86c3e83f72..918360e729 100644
--- a/tools/rust_api/src/database.rs
+++ b/tools/rust_api/src/database.rs
@@ -158,9 +158,8 @@ mod tests {
 
     #[test]
     fn create_database_failure() {
-        let result: Error = Database::new("", SystemConfig::default())
-            .expect_err("An empty string should not be a valid database path!")
-            .into();
+        Database::new("", SystemConfig::default())
+            .expect_err("An empty string should not be a valid database path!");
     }
 
     #[test]