Skip to content

Commit

Permalink
Create and use libdnf5::FileSystemError instead of std filesystem_error
Browse files Browse the repository at this point in the history
This allows catching the excetions by dnf5 and returning exit code
insteaad of a traceback.
  • Loading branch information
kontura committed Nov 28, 2023
1 parent 9f67c83 commit 0170c6c
Show file tree
Hide file tree
Showing 14 changed files with 87 additions and 47 deletions.
30 changes: 30 additions & 0 deletions include/libdnf5/common/exception.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ along with libdnf. If not, see <https://www.gnu.org/licenses/>.

#include <fmt/format.h>

#include <filesystem>
#include <functional>
#include <stdexcept>
#include <type_traits>
Expand Down Expand Up @@ -210,6 +211,35 @@ class SystemError : public Error {
bool has_user_message;
};

/// An exception class for file system errors represented by the `errno` error code and a path.
class FileSystemError : public Error {
public:
/// Constructs the error from the `errno` error code, filesystem path and a formatted message.
/// The formatted message is prepended to the generated system error message.
///
/// @param error_code The `errno` of the error.
/// @param filesystem::path The `path` to the file.
/// @param format The format string for the message.
/// @param args The format arguments.
template <AllowedErrorArgTypes... Args>
explicit FileSystemError(int error_code, std::filesystem::path path, BgettextMessage format, Args... args)
: Error(format, std::forward<Args>(args)...),
error_code(error_code),
path(std::move(path)) {}

const char * what() const noexcept override;

const char * get_domain_name() const noexcept override { return "libdnf5::utils::fs"; }
const char * get_name() const noexcept override { return "FileSystemError"; }

/// @return The error code (`errno`) of the error.
int get_error_code() const noexcept { return error_code; }

private:
int error_code;
std::filesystem::path path;
};


// TODO(lukash) This class is used throughout the code where more specific exceptions should be thrown.
// Kept as a reminder to replace all those with the correct exception classes.
Expand Down
3 changes: 2 additions & 1 deletion include/libdnf5/utils/fs/file.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -21,13 +21,14 @@ along with libdnf. If not, see <https://www.gnu.org/licenses/>.
#define LIBDNF5_UTILS_FS_FILE_HPP

#include <filesystem>
#include <utility>


namespace libdnf5::utils::fs {

/// A wrapper for a `FILE *` that handles opening and closing a file in RAII
/// fashion. Errors are handled by raising instances of
/// `std::filesystem::filesystem_error` so that there's a single exception type
/// `libdnf5::FileSystemError` so that there's a single exception type
/// being raised for all filesystem-related errors.
class File {
public:
Expand Down
2 changes: 1 addition & 1 deletion include/libdnf5/utils/fs/temp.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ class TempDir {

/// A mkstemp wrapper that creates and owns a temporary file, which will be
/// deleted in the destructor unless released. Throws instances of
/// `std::filesystem::filesystem_error` on any I/O failure.
/// `libdnf5::FileSystemError` on any I/O failure.
class TempFile {
public:
/// Creates a temporary file in the system temporary directory path.
Expand Down
2 changes: 1 addition & 1 deletion libdnf5/base/transaction.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -695,7 +695,7 @@ Transaction::TransactionRunResult Transaction::Impl::_run(
module_db->save();
try {
base->p_impl->get_system_state().save();
} catch (const std::filesystem::filesystem_error & ex) {
} catch (const FileSystemError & ex) {
logger->error("Cannot save system state: {}", ex.what());
}
}
Expand Down
21 changes: 21 additions & 0 deletions libdnf5/common/exception.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -174,6 +174,27 @@ const char * SystemError::what() const noexcept {
return message.c_str();
}

const char * FileSystemError::what() const noexcept {
std::string error_message;
try {
error_message = std::system_category().default_error_condition(error_code).message();
} catch (...) {
}

try {
message = fmt::format(
"{}: ({}) - {} [{}]",
formatter ? formatter(TM_(format, 1)) : TM_(format, 1),
error_code,
error_message,
std::string(path));
} catch (...) {
return TM_(format, 1);
}

return message.c_str();
}

const char * RuntimeError::get_description() const noexcept {
return _("General RuntimeError exception");
}
Expand Down
4 changes: 2 additions & 2 deletions libdnf5/conf/config_parser.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -61,8 +61,8 @@ ConfigParserOptionNotFoundError::ConfigParserOptionNotFoundError(
void ConfigParser::read(const std::string & file_path) try {
IniParser parser(file_path);
::libdnf5::read(*this, parser);
} catch (const std::filesystem::filesystem_error & e) {
if (e.code().value() == ENOENT) {
} catch (const FileSystemError & e) {
if (e.get_error_code() == ENOENT) {
std::throw_with_nested(MissingConfigError(M_("Configuration file \"{}\" not found"), file_path));
} else {
std::throw_with_nested(InaccessibleConfigError(M_("Unable to access configuration file \"{}\""), file_path));
Expand Down
2 changes: 1 addition & 1 deletion libdnf5/conf/vars.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -421,7 +421,7 @@ void Vars::load_from_dir(const std::string & directory) {
try {
utils::fs::File file(full_path, "r");
file.read_line(line);
} catch (const std::filesystem::filesystem_error & e) {
} catch (const FileSystemError & e) {
logger.warning("Cannot load variable from file \"{}\": {}", full_path.c_str(), e.what());
continue;
}
Expand Down
4 changes: 2 additions & 2 deletions libdnf5/module/module_sack.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -580,7 +580,7 @@ class PlatformIdFormatError : public libdnf5::Error {
/// @param os_release_path Full path to the 'os-release' file.
/// @return Pair where first item is the platform module name and second is the platform stream.
/// @throws PlatformIdFormatError when platform id in the file has incorrect format.
/// @throws std::filesystem::filesystem_error when there is any error during reading a file.
/// @throws libdnf5::FileSystemError when there is any error during reading a file.
static std::pair<std::string, std::string> parse_platform_id_from_file(const std::string & os_release_path) {
libdnf5::utils::fs::File file{os_release_path, "r"};
std::string line;
Expand Down Expand Up @@ -709,7 +709,7 @@ std::optional<std::pair<std::string, std::string>> ModuleSack::Impl::detect_plat
} catch (const PlatformIdFormatError & id_except) {
base->get_logger()->debug(
"Invalid format of PLATFORM_ID in '{}': {}", full_path, std::string(id_except.what()));
} catch (const std::filesystem::filesystem_error & fs_except) {
} catch (const FileSystemError & fs_except) {
base->get_logger()->debug(
"Detection of module platform in '{}' failed: {}", full_path, std::string(fs_except.what()));
}
Expand Down
6 changes: 3 additions & 3 deletions libdnf5/repo/solv_repo.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -517,8 +517,8 @@ bool SolvRepo::load_solv_cache(solv::Pool & pool, const char * type_name, int fl
}
return true;
}
} catch (const std::filesystem::filesystem_error & e) {
if (e.code().default_error_condition() == std::errc::no_such_file_or_directory) {
} catch (const FileSystemError & e) {
if (std::error_code(e.get_error_code(), std::system_category()).default_error_condition() == std::errc::no_such_file_or_directory) {
logger.trace("Cache file \"{}\" not found", path.native());
} else {
logger.warning("Error opening cache file, ignoring: {}", e.what());
Expand Down Expand Up @@ -692,7 +692,7 @@ bool SolvRepo::read_group_solvable_from_xml(const std::string & path) {
fs::File ext_file;
try {
ext_file = fs::File(path, "r", true);
} catch (std::filesystem::filesystem_error & e) {
} catch (FileSystemError & e) {
logger.warning("Cannot load group extension for system repo from \"{}\": {}", path, e.what());
read_success = false;
}
Expand Down
3 changes: 2 additions & 1 deletion libdnf5/repo/temp_files_memory.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,8 @@ class TempFilesMemory {
/// The resulting list contains both existing and new paths and it's sorted and deduplicated before storing in the file.
/// @param paths A list of file paths to be added into the memory file.
/// @exception libdnf5::Error When an error occurs during parsing of the file with temporary files.
/// @exception std::filesystem::filesystem_error When an error occurs during accessing or writing the memory file.
/// @exception libdnf5::FileSystemprror When an error occurs during accessing or writing the memory file.
/// @exception std::filesystem::filesystem_error When an error occurs during renaming the memory file.
void add_files(const std::vector<std::string> & paths);

/// @brief Deletes the memory file.
Expand Down
2 changes: 1 addition & 1 deletion libdnf5/system/state.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -601,7 +601,7 @@ void State::reset_packages_states(
// In that case ignore the filesystem errors and only keep new system state in memory.
try {
save();
} catch (const std::filesystem::filesystem_error & e) {
} catch (const FileSystemError & e) {
// TODO(mblaha) - log this? (will need access to the base)
}
}
Expand Down
40 changes: 16 additions & 24 deletions libdnf5/utils/fs/file.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,9 @@ along with libdnf. If not, see <https://www.gnu.org/licenses/>.

#include "libdnf5/common/exception.hpp"

#include <libdnf5/utils/bgettext/bgettext-lib.h>
#include <libdnf5/utils/bgettext/bgettext-mark-domain.h>

extern "C" {
#include <solv/solv_xfopen.h>
}
Expand All @@ -34,6 +37,7 @@ extern "C" {

namespace libdnf5::utils::fs {


File::File(const std::filesystem::path & path, const char * mode, bool use_solv_xfopen) {
open(path, mode, use_solv_xfopen);
}
Expand Down Expand Up @@ -77,8 +81,7 @@ void File::open(const std::filesystem::path & path, const char * mode, bool use_

if (file == nullptr) {
this->path = "";
throw std::filesystem::filesystem_error(
"cannot open file", path, std::error_code(errno, std::system_category()));
throw FileSystemError(errno, path, M_("cannot open file"));
}

this->path = path;
Expand All @@ -91,8 +94,7 @@ void File::open(int fd, const std::filesystem::path & path, const char * mode, b
file = use_solv_xfopen_fd ? solv_xfopen_fd(path.c_str(), fd, mode) : ::fdopen(fd, mode);
if (file == nullptr) {
this->path = "";
throw std::filesystem::filesystem_error(
"cannot open file from fd", path, std::error_code(errno, std::system_category()));
throw FileSystemError(errno, path, M_("cannot open file from fd"));
}

this->path = path;
Expand All @@ -102,8 +104,7 @@ void File::open(int fd, const std::filesystem::path & path, const char * mode, b
void File::close() {
if (file != nullptr) {
if (std::fclose(file) != 0) {
throw std::filesystem::filesystem_error(
"cannot close file", path, std::error_code(errno, std::system_category()));
throw FileSystemError(errno, path, M_("cannot close file"));
}
}

Expand Down Expand Up @@ -133,8 +134,7 @@ std::size_t File::read(void * buffer, std::size_t count) {
libdnf_assert(
std::ferror(file) > 0, "Failed to read \"{}\", error expected but no error detected", path.native());

throw std::filesystem::filesystem_error(
"error reading file", path, std::error_code(errno, std::system_category()));
throw FileSystemError(errno, path, M_("error reading file"));
}

return res;
Expand All @@ -149,8 +149,7 @@ void File::write(const void * buffer, std::size_t count) {
libdnf_assert(
std::ferror(file) > 0, "Failed to write \"{}\", error expected but no error detected", path.native());

throw std::filesystem::filesystem_error(
"error writing file", path, std::error_code(errno, std::system_category()));
throw FileSystemError(errno, path, M_("error writing file"));
}
}

Expand All @@ -164,8 +163,7 @@ bool File::getc(char & c) {
return false;
}

throw std::filesystem::filesystem_error(
"error reading file", path, std::error_code(errno, std::system_category()));
throw FileSystemError(errno, path, M_("error reading file"));
}

c = static_cast<char>(res);
Expand All @@ -177,8 +175,7 @@ void File::putc(char c) {
libdnf_assert_file_open();

if (std::fputc(c, file) == EOF) {
throw std::filesystem::filesystem_error(
"error writing file", path, std::error_code(errno, std::system_category()));
throw FileSystemError(errno, path, M_("error writing file"));
}
}

Expand All @@ -187,8 +184,7 @@ void File::flush() {
libdnf_assert_file_open();

if (std::fflush(file) == -1) {
throw std::filesystem::filesystem_error(
"error flushing file", path, std::error_code(errno, std::system_category()));
throw FileSystemError(errno, path, M_("error flushing file"));
}
}

Expand All @@ -197,8 +193,7 @@ void File::seek(long offset, int whence) {
libdnf_assert_file_open();

if (std::fseek(file, offset, whence) == -1) {
throw std::filesystem::filesystem_error(
"error seeking in file", path, std::error_code(errno, std::system_category()));
throw FileSystemError(errno, path, M_("error seeking in file"));
}
}

Expand All @@ -209,8 +204,7 @@ long File::tell() const {
auto res = std::ftell(file);

if (res == -1) {
throw std::filesystem::filesystem_error(
"error retrieving file position", path, std::error_code(errno, std::system_category()));
throw FileSystemError(errno, path, M_("error retrieving file position"));
}

return res;
Expand Down Expand Up @@ -263,8 +257,7 @@ bool File::read_line(std::string & line) {
return false;
}

throw std::filesystem::filesystem_error(
"error reading a line from file", path, std::error_code(errno, std::system_category()));
throw FileSystemError(errno, path, M_("error reading a line from file"));
}

while (line_len > 0 && (buf[line_len - 1] == '\r' || buf[line_len - 1] == '\n')) {
Expand All @@ -287,8 +280,7 @@ int File::get_fd() const {
int fd = ::fileno(file);

if (fd == -1) {
throw std::filesystem::filesystem_error(
"error retrieving file descriptor", path, std::error_code(errno, std::system_category()));
throw FileSystemError(errno, path, M_("error retrieving file descriptor"));
}

return fd;
Expand Down
13 changes: 4 additions & 9 deletions libdnf5/utils/fs/temp.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -22,11 +22,9 @@ along with libdnf. If not, see <https://www.gnu.org/licenses/>.

#include "libdnf5/common/exception.hpp"

#include <stdio.h>
#include <libdnf5/utils/bgettext/bgettext-mark-domain.h>
#include <unistd.h>

#include <cstdlib>


namespace libdnf5::utils::fs {

Expand All @@ -41,8 +39,7 @@ TempDir::TempDir(std::filesystem::path destdir, const std::string & name_prefix)
std::string dest = destdir;
const char * temp_path = mkdtemp(dest.data());
if (temp_path == nullptr) {
throw std::filesystem::filesystem_error(
"cannot create temporary directory", destdir, std::error_code(errno, std::system_category()));
throw FileSystemError(errno, destdir, M_("cannot create temporary directory"));
}
path = temp_path;
}
Expand Down Expand Up @@ -89,8 +86,7 @@ TempFile::TempFile(std::filesystem::path destdir, const std::string & name_prefi
std::string dest = destdir;
fd = mkstemp(dest.data());
if (fd == -1) {
throw std::filesystem::filesystem_error(
"cannot create temporary file", destdir, std::error_code(errno, std::system_category()));
throw FileSystemError(errno, destdir, M_("cannot create temporary file"));
}

path = dest;
Expand Down Expand Up @@ -143,8 +139,7 @@ void TempFile::close() {
file.reset();
} else if (fd != -1) {
if (::close(fd) != 0) {
throw std::filesystem::filesystem_error(
"cannot close temporary file", path, std::error_code(errno, std::system_category()));
throw FileSystemError(errno, path, M_("cannot close temporary file"));
}
}

Expand Down
2 changes: 1 addition & 1 deletion test/libdnf5/utils/test_fs.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -368,7 +368,7 @@ void UtilsFsTest::test_file_seek() {
CPPUNIT_ASSERT_EQUAL(9l, file.tell());
CPPUNIT_ASSERT_EQUAL(data_w.substr(9), file.read());

CPPUNIT_ASSERT_THROW(file.seek(-1, SEEK_SET), std::filesystem::filesystem_error);
CPPUNIT_ASSERT_THROW(file.seek(-1, SEEK_SET), libdnf5::FileSystemError);
}


Expand Down

0 comments on commit 0170c6c

Please sign in to comment.