Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

PyException cleanup #2597

Merged
merged 5 commits into from
Aug 2, 2024
Merged

PyException cleanup #2597

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
ea1e8a4
PyException cleanup
pepone Jul 31, 2024
2fd572b
Remove includeStackTrace param
pepone Aug 1, 2024
2b6749b
Review fixes
pepone Aug 1, 2024
9b4a552
Formatting fixes
pepone Aug 1, 2024
35dff05
Review fixes
pepone Aug 2, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion python/modules/IcePy/Logger.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,7 @@ loggerNew(PyTypeObject* type, PyObject* /*args*/, PyObject* /*kwds*/)
{
return nullptr;
}
self->logger = 0;
self->logger = nullptr;
return self;
}

Expand Down
4 changes: 2 additions & 2 deletions python/modules/IcePy/Operation.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -859,7 +859,7 @@ Operation::marshalResult(Ice::OutputStream& os, PyObject* result)
{
try
{
PyException().raise();
throwPythonException();
}
catch (const Ice::UnknownException& ex)
{
Expand All @@ -879,7 +879,7 @@ Operation::marshalResult(Ice::OutputStream& os, PyObject* result)
{
try
{
PyException().raise();
throwPythonException();
}
catch (const Ice::UnknownException& ex)
{
Expand Down
265 changes: 84 additions & 181 deletions python/modules/IcePy/Util.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -269,226 +269,132 @@ IcePy::PyObjectHandle::release()
return result;
}

IcePy::PyException::PyException()
{
ex = PyErr_GetRaisedException();
if (ex)
{
PyObject* type = reinterpret_cast<PyObject*>(Py_TYPE(ex.get()));
Py_INCREF(type);
_type = type;
_tb = PyException_GetTraceback(ex.get());
}
}
IcePy::PyException::PyException() { ex = PyErr_GetRaisedException(); }

IcePy::PyException::PyException(PyObject* raisedException)
{
Py_XINCREF(raisedException);
ex = raisedException;
PyObject* type = reinterpret_cast<PyObject*>(Py_TYPE(raisedException));
Py_INCREF(type);
_type = type;
_tb = PyException_GetTraceback(raisedException);
}

void
IcePy::PyException::raise()
namespace
{
assert(ex.get());

PyObject* userExceptionType = lookupType("Ice.UserException");
PyObject* localExceptionType = lookupType("Ice.LocalException");

// TODO: create better error messages.

if (PyObject_IsInstance(ex.get(), userExceptionType))
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We no longer handel user exceptions in raise. TypedUpcall::exception always marshall user exceptions, and Invocation::unmarshalException unmarshall exceptions and validates exception specificiation. A user exception in PyException::raise doesn't have a special meaning.

string getTypeName(PyObject* ex)
{
string tb = getTraceback();
if (!tb.empty())
{
throw Ice::UnknownUserException{__FILE__, __LINE__, tb};
}
else
{
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())};
}
}
PyObject* cls = reinterpret_cast<PyObject*>(ex->ob_type);
IcePy::PyObjectHandle name = IcePy::getAttr(cls, "__name__", false);
assert(name.get());
IcePy::PyObjectHandle mod = IcePy::getAttr(cls, "__module__", false);
assert(mod.get());
string result = IcePy::getString(mod.get());
result += ".";
result += IcePy::getString(name.get());
return result;
}
else if (PyObject_IsInstance(ex.get(), localExceptionType))
string createUnknownExceptionMessage(PyObject* ex)
{
raiseLocalException();
}
else
{
string tb = getTraceback();
if (!tb.empty())
{
throw Ice::UnknownException{__FILE__, __LINE__, tb};
}
else
ostringstream ostr;
ostr << getTypeName(ex);
IcePy::PyObjectHandle exStr = PyObject_Str(ex);
if (exStr.get() && IcePy::checkString(exStr.get()))
{
ostringstream ostr;

ostr << getTypeName();

IcePy::PyObjectHandle msg = PyObject_Str(ex.get());
if (msg.get())
string message = IcePy::getString(exStr.get());
if (!message.empty())
{
string s = getString(msg.get());
if (!s.empty())
{
ostr << ": " << s;
}
ostr << ": " << message;
}

throw Ice::UnknownException{__FILE__, __LINE__, ostr.str()};
}
return ostr.str();
}
}

void
IcePy::PyException::checkSystemExit()
IcePy::PyException::raise()
{
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()
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

merged raiseLocalException and raise

{
string typeName = getTypeName();
PyObject* requestFailedExceptionType = lookupType("Ice.RequestFailedException");

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());

if (PyObject_IsInstance(ex.get(), requestFailedExceptionType))
{
PyObjectHandle idAttr = getAttr(ex.get(), "id", false);
Ice::Identity id;
if (idAttr.get())
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));
}
}

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())};
}
else
{
throw Ice::UnknownLocalException{__FILE__, __LINE__, typeName};
throw Ice::UnknownException{__FILE__, __LINE__, createUnknownExceptionMessage(ex.get())};
}
}

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<PyObject*>(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*
Expand Down Expand Up @@ -767,7 +673,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)
Expand All @@ -776,7 +681,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)
Expand All @@ -796,7 +700,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.
Expand Down
10 changes: 1 addition & 9 deletions python/modules/IcePy/Util.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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();

Expand All @@ -97,14 +97,6 @@ namespace IcePy
void checkSystemExit();

PyObjectHandle ex;

private:
void raiseLocalException();
std::string getTraceback();
std::string getTypeName();

PyObjectHandle _type;
PyObjectHandle _tb;
};

//
Expand Down
Loading