From 883cd862876ff4199525c39094b6fbbd0a5a1bd8 Mon Sep 17 00:00:00 2001
From: Balazs Racz <balazs.racz@gmail.com>
Date: Sat, 7 Sep 2024 13:48:56 +0200
Subject: [PATCH] Removes NullErrorHandler. Instead, prevents generating an OIR
 for any incoming unhandled error message (OIR and TDE).

---
 src/openlcb/IfCan.cxx  |  1 -
 src/openlcb/IfImpl.hxx | 35 ++++++++---------------------------
 src/openlcb/IfTcp.cxx  |  1 -
 3 files changed, 8 insertions(+), 29 deletions(-)

diff --git a/src/openlcb/IfCan.cxx b/src/openlcb/IfCan.cxx
index dad789844..f8678d82d 100644
--- a/src/openlcb/IfCan.cxx
+++ b/src/openlcb/IfCan.cxx
@@ -721,7 +721,6 @@ IfCan::IfCan(ExecutorBase *executor, CanHubFlow *device,
     add_owned_flow(new FrameToGlobalMessageParser(this));
     add_owned_flow(new VerifyNodeIdHandler(this));
     add_owned_flow(new UnhandledAddressedMessageHandler(this));
-    add_owned_flow(new NullErrorHandler(this));
     add_owned_flow(new RemoteAliasCacheUpdater(this));
     add_owned_flow(new AMEQueryHandler(this));
     add_owned_flow(new AMEGlobalQueryHandler(this));
diff --git a/src/openlcb/IfImpl.hxx b/src/openlcb/IfImpl.hxx
index 47d0df4c8..126553cb3 100644
--- a/src/openlcb/IfImpl.hxx
+++ b/src/openlcb/IfImpl.hxx
@@ -293,6 +293,14 @@ public:
             // Destination is not a local node.
             return release_and_exit();
         }
+        auto mti = message()->data()->mti;
+        if (mti == Defs::MTI_OPTIONAL_INTERACTION_REJECTED ||
+            mti == Defs::MTI_TERMINATE_DUE_TO_ERROR) {
+            // We don't generate an OIR for an incoming error report, as this
+            // generally would cause an infinite bouncing of error reports back
+            // and forth between two OpenMRN nodes.
+            return release_and_exit();
+        }
         return allocate_and_call(
             iface()->addressed_message_write_flow(), STATE(fill_oir));
     }
@@ -311,33 +319,6 @@ public:
     }
 };
 
-/// This is an addressed message handler that catches errors (OIR and TDE) but
-/// ignores them. While this sounds not super useful, it ensures that these
-/// messages never trigger the UnhandledAddressedMessageHandler.
-class NullErrorHandler : public IncomingMessageStateFlow
-{
-public:
-    NullErrorHandler(If *service)
-        : IncomingMessageStateFlow(service)
-    {
-        service->dispatcher()->register_handler(
-            this, Defs::MTI_OPTIONAL_INTERACTION_REJECTED, Defs::MTI_EXACT);
-        service->dispatcher()->register_handler(
-            this, Defs::MTI_TERMINATE_DUE_TO_ERROR, Defs::MTI_EXACT);
-    }
-
-    void send(Buffer<GenMessage> *b, unsigned) override
-    {
-        b->unref();
-    }
-
-    Action entry() override
-    {
-        DIE("should not get here");
-        return release_and_exit();
-    }
-};
-
 } // namespace openlcb
 
 #endif // _OPENLCB_IFIMPL_HXX_
diff --git a/src/openlcb/IfTcp.cxx b/src/openlcb/IfTcp.cxx
index f603802ab..f1cd43c57 100644
--- a/src/openlcb/IfTcp.cxx
+++ b/src/openlcb/IfTcp.cxx
@@ -188,7 +188,6 @@ IfTcp::IfTcp(NodeID gateway_node_id, HubFlow *device, int local_nodes_count)
     , device_(device)
 {
     add_owned_flow(new VerifyNodeIdHandler(this));
-    add_owned_flow(new NullErrorHandler(this));
     add_owned_flow(new UnhandledAddressedMessageHandler(this));
     seq_ = new ClockBaseSequenceNumberGenerator;
     add_owned_flow(seq_);