search

PyO3 / pyo3

Rust bindings for the Python interpreter
https://pyo3.rs
Apache License 2.0
11.96k stars 736 forks source link

pyo3-error crate #4500

Open juntyr opened 2 weeks ago

juntyr commented 2 weeks ago

I've recently been down a rabbit hole to improve error reporting across Python and Rust with preserved causality chains and just published the first version of my new https://github.com/juntyr/pyo3-error crate.

Most importantly, the crate provides conversions from std::error::Error::source causal chains to pyo3::PyErr::cause chains and a wrapper for PyErr to interpret its pyo3::PyErr::cause chain as a std::error::Error::source chain.

I hope that this crate might be useful to the broader PyO3 community (and I am sure someone will have ideas for how to make its API a lot more convenient :) )

bschoenmaeckers commented 2 weeks ago

What is the performance impact of converting all source errors into rust compared to a plain PyErr?

juntyr commented 2 weeks ago

What is the performance impact of converting all source errors into rust compared to a plain PyErr?

At the moment, this conversion scales linearly with the length of the causal chain. std::error::Error::source returns a reference lifetime-bound to the original error, but PyErr::cause returns an unbound PyErr. Therefore, we need to materialize the full chain into a second chain on the Rust side (broadly speaking, struct PyErrChain { err: PyErr, source: Option<Box<PyErrChain>> }). Since PyErr can modify its cause through a clone, this materialization is also just a snapshot.

An alternative design I could think of is one that would provide a zero-cost wrapper around PyErr and push all cost to when source is called. Every error down the source chain would still point to the original top-level PyErr but use type-system-level-only layers of wrapping to denote the chain depth. However, this would mean that when you have a reference to the 3rd cause in the chain and call it's source method, it would have to traverse all the way from the top-level back down to the 4th cause layer. Furthermore, this approach might need to panic if the causal chain has been modified in Python since wrapping it in Python.

Since I personally prefer the snapshot approach, I went with that one for the initial implementation.

I'd be delighted to hear if you have some ideas for how the API and its cost could be improved :)