From ea1e8a46a1ba1547474724f5bcd02343ad624853 Mon Sep 17 00:00:00 2001 From: Jose Date: Wed, 31 Jul 2024 20:36:13 +0200 Subject: [PATCH 1/5] PyException cleanup --- .../modules/IcePy/BatchRequestInterceptor.cpp | 2 +- python/modules/IcePy/Connection.cpp | 2 +- python/modules/IcePy/Dispatcher.cpp | 2 +- python/modules/IcePy/Logger.cpp | 12 +- python/modules/IcePy/ObjectAdapter.cpp | 8 +- python/modules/IcePy/Operation.cpp | 40 +-- python/modules/IcePy/Thread.cpp | 4 +- python/modules/IcePy/Util.cpp | 284 ++++++++---------- python/modules/IcePy/Util.h | 7 +- 9 files changed, 161 insertions(+), 200 deletions(-) diff --git a/python/modules/IcePy/BatchRequestInterceptor.cpp b/python/modules/IcePy/BatchRequestInterceptor.cpp index 0e38d6db93b..dedf5547d74 100644 --- a/python/modules/IcePy/BatchRequestInterceptor.cpp +++ b/python/modules/IcePy/BatchRequestInterceptor.cpp @@ -242,6 +242,6 @@ IcePy::BatchRequestInterceptorWrapper::enqueue(const Ice::BatchRequest& request, Py_DECREF(reinterpret_cast(obj)); if (!tmp.get()) { - throwPythonException(); + throwPythonException(true); } } diff --git a/python/modules/IcePy/Connection.cpp b/python/modules/IcePy/Connection.cpp index 71d854fd2fc..1d052cd5f25 100644 --- a/python/modules/IcePy/Connection.cpp +++ b/python/modules/IcePy/Connection.cpp @@ -102,7 +102,7 @@ namespace IcePy // ex.checkSystemExit(); - ex.raise(); + ex.raise(true); } } diff --git a/python/modules/IcePy/Dispatcher.cpp b/python/modules/IcePy/Dispatcher.cpp index b74f3010144..769dc27cac0 100644 --- a/python/modules/IcePy/Dispatcher.cpp +++ b/python/modules/IcePy/Dispatcher.cpp @@ -142,6 +142,6 @@ IcePy::Dispatcher::dispatch(function call, const Ice::ConnectionPtr& con Py_DECREF(reinterpret_cast(obj)); if (!tmp.get()) { - throwPythonException(); + throwPythonException(false); } } diff --git a/python/modules/IcePy/Logger.cpp b/python/modules/IcePy/Logger.cpp index abf66f2df8d..1d59dc7a6d5 100644 --- a/python/modules/IcePy/Logger.cpp +++ b/python/modules/IcePy/Logger.cpp @@ -32,7 +32,7 @@ IcePy::LoggerWrapper::print(const string& message) PyObjectHandle tmp = PyObject_CallMethod(_logger.get(), "_print", "s", message.c_str()); if (!tmp.get()) { - throwPythonException(); + throwPythonException(false); } } @@ -44,7 +44,7 @@ IcePy::LoggerWrapper::trace(const string& category, const string& message) PyObjectHandle tmp = PyObject_CallMethod(_logger.get(), "trace", "ss", category.c_str(), message.c_str()); if (!tmp.get()) { - throwPythonException(); + throwPythonException(false); } } @@ -56,7 +56,7 @@ IcePy::LoggerWrapper::warning(const string& message) PyObjectHandle tmp = PyObject_CallMethod(_logger.get(), "warning", "s", message.c_str()); if (!tmp.get()) { - throwPythonException(); + throwPythonException(false); } } @@ -68,7 +68,7 @@ IcePy::LoggerWrapper::error(const string& message) PyObjectHandle tmp = PyObject_CallMethod(_logger.get(), "error", "s", message.c_str()); if (!tmp.get()) { - throwPythonException(); + throwPythonException(false); } } @@ -80,7 +80,7 @@ IcePy::LoggerWrapper::getPrefix() PyObjectHandle tmp = PyObject_CallMethod(_logger.get(), "getPrefix", 0); if (!tmp.get()) { - throwPythonException(); + throwPythonException(false); } return getString(tmp.get()); } @@ -93,7 +93,7 @@ IcePy::LoggerWrapper::cloneWithPrefix(const string& prefix) PyObjectHandle tmp = PyObject_CallMethod(_logger.get(), "cloneWithPrefix", "s", prefix.c_str()); if (!tmp.get()) { - throwPythonException(); + throwPythonException(false); } return make_shared(tmp.get()); diff --git a/python/modules/IcePy/ObjectAdapter.cpp b/python/modules/IcePy/ObjectAdapter.cpp index db43c345a2e..45ffe848142 100644 --- a/python/modules/IcePy/ObjectAdapter.cpp +++ b/python/modules/IcePy/ObjectAdapter.cpp @@ -129,7 +129,7 @@ IcePy::ServantLocatorWrapper::locate(const Ice::Current& current, shared_ptrcurrent = createCurrent(current); if (!c->current) { - throwPythonException(); + throwPythonException(false); } // @@ -156,7 +156,7 @@ IcePy::ServantLocatorWrapper::locate(const Ice::Current& current, shared_ptrop; } - void handleException() + void handleException(bool includeStackTrace) { assert(PyErr_Occurred()); @@ -320,7 +320,7 @@ namespace // ex.checkSystemExit(); - ex.raise(); + ex.raise(includeStackTrace); } } @@ -859,7 +859,7 @@ Operation::marshalResult(Ice::OutputStream& os, PyObject* result) { try { - PyException().raise(); + throwPythonException(false); } catch (const Ice::UnknownException& ex) { @@ -879,7 +879,7 @@ Operation::marshalResult(Ice::OutputStream& os, PyObject* result) { try { - PyException().raise(); + throwPythonException(false); } catch (const Ice::UnknownException& ex) { @@ -1857,7 +1857,7 @@ IcePy::AsyncInvocation::response(bool ok, pair results handleResponse(future.get(), ok, results); if (PyErr_Occurred()) { - handleException(); + handleException(true); } } @@ -1886,7 +1886,7 @@ IcePy::AsyncInvocation::exception(std::exception_ptr ex) PyObjectHandle tmp = callMethod(future.get(), "set_exception", exh.get()); if (PyErr_Occurred()) { - handleException(); + handleException(true); } } @@ -1924,7 +1924,7 @@ IcePy::AsyncInvocation::sent(bool sentSynchronously) PyObjectHandle tmp = callMethod(future.get(), "set_sent", sentSynchronously ? Py_True : Py_False); if (PyErr_Occurred()) { - handleException(); + handleException(true); } if (!_twoway) @@ -1935,7 +1935,7 @@ IcePy::AsyncInvocation::sent(bool sentSynchronously) tmp = callMethod(future.get(), "set_result", Py_None); if (PyErr_Occurred()) { - handleException(); + handleException(true); } } } @@ -2117,7 +2117,7 @@ IcePy::SyncBlobjectInvocation::invoke(PyObject* args, PyObject* /* kwds */) PyObjectHandle result = PyTuple_New(2); if (!result.get()) { - throwPythonException(); + throwPythonException(true); } PyTuple_SET_ITEM(result.get(), 0, ok ? Py_True : Py_False); @@ -2133,7 +2133,7 @@ IcePy::SyncBlobjectInvocation::invoke(PyObject* args, PyObject* /* kwds */) } if (!op.get()) { - throwPythonException(); + throwPythonException(true); } PyTuple_SET_ITEM(result.get(), 1, op.release()); // PyTuple_SET_ITEM steals a reference. @@ -2271,13 +2271,13 @@ Upcall::dispatchImpl(PyObject* servant, const string& dispatchName, PyObject* ar PyObjectHandle dispatchArgs = PyTuple_New(3); if (!dispatchArgs.get()) { - throwPythonException(); + throwPythonException(false); } DispatchCallbackObject* callback = dispatchCallbackNew(&DispatchCallbackType, 0, 0); if (!callback) { - throwPythonException(); + throwPythonException(false); } callback->upcall = new UpcallPtr(shared_from_this()); PyTuple_SET_ITEM(dispatchArgs.get(), 0, reinterpret_cast(callback)); // Steals a reference. @@ -2329,7 +2329,7 @@ IcePy::TypedUpcall::dispatch(PyObject* servant, pair i PyObjectHandle args = PyTuple_New(count); if (!args.get()) { - throwPythonException(); + throwPythonException(false); } if (!_op->inParams.empty()) @@ -2391,7 +2391,7 @@ IcePy::TypedUpcall::dispatch(PyObject* servant, pair i } catch (const AbortMarshaling&) { - throwPythonException(); + throwPythonException(false); } } @@ -2431,7 +2431,7 @@ IcePy::TypedUpcall::response(PyObject* result) { try { - throwPythonException(); + throwPythonException(false); } catch (...) { @@ -2483,12 +2483,12 @@ IcePy::TypedUpcall::exception(PyException& ex) } else { - ex.raise(); + ex.raise(false); } } catch (const AbortMarshaling&) { - throwPythonException(); + throwPythonException(false); } } catch (...) @@ -2520,7 +2520,7 @@ IcePy::BlobjectUpcall::dispatch(PyObject* servant, pair(ex->ob_type); + PyObjectHandle name = getAttr(cls, "__name__", false); + assert(name.get()); + PyObjectHandle mod = getAttr(cls, "__module__", false); + assert(mod.get()); + string result = getString(mod.get()); + result += "."; + result += getString(name.get()); + return result; + } - if (PyObject_IsInstance(ex.get(), userExceptionType)) + string getTraceback(PyObject* type, PyObject* ex, PyObject* tb) { - string tb = getTraceback(); - if (!tb.empty()) - { - throw Ice::UnknownUserException{__FILE__, __LINE__, tb}; - } - else + ostringstream os; + assert(tb); + // + // We need the equivalent of the following Python code: + // + // import traceback + // list = traceback.format_exception(type, ex, tb) + // + PyObjectHandle str = createString("traceback"); + PyObjectHandle mod = PyImport_Import(str.get()); + assert(mod.get()); // Unable to import traceback module - Python installation error? + PyObject* func = PyDict_GetItemString(PyModule_GetDict(mod.get()), "format_exception"); + assert(func); // traceback.format_exception must be present. + PyObjectHandle args = Py_BuildValue("(OOO)", type, ex, tb); + assert(args.get()); + PyObjectHandle list = PyObject_CallObject(func, args.get()); + assert(list.get()); + + for (Py_ssize_t i = 0; i < PyList_GET_SIZE(list.get()); ++i) { - PyObjectHandle name = PyObject_CallMethod(ex.get(), "ice_id", 0); - PyErr_Clear(); - if (!name.get()) - { - throw Ice::UnknownUserException{__FILE__, __LINE__, getTypeName()}; - } - else - { - throw Ice::UnknownUserException{__FILE__, __LINE__, getString(name.get())}; - } + os << getString(PyList_GetItem(list.get(), i)); } + return os.str(); } - else if (PyObject_IsInstance(ex.get(), localExceptionType)) - { - raiseLocalException(); - } - else + + string createUnknownExceptionMessage(PyObject* ex, PyObject* type, PyObject* tb, bool includeStackTrace) { - string tb = getTraceback(); - if (!tb.empty()) + string traceback; + if (includeStackTrace) { - throw Ice::UnknownException{__FILE__, __LINE__, tb}; + traceback = getTraceback(type, ex, tb); } - else + + if (traceback.empty()) { ostringstream ostr; - - ostr << getTypeName(); - - IcePy::PyObjectHandle msg = PyObject_Str(ex.get()); - if (msg.get()) + ostr << getTypeName(ex); + IcePy::PyObjectHandle exStr = PyObject_Str(ex); + if (exStr.get() && IcePy::checkString(exStr.get())) { - string s = getString(msg.get()); - if (!s.empty()) + string message = IcePy::getString(exStr.get()); + if (!message.empty()) { - ostr << ": " << s; + ostr << ": " << message; } } - - throw Ice::UnknownException{__FILE__, __LINE__, ostr.str()}; + return ostr.str(); + } + else + { + return traceback; } } } void -IcePy::PyException::checkSystemExit() +IcePy::PyException::raise(bool includeStackTrace) { - if (PyObject_IsInstance(ex.get(), PyExc_SystemExit)) + assert(ex.get()); + PyObject* localExceptionType = lookupType("Ice.LocalException"); + if (PyObject_IsInstance(ex.get(), localExceptionType)) { - handleSystemExit(ex.get()); // Does not return. - } -} + string typeName = getTypeName(ex.get()); -void -IcePy::PyException::raiseLocalException() -{ - string typeName = getTypeName(); + PyObject* requestFailedExceptionType = lookupType("Ice.RequestFailedException"); + + if (PyObject_IsInstance(ex.get(), requestFailedExceptionType)) + { + PyObjectHandle idAttr = getAttr(ex.get(), "id", false); + Ice::Identity id; + if (idAttr.get()) + { + IcePy::getIdentity(idAttr.get(), id); + } + PyObjectHandle facetAttr = getAttr(ex.get(), "facet", false); + string facet = getString(facetAttr.get()); + PyObjectHandle operationAttr = getAttr(ex.get(), "operation", false); + string operation = getString(operationAttr.get()); - PyObject* requestFailedExceptionType = lookupType("Ice.RequestFailedException"); + if (typeName == "Ice.ObjectNotExistException") + { + throw Ice::ObjectNotExistException( + __FILE__, + __LINE__, + std::move(id), + std::move(facet), + std::move(operation)); + } + else if (typeName == "Ice.OperationNotExistException") + { + throw Ice::OperationNotExistException( + __FILE__, + __LINE__, + std::move(id), + std::move(facet), + std::move(operation)); + } + else if (typeName == "Ice.FacetNotExistException") + { + throw Ice::FacetNotExistException( + __FILE__, + __LINE__, + std::move(id), + std::move(facet), + std::move(operation)); + } + } - if (PyObject_IsInstance(ex.get(), requestFailedExceptionType)) - { - PyObjectHandle idAttr = getAttr(ex.get(), "id", false); - Ice::Identity id; - if (idAttr.get()) + IcePy::PyObjectHandle exStr = PyObject_Str(ex.get()); + string message; + if (exStr.get() && checkString(exStr.get())) { - IcePy::getIdentity(idAttr.get(), id); + message = getString(exStr.get()); } - PyObjectHandle facetAttr = getAttr(ex.get(), "facet", false); - string facet = getString(facetAttr.get()); - PyObjectHandle operationAttr = getAttr(ex.get(), "operation", false); - string operation = getString(operationAttr.get()); - if (typeName == "Ice.ObjectNotExistException") + if (typeName == "Ice.UnknownLocalException") { - throw Ice::ObjectNotExistException( - __FILE__, - __LINE__, - std::move(id), - std::move(facet), - std::move(operation)); + throw Ice::UnknownLocalException{__FILE__, __LINE__, std::move(message)}; } - else if (typeName == "Ice.OperationNotExistException") + else if (typeName == "Ice.UnknownUserException") { - throw Ice::OperationNotExistException( - __FILE__, - __LINE__, - std::move(id), - std::move(facet), - std::move(operation)); + throw Ice::UnknownUserException{__FILE__, __LINE__, std::move(message)}; } - else if (typeName == "Ice.FacetNotExistException") + else if (typeName == "Ice.UnknownException") { - throw Ice::FacetNotExistException( - __FILE__, - __LINE__, - std::move(id), - std::move(facet), - std::move(operation)); + throw Ice::UnknownException{__FILE__, __LINE__, std::move(message)}; } - } - - IcePy::PyObjectHandle exStr = PyObject_Str(ex.get()); - string message; - if (exStr.get() && checkString(exStr.get())) - { - message = getString(exStr.get()); - } - - if (typeName == "Ice.UnknownLocalException") - { - throw Ice::UnknownLocalException{__FILE__, __LINE__, std::move(message)}; - } - else if (typeName == "Ice.UnknownUserException") - { - throw Ice::UnknownUserException{__FILE__, __LINE__, std::move(message)}; - } - else if (typeName == "Ice.UnknownException") - { - throw Ice::UnknownException{__FILE__, __LINE__, std::move(message)}; - } - - string tb = getTraceback(); - if (!tb.empty()) - { - throw Ice::UnknownLocalException{__FILE__, __LINE__, tb}; + throw Ice::UnknownLocalException{ + __FILE__, + __LINE__, + createUnknownExceptionMessage(ex.get(), _type.get(), _tb.get(), includeStackTrace)}; } else { - throw Ice::UnknownLocalException{__FILE__, __LINE__, typeName}; + throw Ice::UnknownException{ + __FILE__, + __LINE__, + createUnknownExceptionMessage(ex.get(), _type.get(), _tb.get(), includeStackTrace)}; } } -string -IcePy::PyException::getTraceback() +void +IcePy::PyException::checkSystemExit() { - if (!_tb.get()) - { - return string(); - } - - // - // We need the equivalent of the following Python code: - // - // import traceback - // list = traceback.format_exception(type, ex, tb) - // - PyObjectHandle str = createString("traceback"); - PyObjectHandle mod = PyImport_Import(str.get()); - assert(mod.get()); // Unable to import traceback module - Python installation error? - PyObject* func = PyDict_GetItemString(PyModule_GetDict(mod.get()), "format_exception"); - assert(func); // traceback.format_exception must be present. - PyObjectHandle args = Py_BuildValue("(OOO)", _type.get(), ex.get(), _tb.get()); - assert(args.get()); - PyObjectHandle list = PyObject_CallObject(func, args.get()); - assert(list.get()); - - string result; - for (Py_ssize_t i = 0; i < PyList_GET_SIZE(list.get()); ++i) + if (PyObject_IsInstance(ex.get(), PyExc_SystemExit)) { - string s = getString(PyList_GetItem(list.get(), i)); - result += s; + handleSystemExit(ex.get()); // Does not return. } - - return result; -} - -string -IcePy::PyException::getTypeName() -{ - PyObject* cls = reinterpret_cast(ex.get()->ob_type); - PyObjectHandle name = getAttr(cls, "__name__", false); - assert(name.get()); - PyObjectHandle mod = getAttr(cls, "__module__", false); - assert(mod.get()); - string result = getString(mod.get()); - result += "."; - result += getString(name.get()); - return result; } PyObject* @@ -767,7 +734,6 @@ IcePy::convertException(std::exception_ptr exPtr) IcePy::createString(ex.kindOfObject()), IcePy::createString(ex.id()), IcePy::createString(ex.what())}; - return createPythonException(ex.ice_id(), std::move(args)); } catch (const Ice::NotRegisteredException& ex) @@ -776,7 +742,6 @@ IcePy::convertException(std::exception_ptr exPtr) IcePy::createString(ex.kindOfObject()), IcePy::createString(ex.id()), IcePy::createString(ex.what())}; - return createPythonException(ex.ice_id(), std::move(args)); } catch (const Ice::ConnectionAbortedException& ex) @@ -796,7 +761,6 @@ IcePy::convertException(std::exception_ptr exPtr) IcePy::createString(ex.facet()), IcePy::createString(ex.operation()), IcePy::createString(ex.what())}; - return createPythonException(ex.ice_id(), std::move(args)); } // Then all other exceptions. @@ -838,10 +802,10 @@ IcePy::setPythonException(PyObject* ex) } void -IcePy::throwPythonException() +IcePy::throwPythonException(bool includeStackTrace) { PyException ex; - ex.raise(); + ex.raise(includeStackTrace); } void diff --git a/python/modules/IcePy/Util.h b/python/modules/IcePy/Util.h index 60fd7ead059..e680c95651b 100644 --- a/python/modules/IcePy/Util.h +++ b/python/modules/IcePy/Util.h @@ -89,7 +89,7 @@ namespace IcePy // // Convert the Python exception to its C++ equivalent. // - void raise(); + void raise(bool includeStackTrace); // // If the Python exception is SystemExit, act on it. May not return. @@ -99,9 +99,6 @@ namespace IcePy PyObjectHandle ex; private: - void raiseLocalException(); - std::string getTraceback(); - std::string getTypeName(); PyObjectHandle _type; PyObjectHandle _tb; @@ -159,7 +156,7 @@ namespace IcePy // Converts the interpreter's current exception into an Ice exception // and throws it. // - void throwPythonException(); + void throwPythonException(bool includeStackTrace); // // Handle the SystemExit exception. From 2fd572b454c985f6847cf63259e400dea6300880 Mon Sep 17 00:00:00 2001 From: Jose Date: Thu, 1 Aug 2024 18:07:57 +0200 Subject: [PATCH 2/5] Remove includeStackTrace param --- .../modules/IcePy/BatchRequestInterceptor.cpp | 2 +- python/modules/IcePy/Connection.cpp | 2 +- python/modules/IcePy/Dispatcher.cpp | 2 +- python/modules/IcePy/Logger.cpp | 14 ++-- python/modules/IcePy/ObjectAdapter.cpp | 8 +- python/modules/IcePy/Operation.cpp | 40 ++++----- python/modules/IcePy/Thread.cpp | 4 +- python/modules/IcePy/Util.cpp | 81 ++++--------------- python/modules/IcePy/Util.h | 9 +-- 9 files changed, 52 insertions(+), 110 deletions(-) diff --git a/python/modules/IcePy/BatchRequestInterceptor.cpp b/python/modules/IcePy/BatchRequestInterceptor.cpp index dedf5547d74..0e38d6db93b 100644 --- a/python/modules/IcePy/BatchRequestInterceptor.cpp +++ b/python/modules/IcePy/BatchRequestInterceptor.cpp @@ -242,6 +242,6 @@ IcePy::BatchRequestInterceptorWrapper::enqueue(const Ice::BatchRequest& request, Py_DECREF(reinterpret_cast(obj)); if (!tmp.get()) { - throwPythonException(true); + throwPythonException(); } } diff --git a/python/modules/IcePy/Connection.cpp b/python/modules/IcePy/Connection.cpp index 1d052cd5f25..71d854fd2fc 100644 --- a/python/modules/IcePy/Connection.cpp +++ b/python/modules/IcePy/Connection.cpp @@ -102,7 +102,7 @@ namespace IcePy // ex.checkSystemExit(); - ex.raise(true); + ex.raise(); } } diff --git a/python/modules/IcePy/Dispatcher.cpp b/python/modules/IcePy/Dispatcher.cpp index 769dc27cac0..b74f3010144 100644 --- a/python/modules/IcePy/Dispatcher.cpp +++ b/python/modules/IcePy/Dispatcher.cpp @@ -142,6 +142,6 @@ IcePy::Dispatcher::dispatch(function call, const Ice::ConnectionPtr& con Py_DECREF(reinterpret_cast(obj)); if (!tmp.get()) { - throwPythonException(false); + throwPythonException(); } } diff --git a/python/modules/IcePy/Logger.cpp b/python/modules/IcePy/Logger.cpp index 1d59dc7a6d5..d5e1f715f67 100644 --- a/python/modules/IcePy/Logger.cpp +++ b/python/modules/IcePy/Logger.cpp @@ -32,7 +32,7 @@ IcePy::LoggerWrapper::print(const string& message) PyObjectHandle tmp = PyObject_CallMethod(_logger.get(), "_print", "s", message.c_str()); if (!tmp.get()) { - throwPythonException(false); + throwPythonException(); } } @@ -44,7 +44,7 @@ IcePy::LoggerWrapper::trace(const string& category, const string& message) PyObjectHandle tmp = PyObject_CallMethod(_logger.get(), "trace", "ss", category.c_str(), message.c_str()); if (!tmp.get()) { - throwPythonException(false); + throwPythonException(); } } @@ -56,7 +56,7 @@ IcePy::LoggerWrapper::warning(const string& message) PyObjectHandle tmp = PyObject_CallMethod(_logger.get(), "warning", "s", message.c_str()); if (!tmp.get()) { - throwPythonException(false); + throwPythonException(); } } @@ -68,7 +68,7 @@ IcePy::LoggerWrapper::error(const string& message) PyObjectHandle tmp = PyObject_CallMethod(_logger.get(), "error", "s", message.c_str()); if (!tmp.get()) { - throwPythonException(false); + throwPythonException(); } } @@ -80,7 +80,7 @@ IcePy::LoggerWrapper::getPrefix() PyObjectHandle tmp = PyObject_CallMethod(_logger.get(), "getPrefix", 0); if (!tmp.get()) { - throwPythonException(false); + throwPythonException(); } return getString(tmp.get()); } @@ -93,7 +93,7 @@ IcePy::LoggerWrapper::cloneWithPrefix(const string& prefix) PyObjectHandle tmp = PyObject_CallMethod(_logger.get(), "cloneWithPrefix", "s", prefix.c_str()); if (!tmp.get()) { - throwPythonException(false); + throwPythonException(); } return make_shared(tmp.get()); @@ -113,7 +113,7 @@ loggerNew(PyTypeObject* type, PyObject* /*args*/, PyObject* /*kwds*/) { return nullptr; } - self->logger = 0; + self->logger = nullptr; return self; } diff --git a/python/modules/IcePy/ObjectAdapter.cpp b/python/modules/IcePy/ObjectAdapter.cpp index 45ffe848142..db43c345a2e 100644 --- a/python/modules/IcePy/ObjectAdapter.cpp +++ b/python/modules/IcePy/ObjectAdapter.cpp @@ -129,7 +129,7 @@ IcePy::ServantLocatorWrapper::locate(const Ice::Current& current, shared_ptrcurrent = createCurrent(current); if (!c->current) { - throwPythonException(false); + throwPythonException(); } // @@ -156,7 +156,7 @@ IcePy::ServantLocatorWrapper::locate(const Ice::Current& current, shared_ptrop; } - void handleException(bool includeStackTrace) + void handleException() { assert(PyErr_Occurred()); @@ -320,7 +320,7 @@ namespace // ex.checkSystemExit(); - ex.raise(includeStackTrace); + ex.raise(); } } @@ -859,7 +859,7 @@ Operation::marshalResult(Ice::OutputStream& os, PyObject* result) { try { - throwPythonException(false); + throwPythonException(); } catch (const Ice::UnknownException& ex) { @@ -879,7 +879,7 @@ Operation::marshalResult(Ice::OutputStream& os, PyObject* result) { try { - throwPythonException(false); + throwPythonException(); } catch (const Ice::UnknownException& ex) { @@ -1857,7 +1857,7 @@ IcePy::AsyncInvocation::response(bool ok, pair results handleResponse(future.get(), ok, results); if (PyErr_Occurred()) { - handleException(true); + handleException(); } } @@ -1886,7 +1886,7 @@ IcePy::AsyncInvocation::exception(std::exception_ptr ex) PyObjectHandle tmp = callMethod(future.get(), "set_exception", exh.get()); if (PyErr_Occurred()) { - handleException(true); + handleException(); } } @@ -1924,7 +1924,7 @@ IcePy::AsyncInvocation::sent(bool sentSynchronously) PyObjectHandle tmp = callMethod(future.get(), "set_sent", sentSynchronously ? Py_True : Py_False); if (PyErr_Occurred()) { - handleException(true); + handleException(); } if (!_twoway) @@ -1935,7 +1935,7 @@ IcePy::AsyncInvocation::sent(bool sentSynchronously) tmp = callMethod(future.get(), "set_result", Py_None); if (PyErr_Occurred()) { - handleException(true); + handleException(); } } } @@ -2117,7 +2117,7 @@ IcePy::SyncBlobjectInvocation::invoke(PyObject* args, PyObject* /* kwds */) PyObjectHandle result = PyTuple_New(2); if (!result.get()) { - throwPythonException(true); + throwPythonException(); } PyTuple_SET_ITEM(result.get(), 0, ok ? Py_True : Py_False); @@ -2133,7 +2133,7 @@ IcePy::SyncBlobjectInvocation::invoke(PyObject* args, PyObject* /* kwds */) } if (!op.get()) { - throwPythonException(true); + throwPythonException(); } PyTuple_SET_ITEM(result.get(), 1, op.release()); // PyTuple_SET_ITEM steals a reference. @@ -2271,13 +2271,13 @@ Upcall::dispatchImpl(PyObject* servant, const string& dispatchName, PyObject* ar PyObjectHandle dispatchArgs = PyTuple_New(3); if (!dispatchArgs.get()) { - throwPythonException(false); + throwPythonException(); } DispatchCallbackObject* callback = dispatchCallbackNew(&DispatchCallbackType, 0, 0); if (!callback) { - throwPythonException(false); + throwPythonException(); } callback->upcall = new UpcallPtr(shared_from_this()); PyTuple_SET_ITEM(dispatchArgs.get(), 0, reinterpret_cast(callback)); // Steals a reference. @@ -2329,7 +2329,7 @@ IcePy::TypedUpcall::dispatch(PyObject* servant, pair i PyObjectHandle args = PyTuple_New(count); if (!args.get()) { - throwPythonException(false); + throwPythonException(); } if (!_op->inParams.empty()) @@ -2391,7 +2391,7 @@ IcePy::TypedUpcall::dispatch(PyObject* servant, pair i } catch (const AbortMarshaling&) { - throwPythonException(false); + throwPythonException(); } } @@ -2431,7 +2431,7 @@ IcePy::TypedUpcall::response(PyObject* result) { try { - throwPythonException(false); + throwPythonException(); } catch (...) { @@ -2483,12 +2483,12 @@ IcePy::TypedUpcall::exception(PyException& ex) } else { - ex.raise(false); + ex.raise(); } } catch (const AbortMarshaling&) { - throwPythonException(false); + throwPythonException(); } } catch (...) @@ -2520,7 +2520,7 @@ IcePy::BlobjectUpcall::dispatch(PyObject* servant, pair(Py_TYPE(ex.get())); - Py_INCREF(type); - _type = type; - _tb = PyException_GetTraceback(ex.get()); - } } IcePy::PyException::PyException(PyObject* raisedException) { Py_XINCREF(raisedException); - ex = raisedException; - PyObject* type = reinterpret_cast(Py_TYPE(raisedException)); - Py_INCREF(type); - _type = type; - _tb = PyException_GetTraceback(raisedException); } namespace IcePy @@ -305,66 +293,25 @@ namespace IcePy result += getString(name.get()); return result; } - - string getTraceback(PyObject* type, PyObject* ex, PyObject* tb) + string createUnknownExceptionMessage(PyObject* ex) { - ostringstream os; - assert(tb); - // - // We need the equivalent of the following Python code: - // - // import traceback - // list = traceback.format_exception(type, ex, tb) - // - PyObjectHandle str = createString("traceback"); - PyObjectHandle mod = PyImport_Import(str.get()); - assert(mod.get()); // Unable to import traceback module - Python installation error? - PyObject* func = PyDict_GetItemString(PyModule_GetDict(mod.get()), "format_exception"); - assert(func); // traceback.format_exception must be present. - PyObjectHandle args = Py_BuildValue("(OOO)", type, ex, tb); - assert(args.get()); - PyObjectHandle list = PyObject_CallObject(func, args.get()); - assert(list.get()); - - for (Py_ssize_t i = 0; i < PyList_GET_SIZE(list.get()); ++i) + ostringstream ostr; + ostr << getTypeName(ex); + IcePy::PyObjectHandle exStr = PyObject_Str(ex); + if (exStr.get() && IcePy::checkString(exStr.get())) { - os << getString(PyList_GetItem(list.get(), i)); - } - return os.str(); - } - - string createUnknownExceptionMessage(PyObject* ex, PyObject* type, PyObject* tb, bool includeStackTrace) - { - string traceback; - if (includeStackTrace) - { - traceback = getTraceback(type, ex, tb); - } - - if (traceback.empty()) - { - ostringstream ostr; - ostr << getTypeName(ex); - IcePy::PyObjectHandle exStr = PyObject_Str(ex); - if (exStr.get() && IcePy::checkString(exStr.get())) + string message = IcePy::getString(exStr.get()); + if (!message.empty()) { - string message = IcePy::getString(exStr.get()); - if (!message.empty()) - { - ostr << ": " << message; - } + ostr << ": " << message; } - return ostr.str(); - } - else - { - return traceback; } + return ostr.str(); } } void -IcePy::PyException::raise(bool includeStackTrace) +IcePy::PyException::raise() { assert(ex.get()); PyObject* localExceptionType = lookupType("Ice.LocalException"); @@ -438,14 +385,14 @@ IcePy::PyException::raise(bool includeStackTrace) throw Ice::UnknownLocalException{ __FILE__, __LINE__, - createUnknownExceptionMessage(ex.get(), _type.get(), _tb.get(), includeStackTrace)}; + createUnknownExceptionMessage(ex.get())}; } else { throw Ice::UnknownException{ __FILE__, __LINE__, - createUnknownExceptionMessage(ex.get(), _type.get(), _tb.get(), includeStackTrace)}; + createUnknownExceptionMessage(ex.get())}; } } @@ -802,10 +749,10 @@ IcePy::setPythonException(PyObject* ex) } void -IcePy::throwPythonException(bool includeStackTrace) +IcePy::throwPythonException() { PyException ex; - ex.raise(includeStackTrace); + ex.raise(); } void diff --git a/python/modules/IcePy/Util.h b/python/modules/IcePy/Util.h index e680c95651b..364f0e94692 100644 --- a/python/modules/IcePy/Util.h +++ b/python/modules/IcePy/Util.h @@ -89,7 +89,7 @@ namespace IcePy // // Convert the Python exception to its C++ equivalent. // - void raise(bool includeStackTrace); + void raise(); // // If the Python exception is SystemExit, act on it. May not return. @@ -97,11 +97,6 @@ namespace IcePy void checkSystemExit(); PyObjectHandle ex; - - private: - - PyObjectHandle _type; - PyObjectHandle _tb; }; // @@ -156,7 +151,7 @@ namespace IcePy // Converts the interpreter's current exception into an Ice exception // and throws it. // - void throwPythonException(bool includeStackTrace); + void throwPythonException(); // // Handle the SystemExit exception. From 2b6749bf6a708fe58c4e9faab9f9e42760aa7977 Mon Sep 17 00:00:00 2001 From: Jose Date: Thu, 1 Aug 2024 18:18:22 +0200 Subject: [PATCH 3/5] Review fixes --- python/modules/IcePy/Util.cpp | 1 + python/modules/IcePy/Util.h | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/python/modules/IcePy/Util.cpp b/python/modules/IcePy/Util.cpp index b46661e2a07..5af0570fe6d 100644 --- a/python/modules/IcePy/Util.cpp +++ b/python/modules/IcePy/Util.cpp @@ -277,6 +277,7 @@ IcePy::PyException::PyException() IcePy::PyException::PyException(PyObject* raisedException) { Py_XINCREF(raisedException); + ex = raisedException; } namespace IcePy diff --git a/python/modules/IcePy/Util.h b/python/modules/IcePy/Util.h index 364f0e94692..e47b7f778ec 100644 --- a/python/modules/IcePy/Util.h +++ b/python/modules/IcePy/Util.h @@ -87,7 +87,7 @@ namespace IcePy PyException(PyObject*); // - // Convert the Python exception to its C++ equivalent. + // Converts the Python exception into one of the 6 special Ice local exceptions. // void raise(); From 9b4a552033c7bb14322397ea3ae77232689c907f Mon Sep 17 00:00:00 2001 From: Jose Date: Thu, 1 Aug 2024 18:18:57 +0200 Subject: [PATCH 4/5] Formatting fixes --- python/modules/IcePy/Util.cpp | 15 +++------------ 1 file changed, 3 insertions(+), 12 deletions(-) diff --git a/python/modules/IcePy/Util.cpp b/python/modules/IcePy/Util.cpp index 5af0570fe6d..31abaf2e09a 100644 --- a/python/modules/IcePy/Util.cpp +++ b/python/modules/IcePy/Util.cpp @@ -269,10 +269,7 @@ IcePy::PyObjectHandle::release() return result; } -IcePy::PyException::PyException() -{ - ex = PyErr_GetRaisedException(); -} +IcePy::PyException::PyException() { ex = PyErr_GetRaisedException(); } IcePy::PyException::PyException(PyObject* raisedException) { @@ -383,17 +380,11 @@ IcePy::PyException::raise() { throw Ice::UnknownException{__FILE__, __LINE__, std::move(message)}; } - throw Ice::UnknownLocalException{ - __FILE__, - __LINE__, - createUnknownExceptionMessage(ex.get())}; + throw Ice::UnknownLocalException{__FILE__, __LINE__, createUnknownExceptionMessage(ex.get())}; } else { - throw Ice::UnknownException{ - __FILE__, - __LINE__, - createUnknownExceptionMessage(ex.get())}; + throw Ice::UnknownException{__FILE__, __LINE__, createUnknownExceptionMessage(ex.get())}; } } From 35dff05335ca9a3a0c92d6e79441853c9bb26bde Mon Sep 17 00:00:00 2001 From: Jose Date: Fri, 2 Aug 2024 09:18:11 +0200 Subject: [PATCH 5/5] Review fixes --- python/modules/IcePy/Util.cpp | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/python/modules/IcePy/Util.cpp b/python/modules/IcePy/Util.cpp index 31abaf2e09a..5a4e3f4dc06 100644 --- a/python/modules/IcePy/Util.cpp +++ b/python/modules/IcePy/Util.cpp @@ -277,18 +277,18 @@ IcePy::PyException::PyException(PyObject* raisedException) ex = raisedException; } -namespace IcePy +namespace { string getTypeName(PyObject* ex) { PyObject* cls = reinterpret_cast(ex->ob_type); - PyObjectHandle name = getAttr(cls, "__name__", false); + IcePy::PyObjectHandle name = IcePy::getAttr(cls, "__name__", false); assert(name.get()); - PyObjectHandle mod = getAttr(cls, "__module__", false); + IcePy::PyObjectHandle mod = IcePy::getAttr(cls, "__module__", false); assert(mod.get()); - string result = getString(mod.get()); + string result = IcePy::getString(mod.get()); result += "."; - result += getString(name.get()); + result += IcePy::getString(name.get()); return result; } string createUnknownExceptionMessage(PyObject* ex)