Exception uninit (#114)

* * Working on feature request to add exceptions on reading uninitialized variables in the KB * [skip ci] * * Fixed test_karl_exceptions.cpp to use the appropriate settings
jredmondson · Oct 5, 2018 · e2e2dbe · e2e2dbe
1 parent 0573f4b
commit e2e2dbe
Show file tree

Hide file tree

Showing 10 changed files with 422 additions and 63 deletions.
diff --git a/include/madara/exceptions/UninitializedException.h b/include/madara/exceptions/UninitializedException.h
@@ -0,0 +1,23 @@
+/* -*- C++ -*- */
+#ifndef _MADARA_EXCEPTIONS_UNINITIALIZEDEXCEPTION_H_
+#define _MADARA_EXCEPTIONS_UNINITIALIZEDEXCEPTION_H_
+
+#include <string>
+#include "MadaraException.h"
+
+namespace madara
+{
+namespace exceptions
+{
+/**
+ * An exception for attempting to access an invalid context1
+ **/
+class UninitializedException : public MadaraException
+{
+public:
+  using MadaraException::MadaraException;
+};
+}
+}
+
+#endif /* _MADARA_EXCEPTIONS_UNINITIALIZEDEXCEPTION_H_ */
diff --git a/include/madara/expression/CompositeArrayReference.cpp b/include/madara/expression/CompositeArrayReference.cpp
@@ -76,7 +76,9 @@ madara::expression::CompositeArrayReference::item() const
     return ref_.get_record_unsafe()->retrieve_index(index);
   }
   else
+  {
     return context_.get(expand_key()).retrieve_index(index);
+  }
 }
 
 /// Prune the tree of unnecessary nodes.
@@ -138,10 +140,34 @@ madara::expression::CompositeArrayReference::evaluate(
 
   if (ref_.is_valid())
   {
-    return ref_.get_record_unsafe()->retrieve_index(index);
+    auto ret = ref_.get_record_unsafe();
+
+    if (settings.exception_on_unitialized && !ret->exists ())
+    {
+      std::stringstream buffer;
+      buffer << "madara::expression::CompositeArrayReference::evaluate: ";
+      buffer << "ERROR: settings do not allow reads of unset vars and ";
+      buffer << ref_.get_name() << " is uninitialized";
+      throw exceptions::UninitializedException (buffer.str ());
+    }
+
+    return ret->retrieve_index(index);
   }
   else
-    return context_.get(expand_key()).retrieve_index(index);
+  {
+    auto ret = context_.get(expand_key()).retrieve_index(index);
+
+    if (settings.exception_on_unitialized && !ret.exists ())
+    {
+      std::stringstream buffer;
+      buffer << "madara::expression::CompositeArrayReference::evaluate: ";
+      buffer << "ERROR: settings do not allow reads of unset vars and ";
+      buffer << ref_.get_name() << " is uninitialized";
+      throw exceptions::UninitializedException (buffer.str ());
+    }
+
+    return ret;
+  }
 }
 
 const std::string& madara::expression::CompositeArrayReference::key() const

diff --git a/include/madara/expression/VariableNode.cpp b/include/madara/expression/VariableNode.cpp
@@ -4,6 +4,7 @@
 #include "madara/expression/VariableNode.h"
 #include "madara/utility/Utility.h"
 #include "VariableExpander.h"
+#include "madara/exceptions/UninitializedException.h"
 
 #include <string>
 #include <sstream>
@@ -252,10 +253,41 @@ madara::knowledge::KnowledgeRecord madara::expression::VariableNode::prune(
 madara::knowledge::KnowledgeRecord madara::expression::VariableNode::evaluate(
     const madara::knowledge::KnowledgeUpdateSettings& settings)
 {
+  madara_logger_ptr_log(logger_, logger::LOG_TRACE,
+      "madara::expression::VariableNode::evaluate: "
+      "Returning variable %s.\n",
+      key_.c_str());
+
   if (ref_.is_valid())
-    return *ref_.get_record_unsafe();
+  {
+    auto ret = ref_.get_record_unsafe();
+
+    if (settings.exception_on_unitialized && !ret->exists ())
+    {
+      std::stringstream buffer;
+      buffer << "madara::expression::VariableNode::evaluate: ";
+      buffer << "ERROR: settings do not allow reads of unset vars and ";
+      buffer << ref_.get_name() << " is uninitialized";
+      throw exceptions::UninitializedException (buffer.str ());
+    }
+
+    return *ret;
+  }
   else
-    return context_.get(expand_key(), settings);
+  {
+    auto ret = context_.get(expand_key(), settings);
+
+    if (settings.exception_on_unitialized && !ret.exists ())
+    {
+      std::stringstream buffer;
+      buffer << "madara::expression::VariableNode::evaluate: ";
+      buffer << "ERROR: settings do not allow reads of unset vars and ";
+      buffer << ref_.get_name() << " is uninitialized";
+      throw exceptions::UninitializedException (buffer.str ());
+    }
+
+    return ret;
+  }
 }
 
 const std::string& madara::expression::VariableNode::key() const

diff --git a/include/madara/knowledge/EvalSettings.h b/include/madara/knowledge/EvalSettings.h
@@ -97,17 +97,24 @@ struct MADARA_EXPORT EvalSettings : public KnowledgeUpdateSettings
    * @param  t_treat_locals_as_globals true if local variable changes should
    *                                   be sent over the network (dangerous).
    *                                   @see treat_locals_as_globals
+   * @param  t_stream_changes          true if changes must be streamed
+   * @param  t_exceptions_on_unitialized true if exceptions must be thrown
+   *                                   when reading uninitialized variables
    **/
   explicit EvalSettings(bool t_delay_sending_modifieds,
       bool t_treat_globals_as_locals = false, bool t_signal_updates = true,
       bool t_always_overwrite = false, bool t_always_expand = true,
       bool t_track_local_changes = false,
       std::string t_pre_print_statement = "",
       std::string t_post_print_statement = "", uint64_t t_clock_increment = 1,
-      bool t_treat_locals_as_globals = false)
+      bool t_treat_locals_as_globals = false,
+      bool t_stream_changes = true,
+      bool t_exceptions_on_unitialized = false)
     : KnowledgeUpdateSettings(t_treat_globals_as_locals, t_signal_updates,
           t_always_overwrite, t_always_expand, t_track_local_changes,
-          t_clock_increment, t_treat_locals_as_globals),
+          t_clock_increment, t_treat_locals_as_globals,
+          t_stream_changes,
+          t_exceptions_on_unitialized),
       delay_sending_modifieds(t_delay_sending_modifieds),
       pre_print_statement(t_pre_print_statement),
       post_print_statement(t_post_print_statement)

diff --git a/include/madara/knowledge/KnowledgeReferenceSettings.h b/include/madara/knowledge/KnowledgeReferenceSettings.h
@@ -29,21 +29,30 @@ class MADARA_EXPORT KnowledgeReferenceSettings
   /**
    * Constructor
    **/
-  KnowledgeReferenceSettings() : expand_variables(true), never_exit(false) {}
+  KnowledgeReferenceSettings()
+   : expand_variables(true), never_exit(false),
+     exception_on_unitialized(false)
+  {
+  }
 
   /**
    * Constructor
+   * @param t_expand_variables  if true, perform variable expansion
+   * @param t_exception         if uninitialized variable, throw exception
    **/
-  KnowledgeReferenceSettings(bool t_expand_variables)
-    : expand_variables(t_expand_variables), never_exit(false)
+  KnowledgeReferenceSettings(bool t_expand_variables, bool t_exception=false)
+    : expand_variables(t_expand_variables), never_exit(false),
+     exception_on_unitialized(t_exception)
   {
   }
 
   /**
    * Copy constructor
+   * @param rhs                 reference to copy
    **/
   KnowledgeReferenceSettings(const KnowledgeReferenceSettings& rhs)
-    : expand_variables(rhs.expand_variables), never_exit(rhs.never_exit)
+    : expand_variables(rhs.expand_variables), never_exit(rhs.never_exit),
+      exception_on_unitialized(rhs.exception_on_unitialized)
   {
   }
 
@@ -62,6 +71,9 @@ class MADARA_EXPORT KnowledgeReferenceSettings
    * Never allow MADARA to exit, even with fatal errors or invalid state
    **/
   bool never_exit;
+
+  /// throw an exception if reference is on uninitialized variable
+  bool exception_on_unitialized;
 };
 }
 }

diff --git a/include/madara/knowledge/KnowledgeUpdateSettings.h b/include/madara/knowledge/KnowledgeUpdateSettings.h
@@ -53,13 +53,16 @@ class MADARA_EXPORT KnowledgeUpdateSettings : public KnowledgeReferenceSettings
    * @param  t_treat_locals_as_globals true if local variable changes should
    *                                   be sent over the network (dangerous).
    *                                   @see treat_locals_as_globals
+   * @param  t_stream_changes          true if changes must be streamed
+   * @param  t_exceptions_on_unitialized true if exceptions must be thrown
+   *                                   when reading uninitialized variables
    **/
   KnowledgeUpdateSettings(bool t_treat_globals_as_locals,
       bool t_signal_changes = true, bool t_always_overwrite = false,
       bool t_always_expand = true, bool t_track_local_changes = true,
       uint64_t t_clock_increment = 1, bool t_treat_locals_as_globals = false,
-      bool t_stream_changes = true)
-    : KnowledgeReferenceSettings(t_always_expand),
+      bool t_stream_changes = true, bool t_exceptions_on_unitialized = false)
+    : KnowledgeReferenceSettings(t_always_expand, t_exceptions_on_unitialized),
       treat_globals_as_locals(t_treat_globals_as_locals),
       signal_changes(t_signal_changes),
       always_overwrite(t_always_overwrite),