Skip to content

Expose exception cause as std::nested_exception #4033

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

Draft
wants to merge 3 commits into
base: master
Choose a base branch
from
+22 −2
Draft

Expose exception cause as std::nested_exception #4033

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
1bd0eaa
Expose exception cause as std::nested_exception
ecatmur Jun 28, 2022
a8046ff
don't set nested exception if already set
ecatmur Jun 28, 2022
059362b
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Jun 28, 2022
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
24 changes: 22 additions & 2 deletions include/pybind11/pytypes.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -546,6 +546,8 @@ struct error_fetch_and_normalize {
return (PyErr_GivenExceptionMatches(m_type.ptr(), exc.ptr()) != 0);
}

std::nested_exception get_cause_as_nested() const;

// Not protecting these for simplicity.
object m_type, m_value, m_trace;

Expand All @@ -572,13 +574,17 @@ PYBIND11_NAMESPACE_END(detail)
/// thrown to propagate python-side errors back through C++ which can either be caught manually or
/// else falls back to the function dispatcher (which then raises the captured error back to
/// python).
class PYBIND11_EXPORT_EXCEPTION error_already_set : public std::exception {
class PYBIND11_EXPORT_EXCEPTION error_already_set : public std::exception,
public std::nested_exception {
public:
/// Fetches the current Python exception (using PyErr_Fetch()), which will clear the
/// current Python error indicator.
error_already_set()
: m_fetched_error{new detail::error_fetch_and_normalize("pybind11::error_already_set"),
m_fetched_error_deleter} {}
m_fetched_error_deleter} {
if (not nested_ptr())
static_cast<std::nested_exception &>(*this) = m_fetched_error->get_cause_as_nested();
}

/// The what() result is built lazily on demand.
/// WARNING: This member function needs to acquire the Python GIL. This can lead to
Expand Down Expand Up @@ -2327,6 +2333,20 @@ bool object_api<D>::rich_compare(object_api const &other, int value) const {
return rv == 1;
}

inline std::nested_exception error_fetch_and_normalize::get_cause_as_nested() const {
auto cause = reinterpret_steal<object>(PyException_GetCause(m_value.ptr()));
if (not cause or cause.is_none())
return std::nested_exception();
auto typ = type::of(cause);
auto tb = reinterpret_steal<object>(PyException_GetTraceback(cause.ptr()));
Copy link
Collaborator

Choose a reason for hiding this comment

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

PyException_GetTraceback can return null. Will PyErr_Restore work if this is the case?

PyErr_Restore(typ.release().ptr(), cause.release().ptr(), tb.release().ptr());
try {
throw error_already_set();
Copy link
Collaborator

Choose a reason for hiding this comment

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

Throwing is very slow and something that is best avoided here. is there a way we avoid having to throw the exception just to catch and return an empty std::nested_exception?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Can't we just use the CPython API to check if the exception is nested instead?

} catch (...) {
return std::nested_exception();
}
}

#define PYBIND11_MATH_OPERATOR_UNARY(op, fn) \
template <typename D> \
object object_api<D>::op() const { \
Expand Down