From 18631da824ad0cc3385467ea3612b8bdf72637f1 Mon Sep 17 00:00:00 2001 From: Adam Glustein Date: Thu, 11 Jul 2024 14:25:10 -0400 Subject: [PATCH] Create PythonPassthrough exception of proper type in the shutdown_engine method Signed-off-by: Adam Glustein --- cpp/csp/python/Exception.h | 28 +++++++---------------- cpp/csp/python/PyAdapterManager.cpp | 15 +++++------- cpp/csp/python/PyEngine.h | 9 +++++--- cpp/csp/python/PyPushInputAdapter.cpp | 2 +- cpp/csp/python/PyPushPullInputAdapter.cpp | 2 +- csp/impl/adaptermanager.py | 4 ---- csp/impl/error_handling.py | 11 --------- csp/impl/pushadapter.py | 4 ---- csp/impl/pushpulladapter.py | 4 ---- csp/tests/impl/test_pushadapter.py | 4 ++-- csp/tests/impl/test_pushpulladapter.py | 8 +++---- csp/tests/test_engine.py | 4 ++-- 12 files changed, 30 insertions(+), 65 deletions(-) diff --git a/cpp/csp/python/Exception.h b/cpp/csp/python/Exception.h index 6249d2cc..8fd97568 100644 --- a/cpp/csp/python/Exception.h +++ b/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 ); diff --git a/cpp/csp/python/PyAdapterManager.cpp b/cpp/csp/python/PyAdapterManager.cpp index 1ecd04b3..842483aa 100644 --- a/cpp/csp/python/PyAdapterManager.cpp +++ b/cpp/csp/python/PyAdapterManager.cpp @@ -6,7 +6,6 @@ #include #include #include -#include 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} }; diff --git a/cpp/csp/python/PyEngine.h b/cpp/csp/python/PyEngine.h index e059e81c..93179229 100644 --- a/cpp/csp/python/PyEngine.h +++ b/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; } diff --git a/cpp/csp/python/PyPushInputAdapter.cpp b/cpp/csp/python/PyPushInputAdapter.cpp index 013e78db..a9b01b44 100644 --- a/cpp/csp/python/PyPushInputAdapter.cpp +++ b/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, METH_VARARGS, "engine shutdown" }, + { "shutdown_engine", (PyCFunction) PyEngine_shutdown, METH_VARARGS, "shutdown_engine" }, {NULL} }; diff --git a/cpp/csp/python/PyPushPullInputAdapter.cpp b/cpp/csp/python/PyPushPullInputAdapter.cpp index 97199907..0f4ee5de 100644 --- a/cpp/csp/python/PyPushPullInputAdapter.cpp +++ b/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, METH_VARARGS, "engine shutdown" }, + { "shutdown_engine", (PyCFunction) PyEngine_shutdown, METH_VARARGS, "shutdown engine" }, {NULL} }; diff --git a/csp/impl/adaptermanager.py b/csp/impl/adaptermanager.py index 10c57bc2..9ec42d0e 100644 --- a/csp/impl/adaptermanager.py +++ b/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): diff --git a/csp/impl/error_handling.py b/csp/impl/error_handling.py index b4a0303b..e662b7eb 100644 --- a/csp/impl/error_handling.py +++ b/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 diff --git a/csp/impl/pushadapter.py b/csp/impl/pushadapter.py index e9fe48fc..b6435673 100644 --- a/csp/impl/pushadapter.py +++ b/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 ) diff --git a/csp/impl/pushpulladapter.py b/csp/impl/pushpulladapter.py index c92f8541..f2beccd3 100644 --- a/csp/impl/pushpulladapter.py +++ b/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 ) diff --git a/csp/tests/impl/test_pushadapter.py b/csp/tests/impl/test_pushadapter.py index b77f38f6..0ac9bf98 100644 --- a/csp/tests/impl/test_pushadapter.py +++ b/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) diff --git a/csp/tests/impl/test_pushpulladapter.py b/csp/tests/impl/test_pushpulladapter.py index 92557a2a..b52ce075 100644 --- a/csp/tests/impl/test_pushpulladapter.py +++ b/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, diff --git a/csp/tests/test_engine.py b/csp/tests/test_engine.py index 9b069833..88b3db9b 100644 --- a/csp/tests/test_engine.py +++ b/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):