dbwrapper: add TryRead() to distinguish errors from valid outcomes

Read() returns false for both a missing key and a deserialization
failure, making it impossible for callers to distinguish between
them.

This commits adds TryRead() returning a ReadStatus struct that
discriminates between:

- true:                  record found, value deserialized
- false:                 record not found
- DatabaseError:         levelDB threw during record read
- DeserializationError:  key present, value incompatible with
                         expected format

An err_msg field preserves the original exception message for
diagnostic purposes.

This also makes Read() a thin wrapper over TryRead() to keep
existing call sites unchanged.

Note:
Key serialization is the only operation that may throw in
TryRead(), as callers are expected to provide well-formed keys.
This is why this function is not noexcept.
This commit is contained in:
furszy
2026-03-24 23:51:41 -04:00
parent f78834fac9
commit 5dfbb91b6c
2 changed files with 115 additions and 6 deletions

View File

@@ -10,6 +10,7 @@
#include <span.h>
#include <streams.h>
#include <util/check.h>
#include <util/expected.h>
#include <util/fs.h>
#include <cstddef>
@@ -203,26 +204,84 @@ public:
CDBWrapper(const CDBWrapper&) = delete;
CDBWrapper& operator=(const CDBWrapper&) = delete;
struct ReadFailure {
enum class Code {
DeserializationError, //!< Key exists but value could not be deserialized.
DatabaseError, //!< Unexpected internal DB error.
};
Code status;
std::string err_msg;
};
using ReadStatus = util::Expected<bool, ReadFailure>;
/**
* Read and deserialize a value from the database, with explicit error discrimination.
*
* Unlike Read(), this method distinguishes between a missing key, a deserialization
* failure (DeserializationError), and an internal DB error (DatabaseError),
* enabling callers to treat data corruption differently from an absent entry.
*
* @note Callers are expected to provide well-formed keys; key serialization
* is the only operation that may throw.
*
* @param[in] key The key to look up.
* @param[out] value Populated with the deserialized value when the returned
* Expected holds true; indeterminate otherwise.
* @return On success, true if the key was found (value populated) or false if
* the key was absent. On failure, a ReadFailure describing the error.
*/
template <typename K, typename V>
bool Read(const K& key, V& value) const
[[nodiscard]] ReadStatus TryRead(const K& key, V& value) const
{
DataStream ssKey{};
ssKey.reserve(DBWRAPPER_PREALLOC_KEY_SIZE);
// Key serialization is the only operation that may throw.
// Callers are expected to provide well-formed keys.
ssKey << key;
std::optional<std::string> strValue{ReadImpl(ssKey)};
if (!strValue) {
return false;
std::optional<std::string> strValue;
try {
strValue = ReadImpl(ssKey);
if (!strValue) {
return false; // not found
}
} catch (const std::exception& e) {
return util::Unexpected(ReadFailure{ReadFailure::Code::DatabaseError, e.what()});
}
try {
std::span ssValue{MakeWritableByteSpan(*strValue)};
m_obfuscation(ssValue);
SpanReader{ssValue} >> value;
} catch (const std::exception&) {
return false;
} catch (const std::exception& e) {
return util::Unexpected(ReadFailure{ReadFailure::Code::DeserializationError, e.what()});
}
return true;
}
/**
* Wrapper around TryRead() that preserves the original Read() semantics:
* returns true on success, false if the key is absent or deserialization
* fails, and throws dbwrapper_error on an internal DB error.
*
* Prefer TryRead() when the caller needs to distinguish between a missing
* key and a corrupt value.
*/
template <typename K, typename V>
bool Read(const K& key, V& value) const
{
const ReadStatus res = TryRead(key,value);
if (res.has_value()) return res.value();
switch (const auto& [err_code, err_msg] = res.error(); err_code) {
case ReadFailure::Code::DeserializationError: return false;
case ReadFailure::Code::DatabaseError: throw dbwrapper_error(err_msg);
} // no default case, so the compiler can warn about missing cases
std::abort(); // unreachable
}
template <typename K, typename V>
void Write(const K& key, const V& value, bool fSync = false)
{

View File

@@ -239,6 +239,56 @@ BOOST_AUTO_TEST_CASE(dbwrapper_read_throws_on_db_error)
const auto db{make_db(path, /*force_compact=*/false)};
uint256 result;
BOOST_CHECK_EXCEPTION(db.Read(key, result), dbwrapper_error, HasReason("Fatal LevelDB error"));
// TryRead() must return DatabaseError (without throwing).
CDBWrapper::ReadStatus status = db.TryRead(key, result);
BOOST_REQUIRE(!status);
BOOST_CHECK(status.error().status == CDBWrapper::ReadFailure::Code::DatabaseError);
BOOST_CHECK(status.error().err_msg.find("Fatal LevelDB error") != std::string::npos);
}
// Exercise TryRead() return values directly: found, absent and DeserializationError.
// DatabaseError is tested inside 'dbwrapper_read_throws_on_db_error' test
BOOST_AUTO_TEST_CASE(dbwrapper_tryread)
{
for (const bool obfuscate : {false, true}) {
const fs::path path{m_args.GetDataDirBase() / (obfuscate ? "dbwrapper_tryread_obf" : "dbwrapper_tryread_noobf")};
CDBWrapper dbw({.path = path, .cache_bytes = 1 << 20, .wipe_data = true, .obfuscate = obfuscate});
constexpr uint8_t key_ok{'A'};
constexpr uint8_t key_missing{'B'};
constexpr uint8_t key_bad{'C'};
uint256 written_value{m_rng.rand256()};
dbw.Write(key_ok, written_value);
dbw.Write(key_bad, uint8_t{0xFF});
// Found: key exists, value deserializes correctly
{
uint256 read_value;
CDBWrapper::ReadStatus status = dbw.TryRead(key_ok, read_value);
BOOST_REQUIRE(status);
BOOST_CHECK(status.value());
BOOST_CHECK_EQUAL(read_value, written_value);
}
// Absent: key does not exist
{
uint256 read_value;
CDBWrapper::ReadStatus status = dbw.TryRead(key_missing, read_value);
BOOST_REQUIRE(status);
BOOST_CHECK(!status.value());
}
// DeserializationError: key exists but stored value is too short
{
uint256 read_value;
CDBWrapper::ReadStatus status = dbw.TryRead(key_bad, read_value);
BOOST_REQUIRE(!status);
BOOST_CHECK(status.error().status == CDBWrapper::ReadFailure::Code::DeserializationError);
BOOST_CHECK(!status.error().err_msg.empty());
}
}
}
BOOST_AUTO_TEST_CASE(dbwrapper_iterator)