Review fixes: catch exception in a sensible place.

No need to overload SHAMapMissingNode, instead use std::runtime_error.
XRPLF · Mar 14, 2023 · fddd14c · fddd14c
1 parent 746579f
commit fddd14c
Show file tree

Hide file tree

Showing 3 changed files with 28 additions and 24 deletions.
diff --git a/src/ripple/shamap/SHAMapMissingNode.h b/src/ripple/shamap/SHAMapMissingNode.h
@@ -33,7 +33,6 @@ enum class SHAMapType {
     TRANSACTION = 1,  // A tree of transactions
     STATE = 2,        // A tree of state nodes
     FREE = 3,         // A tree not part of a ledger
-    UNKNOWN = 4       // Could be any type, but it's not there nonetheless
 };
 
 inline std::string
@@ -47,8 +46,6 @@ to_string(SHAMapType t)
             return "State Tree";
         case SHAMapType::FREE:
             return "Free Tree";
-        case SHAMapType::UNKNOWN:
-            return "Unknown Tree";
         default:
             return std::to_string(
                 safe_cast<std::underlying_type_t<SHAMapType>>(t));
@@ -69,13 +66,6 @@ class SHAMapMissingNode : public std::runtime_error
               "Missing Node: " + to_string(t) + ": id " + to_string(id))
     {
     }
-
-    SHAMapMissingNode(uint256 const& id)
-        : std::runtime_error(
-              "Missing Node: " + to_string(SHAMapType::UNKNOWN) + ": hash " +
-              to_string(id))
-    {
-    }
 };
 
 }  // namespace ripple

diff --git a/src/ripple/shamap/impl/NodeFamily.cpp b/src/ripple/shamap/impl/NodeFamily.cpp
@@ -20,9 +20,8 @@
 #include <ripple/app/ledger/LedgerMaster.h>
 #include <ripple/app/main/Application.h>
 #include <ripple/app/main/Tuning.h>
-#include <ripple/basics/contract.h>
 #include <ripple/shamap/NodeFamily.h>
-#include <ripple/shamap/SHAMapMissingNode.h>
+#include <sstream>
 
 namespace ripple {
 
@@ -71,7 +70,12 @@ NodeFamily::missingNodeAcquireBySeq(std::uint32_t seq, uint256 const& nodeHash)
 {
     JLOG(j_.error()) << "Missing node in " << seq;
     if (app_.config().reporting())
-        Throw<SHAMapMissingNode>(nodeHash);
+    {
+        std::stringstream ss;
+        ss << "Node not read, likely a Cassandra error in ledger seq " << seq
+           << " object hash " << nodeHash;
+        Throw<std::runtime_error>(ss.str());
+    }
 
     std::unique_lock<std::mutex> lock(maxSeqMutex_);
     if (maxSeq_ == 0)

diff --git a/src/ripple/shamap/impl/SHAMap.cpp b/src/ripple/shamap/impl/SHAMap.cpp
@@ -173,30 +173,40 @@ SHAMap::finishFetch(
     std::shared_ptr<NodeObject> const& object) const
 {
     assert(backed_);
-    if (!object)
-    {
-        if (full_)
-        {
-            full_ = false;
-            f_.missingNodeAcquireBySeq(ledgerSeq_, hash.as_uint256());
-        }
-        return {};
-    }
 
     std::shared_ptr<SHAMapTreeNode> node;
     try
     {
+        if (!object)
+        {
+            if (full_)
+            {
+                full_ = false;
+                f_.missingNodeAcquireBySeq(ledgerSeq_, hash.as_uint256());
+            }
+            return {};
+        }
+
         node =
             SHAMapTreeNode::makeFromPrefix(makeSlice(object->getData()), hash);
         if (node)
             canonicalize(hash, node);
         return node;
     }
-    catch (std::exception const&)
+    catch (SHAMapMissingNode const& e)
+    {
+        JLOG(journal_.warn()) << "Missing node: " << hash << " : " << e.what();
+    }
+    catch (std::runtime_error const& e)
+    {
+        JLOG(journal_.warn()) << e.what();
+    }
+    catch (...)
     {
         JLOG(journal_.warn()) << "Invalid DB node " << hash;
-        return std::shared_ptr<SHAMapTreeNode>();
     }
+
+    return std::shared_ptr<SHAMapTreeNode>();
 }
 
 // See if a sync filter has a node