Skip to content

Commit

Permalink
refactor: clean up & simplify error handling
Browse files Browse the repository at this point in the history
  • Loading branch information
mhx committed Jan 5, 2024
1 parent 17e4b19 commit 3bdf262
Show file tree
Hide file tree
Showing 13 changed files with 60 additions and 84 deletions.
5 changes: 1 addition & 4 deletions include/dwarfs/error.h
Original file line number Diff line number Diff line change
Expand Up @@ -36,10 +36,7 @@ class error : public std::exception {
int line() const { return line_; }

protected:
error(std::string const& s, char const* file, int line) noexcept
: what_(s)
, file_(file)
, line_(line) {}
error(std::string const& s, char const* file, int line) noexcept;

private:
std::string what_;
Expand Down
7 changes: 7 additions & 0 deletions src/dwarfs/error.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,8 @@
#include <cstdlib>
#include <iostream>

#include <fmt/format.h>

#include <folly/String.h>

#ifdef DWARFS_USE_EXCEPTION_TRACER
Expand All @@ -47,6 +49,11 @@ namespace {

} // namespace

error::error(std::string const& s, char const* file, int line) noexcept
: what_{fmt::format("{} [{}:{}]", s, file, line)}
, file_{file}
, line_{line} {}

system_error::system_error(char const* file, int line) noexcept
: system_error(errno, file, line) {}

Expand Down
14 changes: 4 additions & 10 deletions src/dwarfs/filesystem_v2.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -775,23 +775,17 @@ int filesystem_<LoggerPolicy>::check(filesystem_check_level level,
DWARFS_THROW(runtime_error, "duplicate section: " + s.name());
}
}
} catch (error const& e) {
LOG_ERROR << e.what() << " [" << e.file() << ":" << e.line() << "]";
++errors;
} catch (...) {
LOG_ERROR << folly::exceptionStr(std::current_exception());
} catch (std::exception const& e) {
LOG_ERROR << folly::exceptionStr(e);
++errors;
}
}

if (level == filesystem_check_level::FULL) {
try {
meta_.check_consistency();
} catch (error const& e) {
LOG_ERROR << e.what() << " [" << e.file() << ":" << e.line() << "]";
++errors;
} catch (...) {
LOG_ERROR << folly::exceptionStr(std::current_exception());
} catch (std::exception const& e) {
LOG_ERROR << folly::exceptionStr(e);
++errors;
}
}
Expand Down
2 changes: 0 additions & 2 deletions src/dwarfs/inode_reader_v2.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -279,8 +279,6 @@ inode_reader_<LoggerPolicy>::read_internal(uint32_t inode, size_t size,
num_read += br.size();
}
return num_read;
} catch (runtime_error const& e) {
LOG_ERROR << e.what();
} catch (...) {
LOG_ERROR << folly::exceptionStr(std::current_exception());
}
Expand Down
13 changes: 2 additions & 11 deletions src/dwarfs/safe_main.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -42,18 +42,9 @@ int safe_main(std::function<int(void)> fn) {
terminal::setup();

return fn();
} catch (system_error const& e) {
std::cerr << "ERROR: " << folly::exceptionStr(e) << " [" << e.file() << ":"
<< e.line() << "]\n";
dump_exceptions();
} catch (error const& e) {
std::cerr << "ERROR: " << folly::exceptionStr(e) << " [" << e.file() << ":"
<< e.line() << "]\n";
dump_exceptions();
} catch (std::exception const& e) {
std::cerr << "ERROR: " << folly::exceptionStr(e) << "\n";
dump_exceptions();
} catch (...) {
std::cerr << "ERROR: " << folly::exceptionStr(std::current_exception())
<< "\n";
dump_exceptions();
}
return 1;
Expand Down
2 changes: 1 addition & 1 deletion src/dwarfs/scanner.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -421,7 +421,7 @@ scanner_<LoggerPolicy>::add_entry(std::filesystem::path const& name,

return pe;
} catch (const std::system_error& e) {
LOG_ERROR << "error reading entry: " << e.what();
LOG_ERROR << "error reading entry: " << folly::exceptionStr(e);
prog.errors++;
}

Expand Down
58 changes: 29 additions & 29 deletions src/dwarfs_main.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -273,10 +273,10 @@ void op_lookup(fuse_req_t req, fuse_ino_t parent, char const* name) {
}
}
} catch (dwarfs::system_error const& e) {
LOG_ERROR << e.what();
LOG_ERROR << folly::exceptionStr(e);
err = e.get_errno();
} catch (std::exception const& e) {
LOG_ERROR << e.what();
LOG_ERROR << folly::exceptionStr(e);
err = EIO;
}

Expand All @@ -303,10 +303,10 @@ int op_getattr_common(LogProxy& log_, dwarfs_userdata& userdata,
}
}
} catch (dwarfs::system_error const& e) {
LOG_ERROR << e.what();
LOG_ERROR << folly::exceptionStr(e);
err = e.get_errno();
} catch (std::exception const& e) {
LOG_ERROR << e.what();
LOG_ERROR << folly::exceptionStr(e);
err = EIO;
}

Expand Down Expand Up @@ -357,10 +357,10 @@ int op_access_common(LogProxy& log_, dwarfs_userdata& userdata, int mode,
err = userdata.fs.access(*entry, mode, uid, gid);
}
} catch (dwarfs::system_error const& e) {
LOG_ERROR << e.what();
LOG_ERROR << folly::exceptionStr(e);
err = e.get_errno();
} catch (std::exception const& e) {
LOG_ERROR << e.what();
LOG_ERROR << folly::exceptionStr(e);
err = EIO;
}

Expand Down Expand Up @@ -411,10 +411,10 @@ int op_readlink_common(LogProxy& log_, dwarfs_userdata& userdata,
err = userdata.fs.readlink(*entry, str, readlink_mode::unix);
}
} catch (dwarfs::system_error const& e) {
LOG_ERROR << e.what();
LOG_ERROR << folly::exceptionStr(e);
err = e.get_errno();
} catch (std::exception const& e) {
LOG_ERROR << e.what();
LOG_ERROR << folly::exceptionStr(e);
err = EIO;
}

Expand Down Expand Up @@ -491,10 +491,10 @@ int op_open_common(LogProxy& log_, dwarfs_userdata& userdata,
}
}
} catch (dwarfs::system_error const& e) {
LOG_ERROR << e.what();
LOG_ERROR << folly::exceptionStr(e);
err = e.get_errno();
} catch (std::exception const& e) {
LOG_ERROR << e.what();
LOG_ERROR << folly::exceptionStr(e);
err = EIO;
}

Expand Down Expand Up @@ -569,10 +569,10 @@ void op_read(fuse_req_t req, fuse_ino_t ino, size_t size, file_off_t off,
err = EIO;
}
} catch (dwarfs::system_error const& e) {
LOG_ERROR << e.what();
LOG_ERROR << folly::exceptionStr(e);
err = e.get_errno();
} catch (std::exception const& e) {
LOG_ERROR << e.what();
LOG_ERROR << folly::exceptionStr(e);
err = EIO;
}

Expand Down Expand Up @@ -602,10 +602,10 @@ int op_read(char const* path, char* buf, size_t size, native_off_t off,

err = rv;
} catch (dwarfs::system_error const& e) {
LOG_ERROR << e.what();
LOG_ERROR << folly::exceptionStr(e);
err = -e.get_errno();
} catch (std::exception const& e) {
LOG_ERROR << e.what();
LOG_ERROR << folly::exceptionStr(e);
err = -EIO;
}

Expand Down Expand Up @@ -672,10 +672,10 @@ void op_readdir(fuse_req_t req, fuse_ino_t ino, size_t size, file_off_t off,
err = ENOTDIR;
}
} catch (dwarfs::system_error const& e) {
LOG_ERROR << e.what();
LOG_ERROR << folly::exceptionStr(e);
err = e.get_errno();
} catch (std::exception const& e) {
LOG_ERROR << e.what();
LOG_ERROR << folly::exceptionStr(e);
err = EIO;
}

Expand Down Expand Up @@ -731,10 +731,10 @@ int op_readdir(char const* path, void* buf, fuse_fill_dir_t filler,
err = -ENOTDIR;
}
} catch (dwarfs::system_error const& e) {
LOG_ERROR << e.what();
LOG_ERROR << folly::exceptionStr(e);
err = -e.get_errno();
} catch (std::exception const& e) {
LOG_ERROR << e.what();
LOG_ERROR << folly::exceptionStr(e);
err = -EIO;
}

Expand Down Expand Up @@ -763,10 +763,10 @@ int op_statfs_common(LogProxy& log_, dwarfs_userdata& userdata,
#endif
}
} catch (dwarfs::system_error const& e) {
LOG_ERROR << e.what();
LOG_ERROR << folly::exceptionStr(e);
err = e.get_errno();
} catch (std::exception const& e) {
LOG_ERROR << e.what();
LOG_ERROR << folly::exceptionStr(e);
err = EIO;
}

Expand Down Expand Up @@ -867,10 +867,10 @@ void op_getxattr(fuse_req_t req, fuse_ino_t ino, char const* name,
}
}
} catch (dwarfs::system_error const& e) {
LOG_ERROR << e.what();
LOG_ERROR << folly::exceptionStr(e);
err = e.get_errno();
} catch (std::exception const& e) {
LOG_ERROR << e.what();
LOG_ERROR << folly::exceptionStr(e);
err = EIO;
}

Expand Down Expand Up @@ -922,10 +922,10 @@ void op_listxattr(fuse_req_t req, fuse_ino_t ino, size_t size) {
return;
}
} catch (dwarfs::system_error const& e) {
LOG_ERROR << e.what();
LOG_ERROR << folly::exceptionStr(e);
err = e.get_errno();
} catch (std::exception const& e) {
LOG_ERROR << e.what();
LOG_ERROR << folly::exceptionStr(e);
err = EIO;
}

Expand Down Expand Up @@ -1329,11 +1329,11 @@ int dwarfs_main(int argc, sys_char** argv, iolayer const& iol) {
parse_time_with_unit(opts.cache_tidy_max_age_str);
}
}
} catch (runtime_error const& e) {
iol.err << "error: " << e.what() << "\n";
return 1;
} catch (std::filesystem::filesystem_error const& e) {
iol.err << e.what() << "\n";
iol.err << folly::exceptionStr(e) << "\n";
return 1;
} catch (std::exception const& e) {
iol.err << "error: " << folly::exceptionStr(e) << "\n";
return 1;
}

Expand All @@ -1359,7 +1359,7 @@ int dwarfs_main(int argc, sys_char** argv, iolayer const& iol) {
load_filesystem<prod_logger_policy>(userdata);
}
} catch (std::exception const& e) {
LOG_ERROR << "error initializing file system: " << e.what();
LOG_ERROR << "error initializing file system: " << folly::exceptionStr(e);
return 1;
}

Expand Down
8 changes: 4 additions & 4 deletions src/dwarfsbench_main.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -117,8 +117,8 @@ int dwarfsbench_main(int argc, sys_char** argv, iolayer const& iol) {
int fh = fs.open(inode_data);
fs.read(fh, buf.data(), buf.size());
}
} catch (runtime_error const& e) {
iol.err << "error: " << e.what() << "\n";
} catch (std::exception const& e) {
iol.err << "error: " << folly::exceptionStr(e) << "\n";
} catch (...) {
iol.err << "error: "
<< folly::exceptionStr(std::current_exception()) << "\n";
Expand All @@ -129,8 +129,8 @@ int dwarfsbench_main(int argc, sys_char** argv, iolayer const& iol) {
});

wg.wait();
} catch (runtime_error const& e) {
iol.err << "error: " << e.what() << "\n";
} catch (std::exception const& e) {
iol.err << "error: " << folly::exceptionStr(e) << "\n";
return 1;
}

Expand Down
9 changes: 2 additions & 7 deletions src/dwarfsck_main.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@
#include "dwarfs/options.h"
#include "dwarfs/os_access.h"
#include "dwarfs/tool.h"
#include "dwarfs/util.h"
#include "dwarfs_tool_main.h"

namespace dwarfs {
Expand Down Expand Up @@ -189,13 +190,7 @@ int dwarfsck_main(int argc, sys_char** argv, iolayer const& iol) {
}
}
}
} catch (system_error const& e) {
iol.err << folly::exceptionStr(e) << "\n";
return 1;
} catch (runtime_error const& e) {
iol.err << folly::exceptionStr(e) << "\n";
return 1;
} catch (std::system_error const& e) {
} catch (std::exception const& e) {
iol.err << folly::exceptionStr(e) << "\n";
return 1;
}
Expand Down
8 changes: 1 addition & 7 deletions src/dwarfsextract_main.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -192,13 +192,7 @@ int dwarfsextract_main(int argc, sys_char** argv, iolayer const& iol) {
if (perfmon) {
perfmon->summarize(iol.err);
}
} catch (runtime_error const& e) {
iol.err << folly::exceptionStr(e) << "\n";
return 1;
} catch (system_error const& e) {
iol.err << folly::exceptionStr(e) << "\n";
return 1;
} catch (std::system_error const& e) {
} catch (std::exception const& e) {
iol.err << folly::exceptionStr(e) << "\n";
return 1;
}
Expand Down
7 changes: 2 additions & 5 deletions src/mkdwarfs_main.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1296,11 +1296,8 @@ int mkdwarfs_main(int argc, sys_char** argv, iolayer const& iol) {

options.inode.categorizer_mgr.reset();
}
} catch (runtime_error const& e) {
LOG_ERROR << e.what() << " [" << e.file() << ":" << e.line() << "]";
return 1;
} catch (system_error const& e) {
LOG_ERROR << e.what() << " [" << e.file() << ":" << e.line() << "]";
} catch (std::exception const& e) {
LOG_ERROR << folly::exceptionStr(e);
return 1;
}

Expand Down
6 changes: 3 additions & 3 deletions test/block_cache_test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ TEST(block_range, uncompressed) {

EXPECT_THAT([] { block_range range(nullptr, 0, 0); },
::testing::ThrowsMessage<dwarfs::runtime_error>(
"block_range: block data is null"));
::testing::HasSubstr("block_range: block data is null")));
}

TEST(block_range, compressed) {
Expand Down Expand Up @@ -109,13 +109,13 @@ TEST(block_range, compressed) {
block_range range(block, 0, 0);
},
::testing::ThrowsMessage<dwarfs::runtime_error>(
"block_range: block data is null"));
::testing::HasSubstr("block_range: block data is null")));

EXPECT_THAT(
[&] {
auto block = std::make_shared<mock_cached_block>(data);
block_range range(block, 100, 1);
},
::testing::ThrowsMessage<dwarfs::runtime_error>(
"block_range: size out of range (101 > 100)"));
::testing::HasSubstr("block_range: size out of range (101 > 100)")));
}
5 changes: 4 additions & 1 deletion test/error_test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,8 @@
#include <cerrno>
#include <filesystem>

#include <fmt/format.h>

#include <gmock/gmock.h>
#include <gtest/gtest.h>

Expand Down Expand Up @@ -57,7 +59,8 @@ TEST(error_test, runtime_error) {
test_throw_runtime_error(true);
FAIL() << "expected runtime_error to be thrown";
} catch (const runtime_error& e) {
EXPECT_EQ("my test error", std::string(e.what()));
EXPECT_EQ(fmt::format("my test error [{}:{}]", e.file(), e.line()),
std::string(e.what()));
EXPECT_EQ("error_test.cpp",
std::filesystem::path(e.file()).filename().string());
EXPECT_EQ(expected_line, e.line());
Expand Down

0 comments on commit 3bdf262

Please sign in to comment.