Create PythonPassthrough exception of proper type in the shutdown_eng…

…ine method

Signed-off-by: Adam Glustein <Adam.Glustein@Point72.com>

cpp/csp/python/Exception.h

@@ -8,17 +8,6 @@
 namespace csp::python
 {
 
-class TracebackStringException : public std::exception
-{
-public:
-    TracebackStringException( const char * message ) : m_message( message ) {}
-
-    const char * what() const noexcept override { return m_message; }
-
-private:
-    const char * m_message;
-};
-
 class PythonPassthrough : public csp::Exception
 {
 public:
@@ -27,20 +16,21 @@ class PythonPassthrough : public csp::Exception
         csp::Exception( exType, r, file, func, line )
     {
         //Fetch the current error to clear out the error indicator while the stack gets unwound
+        //We own the references to all the members assigned in PyErr_Fetch
+        //We need to hold the reference since PyErr_Restore takes back a reference to each of its arguments
         PyErr_Fetch( &m_type, &m_value, &m_traceback );
     }
 
     PythonPassthrough( PyObject * pyException ) :
-        m_pyException( pyException ),
-        m_type( PyObject_Type( pyException ) ),
-        m_value( PyObject_Str( pyException ) ),
-        m_traceback( PyException_GetTraceback( pyException ) ),
-        csp::Exception( PyUnicode_AsUTF8( m_type ), std::string( PyUnicode_AsUTF8( m_value ) ) )
+        csp::Exception( "", "" )
     {
-        Py_INCREF( pyException );
+        // Note: all of these methods return strong references, so we own them like in the other constructor
+        m_type = PyObject_Type( pyException );
+        m_value = PyObject_Str( pyException );
+        m_traceback = PyException_GetTraceback( pyException );
     }
 
-    ~PythonPassthrough() { Py_DECREF( m_pyException ); }
+    ~PythonPassthrough() { }
 
     void restore()
     {
@@ -62,8 +52,6 @@ class PythonPassthrough : public csp::Exception
     PyObject * m_type;
     PyObject * m_value;
     PyObject * m_traceback;
-    PyObject * m_pyException;
-
 };
 
 CSP_DECLARE_EXCEPTION( AttributeError, ::csp::Exception );

cpp/csp/python/PyAdapterManager.cpp

@@ -6,7 +6,6 @@
 #include <csp/python/InitHelper.h>
 #include <csp/python/PyObjectPtr.h>
 #include <memory>
-#include<iostream>
 
 namespace csp::python
 {
@@ -69,19 +68,17 @@ class PyAdapterManager : public AdapterManager
 static PyObject * PyAdapterManager_PyObject_starttime( PyAdapterManager_PyObject * self ) { return toPython( self -> manager -> starttime() ); }
 static PyObject * PyAdapterManager_PyObject_endtime( PyAdapterManager_PyObject * self )   { return toPython( self -> manager -> endtime() ); }
 
-static PyObject * PyAdapterManager_PyObject__engine_shutdown( PyAdapterManager_PyObject * self, PyObject * args, PyObject * kwargs)
+static PyObject * PyAdapterManager_PyObject_shutdown_engine( PyAdapterManager_PyObject * self, PyObject * args, PyObject * kwargs )
 {
     CSP_BEGIN_METHOD;
 
-    PyObject * pyException  = nullptr;
-
+    PyObject * pyException;
     if( !PyArg_ParseTuple( args, "O", &pyException ) )
         return NULL;
-
-    PyObject * type = PyObject_Type( pyException );
-    PyObject * val = PyObject_Str( pyException );
-    PyObject * traceback = PyException_GetTraceback( pyException );
 
+    if( !PyExceptionInstance_Check( pyException ) )
+       CSP_THROW( TypeError, "Expected Exception object as argument for shutdown_engine: got " << Py_TYPE( pyException ) -> tp_name );
+
     self -> manager -> rootEngine() -> shutdown( std::make_exception_ptr( PythonPassthrough( pyException ) ) );
 
     CSP_RETURN_NONE;
@@ -104,7 +101,7 @@ static int PyAdapterManager_init( PyAdapterManager_PyObject *self, PyObject *arg
 static PyMethodDef PyAdapterManager_methods[] = {
     { "starttime",          (PyCFunction) PyAdapterManager_PyObject_starttime, METH_NOARGS, "starttime" },
     { "endtime",            (PyCFunction) PyAdapterManager_PyObject_endtime,    METH_NOARGS, "endtime" },
-    { "_engine_shutdown",   (PyCFunction) PyAdapterManager_PyObject__engine_shutdown,  METH_VARARGS, "_engine_shutdown" },
+    { "shutdown_engine",    (PyCFunction) PyAdapterManager_PyObject_shutdown_engine,  METH_VARARGS, "shutdown_engine" },
     {NULL}
 };
 

cpp/csp/python/PyEngine.h

@@ -61,11 +61,14 @@ PyObject * PyEngine_shutdown( T * self, PyObject * args, PyObject * kwargs )
 {
     CSP_BEGIN_METHOD;
 
-    const char * msg;
-    if( !PyArg_ParseTuple( args, "s", &msg ) )
+    PyObject * pyException;
+    if( !PyArg_ParseTuple( args, "O", &pyException ) )
         return NULL;
 
-    self -> adapter -> rootEngine() -> shutdown( std::make_exception_ptr( TracebackStringException( msg ) ) );
+    if( !PyExceptionInstance_Check( pyException ) )
+       CSP_THROW( TypeError, "Expected Exception object as argument for shutdown_engine: got " << Py_TYPE( pyException ) -> tp_name );
+
+    self -> adapter -> rootEngine() -> shutdown( std::make_exception_ptr( PythonPassthrough( pyException ) ) );
 
     CSP_RETURN_NONE;
 }

cpp/csp/python/PyPushInputAdapter.cpp

@@ -203,7 +203,7 @@ struct PyPushInputAdapter_PyObject
 
 static PyMethodDef PyPushInputAdapter_PyObject_methods[] = {
     { "push_tick",          (PyCFunction) PyPushInputAdapter_PyObject::pushTick, METH_VARARGS, "push new tick" },
-    { "_engine_shutdown",   (PyCFunction) PyEngine_shutdown<PyPushInputAdapter_PyObject>,  METH_VARARGS, "engine shutdown" },
+    { "shutdown_engine",    (PyCFunction) PyEngine_shutdown<PyPushInputAdapter_PyObject>,  METH_VARARGS, "shutdown_engine" },
     {NULL}
 };
 

cpp/csp/python/PyPushPullInputAdapter.cpp

@@ -125,7 +125,7 @@ struct PyPushPullInputAdapter_PyObject
 static PyMethodDef PyPushPullInputAdapter_PyObject_methods[] = {
     { "push_tick",              (PyCFunction) PyPushPullInputAdapter_PyObject::pushTick, METH_VARARGS, "push new tick" },
     { "flag_replay_complete",   (PyCFunction) PyPushPullInputAdapter_PyObject::flagReplayComplete, METH_VARARGS, "finish replay ticks" },
-    { "_engine_shutdown",       (PyCFunction) PyEngine_shutdown<PyPushPullInputAdapter_PyObject>,  METH_VARARGS, "engine shutdown" },
+    { "shutdown_engine",        (PyCFunction) PyEngine_shutdown<PyPushPullInputAdapter_PyObject>,  METH_VARARGS, "shutdown engine" },
     {NULL}
 };
 

csp/impl/adaptermanager.py

@@ -2,7 +2,6 @@
 
 import csp
 from csp.impl.__cspimpl import _cspimpl
-from csp.impl.error_handling import format_engine_shutdown_stack
 
 
 class AdapterManagerImpl(_cspimpl.PyAdapterManager):
@@ -25,9 +24,6 @@ def process_next_sim_timeslice(self, now):
         """
         return None
 
-    def engine_shutdown(self, msg):
-        self._engine_shutdown(Exception('test')) # temporary for testing purposes
-
 
 class ManagedSimInputAdapter(_cspimpl.PyManagedSimInputAdapter):
     def __init__(self, typ, field_map):

csp/impl/error_handling.py

@@ -1,6 +1,5 @@
 import ast
 import os
-import traceback
 
 import csp.impl
 from csp.impl.__cspimpl import _cspimpl
@@ -49,13 +48,3 @@ def set_print_full_exception_stack(new_value: bool):
     res = ExceptionContext.PRINT_EXCEPTION_FULL_STACK
     ExceptionContext.PRINT_EXCEPTION_FULL_STACK = new_value
     return res
-
-
-def format_engine_shutdown_stack(msg: str):
-    tb = traceback.format_stack()
-    tb = tb[:-2]  # remove traceback call and internal engine shutdown method
-    if len(tb) > 1:
-        tb = tb[-2:]  # only keep the current function and the caller (adapter manager)
-    tb.append(msg)  # add the user's error message
-    tb = "".join(tb)  # format into a single string
-    return tb

csp/impl/pushadapter.py

@@ -1,5 +1,4 @@
 from csp.impl.__cspimpl import _cspimpl
-from csp.impl.error_handling import format_engine_shutdown_stack
 
 PushGroup = _cspimpl.PushGroup
 PushBatch = _cspimpl.PushBatch
@@ -12,8 +11,5 @@ def start(self, starttime, endtime):
     def stop(self):
         pass
 
-    def engine_shutdown(self, msg):
-        self._engine_shutdown(format_engine_shutdown_stack(msg))
-
     # base class
     # def push_tick( self, value )

csp/impl/pushpulladapter.py

@@ -1,5 +1,4 @@
 from csp.impl.__cspimpl import _cspimpl
-from csp.impl.error_handling import format_engine_shutdown_stack
 
 PushGroup = _cspimpl.PushGroup
 PushBatch = _cspimpl.PushBatch
@@ -12,8 +11,5 @@ def start(self, starttime, endtime):
     def stop(self):
         pass
 
-    def engine_shutdown(self, msg):
-        self._engine_shutdown(format_engine_shutdown_stack(msg))
-
     # base class
     # def push_tick( self, time, value )

csp/tests/impl/test_pushadapter.py

@@ -261,7 +261,7 @@ def _run(self):
                 while self._running:
                     if pushed:
                         time.sleep(0.1)
-                        self.engine_shutdown("Dummy exception message")
+                        self.shutdown_engine(TypeError("Dummy exception message"))
                     else:
                         self.push_tick(0)
                         pushed = True
@@ -281,7 +281,7 @@ def graph():
             node(adapter)
             csp.print("adapter", adapter)
 
-        with self.assertRaisesRegex(Exception, "Dummy exception message"):
+        with self.assertRaisesRegex(TypeError, "Dummy exception message"):
             csp.run(graph, starttime=datetime.utcnow(), realtime=True)
         self.assertEqual(status["count"], 1)
 

csp/tests/impl/test_pushpulladapter.py

@@ -166,7 +166,7 @@ def _run(self):
                 while self._running and idx < len(self._data):
                     if idx and self._shutdown_before_live:
                         time.sleep(0.1)
-                        self.engine_shutdown("Dummy exception message")
+                        self.shutdown_engine(ValueError("Dummy exception message"))
                     t, v = self._data[idx]
                     self.push_tick(False, t, v)
                     idx += 1
@@ -177,7 +177,7 @@ def _run(self):
                     self.push_tick(True, datetime.utcnow(), len(self._data) + 1)
                     if idx and not self._shutdown_before_live:
                         time.sleep(0.1)
-                        self.engine_shutdown("Dummy exception message")
+                        self.shutdown_engine(TypeError("Dummy exception message"))
                     idx += 1
 
         MyPushPullAdapter = py_pushpull_adapter_def(
@@ -190,9 +190,9 @@ def graph(shutdown_before_live: bool):
             adapter = MyPushPullAdapter(int, data, shutdown_before_live)
             csp.print("adapter", adapter)
 
-        with self.assertRaisesRegex(Exception, "Dummy exception message"):
+        with self.assertRaisesRegex(ValueError, "Dummy exception message"):
             csp.run(graph, True, starttime=datetime(2020, 1, 1, 1))
-        with self.assertRaisesRegex(Exception, "Dummy exception message"):
+        with self.assertRaisesRegex(TypeError, "Dummy exception message"):
             csp.run(
                 graph,
                 False,

csp/tests/test_engine.py

@@ -825,7 +825,7 @@ def stop(self):
                 pass
 
             def process_next_sim_timeslice(self, now):
-                self.engine_shutdown("Dummy exception message")
+                self.shutdown_engine(ValueError("Dummy exception message"))
 
         class TestAdapterImpl(ManagedSimInputAdapter):
             def __init__(self, manager_impl):
@@ -838,7 +838,7 @@ def graph():
             nc = adapter.subscribe()
             csp.add_graph_output("nc", nc)
 
-        with self.assertRaisesRegex(Exception, "Dummy exception message"):
+        with self.assertRaisesRegex(ValueError, "Dummy exception message"):
             csp.run(graph, starttime=datetime(2020, 1, 1), endtime=timedelta(seconds=1))
 
     def test_feedback(self):