replace C++ exit(1) statements with exception handling (#329)

* throw runtime_error, rm exit(1) in featuretype&calcfeatures

* rename type->input_type

* throw EfelAssertionError to avoid exit(-1)

* add test to trigger C++->Python AssertionError

* remove featurename.find(";") check

* typo in docstring

* add feature's name to the error message

* Revert "remove featurename.find(";") check"

This reverts commit 954613a.

efel/cppcore/CMakeLists.txt

@@ -40,6 +40,6 @@ add_library(efel SHARED ${FEATURESRCS})
 install(TARGETS efel LIBRARY DESTINATION lib)
 
 install(FILES efel.h cfeature.h FillFptrTable.h LibV1.h LibV2.h LibV3.h
-    LibV5.h mapoperations.h Utils.h DependencyTree.h eFELLogger.h
+    LibV5.h mapoperations.h Utils.h DependencyTree.h eFELLogger.h EfelExceptions.h
     types.h
     DESTINATION include)

efel/cppcore/EfelExceptions.h

@@ -0,0 +1,12 @@
+#ifndef EFEL_EXCEPTIONS_H
+#define EFEL_EXCEPTIONS_H
+
+#include <stdexcept>
+
+// Define the custom exception class
+class EfelAssertionError : public std::runtime_error {
+public:
+  explicit EfelAssertionError(const std::string& message) : std::runtime_error(message) {}
+};
+
+#endif // EFEL_EXCEPTIONS_H

efel/cppcore/Utils.h

@@ -26,6 +26,7 @@
 #include <numeric>
 #include <vector>
 #include <utility>
+#include "EfelExceptions.h"
 
 using std::vector;
 
@@ -85,8 +86,9 @@ inline void
 efel_assert(bool assertion, const char *message, const char *file, const int line)
 {
   if(!assertion){
-    printf("Assertion fired(%s:%d): %s\n", file, line, message);
-    exit(-1);
+    using std::string, std::to_string;
+    string errorMsg = "Assertion fired(" + string(file) + ":" + to_string(line) + "): " + string(message);
+    throw EfelAssertionError(errorMsg);
   }
 }
 

efel/cppcore/cfeature.cpp

@@ -374,14 +374,9 @@ int cFeature::calc_features(const string& name) {
   map<string, vector<featureStringPair> >::const_iterator lookup_it(
       fptrlookup.find(name));
   if (lookup_it == fptrlookup.end()) {
-    fprintf(stderr,
-            "\nFeature [ %s ] dependency file entry or pointer table entry is "
-            "missing. Exiting\n",
-            name.c_str());
-    fflush(stderr);
-    exit(1);
+    throw std::runtime_error("Feature dependency file entry or pointer table "
+                             "entry for '" + name + "' is missing.");
   }
-
   bool last_failed = false;
 
   for (vector<featureStringPair>::const_iterator pfptrstring =
@@ -581,17 +576,12 @@ double cFeature::getDistance(string strName, double mean, double std,
       }
   }
 
-  // check datatype of feature
   featureType = featuretype(strName);
-  if (featureType.empty()) {
-    printf("Error : Feature [%s] not found. Exiting..\n", strName.c_str());
-    exit(1);
-  }
 
   if (featureType == "int") {
     retVal = getFeature<int>(strName, feature_veci);
     intFlag = 1;
-  } else {
+  } else { // double
     retVal = getFeature<double>(strName, feature_vec);
     intFlag = 0;
   }
@@ -634,13 +624,14 @@ double cFeature::getDistance(string strName, double mean, double std,
 
 string cFeature::featuretype(string featurename) {
   int npos = featurename.find(";");
-  if (npos >= 0) {
+  if (npos != string::npos) {
     featurename = featurename.substr(0, npos);
   }
-  string type(featuretypes[featurename]);
-  if (type.empty()) {
-    GErrorStr += featurename + "missing in featuretypes map.\n";
-  }
+  if (featurename == "__test_efel_assertion__")  // for testing only
+    throw EfelAssertionError("Test efel assertion is successfully triggered.");
+  string type = featuretypes[featurename];
+  if (type != "int" && type != "double")
+    throw std::runtime_error("Unknown feature name: " + featurename);
   return type;
 }
 

efel/cppcore/cppcore.cpp

@@ -108,36 +108,48 @@ static void PyList_from_vectorstring(vector<string> input, PyObject* output) {
 }
 
 static PyObject*
-_getfeature(PyObject* self, PyObject* args, const string &type) {
+_getfeature(PyObject* self, PyObject* args, const string &input_type) {
   char* feature_name;
   PyObject* py_values;
 
   int return_value;
   if (!PyArg_ParseTuple(args, "sO!", &feature_name, &PyList_Type, &py_values)) {
+    PyErr_SetString(PyExc_TypeError, "Unexpected argument type provided.");
     return NULL;
   }
 
-  string feature_type = pFeature->featuretype(string(feature_name));
-
-  if (!type.empty() && feature_type != type){
-    PyErr_SetString(PyExc_TypeError, "Feature type does not match");
+  try {
+    string feature_type = pFeature->featuretype(string(feature_name));
+
+    if (!input_type.empty() && feature_type != input_type){  // when types do not match
+      PyErr_SetString(PyExc_TypeError, "Feature type does not match");
+      return NULL;
+    }
+
+    if (feature_type == "int") {
+      vector<int> values;
+      return_value = pFeature->getFeature<int>(string(feature_name), values);
+      PyList_from_vectorint(values, py_values);
+      return Py_BuildValue("i", return_value);
+    } else { // double
+      vector<double> values;
+      return_value = pFeature->getFeature<double>(string(feature_name), values);
+      PyList_from_vectordouble(values, py_values);
+      return Py_BuildValue("i", return_value);
+    }
+  }
+  catch(EfelAssertionError& e) {  // more specialised exception
+    PyErr_SetString(PyExc_AssertionError, e.what());
     return NULL;
   }
-
-  if (feature_type == "int") {
-    vector<int> values;
-    return_value = pFeature->getFeature<int>(string(feature_name), values);
-    PyList_from_vectorint(values, py_values);
-  } else if (feature_type == "double") {
-    vector<double> values;
-    return_value = pFeature->getFeature<double>(string(feature_name), values);
-    PyList_from_vectordouble(values, py_values);
-  } else {
-    PyErr_SetString(PyExc_TypeError, "Unknown feature name");
+  catch(const std::runtime_error& e) {
+    PyErr_SetString(PyExc_RuntimeError, e.what());
+    return NULL;
+  }
+  catch(...) {
+    PyErr_SetString(PyExc_RuntimeError, "Unknown error");
     return NULL;
   }
-
-  return Py_BuildValue("i", return_value);
 }
 
 

tests/test_basic.py

@@ -183,7 +183,7 @@ def test_nonexisting_feature():
     trace['stim_end'] = [75]
 
     pytest.raises(
-        TypeError,
+        RuntimeError,
         efel.getFeatureValues,
         [trace],
         ['nonexisting_feature'])

tests/test_cppcore.py

@@ -160,7 +160,7 @@ def test_getFeature_failure(self):  # pylint: disable=R0201
         return_value = efel.cppcore.getFeature("AP_amplitude", feature_values)
         assert return_value == -1
 
-    @pytest.mark.xfail(raises=TypeError)
+    @pytest.mark.xfail(raises=RuntimeError)
     def test_getFeature_non_existant(self):  # pylint: disable=R0201
         """cppcore: Testing failure exit code in getFeature"""
         import efel
@@ -209,3 +209,15 @@ def test_caching(self, feature_name):
         assert contents.count(f"Calculated feature {feature_name}") == 1
         # make sure Reusing computed value of text occurs twice
         assert contents.count(f"Reusing computed value of {feature_name}") == 2
+
+
+def test_efel_assertion_error():
+    """Testing if C++ assertion error is propagated to Python correctly."""
+    import efel
+    efel.reset()
+    trace = {
+        "stim_start": [25],
+        "stim_end": [75],
+    }
+    with pytest.raises(AssertionError):
+        efel.getFeatureValues([trace], ["__test_efel_assertion__"])