Skip to content

feat: rust error emitting#432

Open
bepri wants to merge 2 commits intofeature/oxidationfrom
work/errors-rust/CRAFT-4895
Open

feat: rust error emitting#432
bepri wants to merge 2 commits intofeature/oxidationfrom
work/errors-rust/CRAFT-4895

Conversation

@bepri
Copy link
Member

@bepri bepri commented Mar 26, 2026

  • Have you followed the guidelines for contributing?
  • Have you signed the CLA?
  • Have you successfully run make lint && make test?

Adds back error and report_error to the Rust Emitter.

Here's a script for easy testing:

from craft_cli import CraftError, Emitter, Verbosity


class CraftierError(CraftError):
    """An error, but it's real crafty. Look out, kids."""

e = Emitter(Verbosity.TRACE, "blah.log", "Hello", streaming_brief=True)

err = CraftierError(
    "You smell bad.",
    details="We're talking like, I'm reasonably sure there's a provision in the Geneva Conventions for the chemicals you're emitting.",
    resolution="Shower.",
    docs_url="https://www.homedepot.com/b/Bath-Showers/N-5yc1vZbzcd",
    show_logpath=False,
)

e.error(err)

e.message("I'm going to explode, but at least it's not a panic!")

@bepri bepri self-assigned this Mar 26, 2026
@@ -0,0 +1,46 @@
"""Base error type supported by an Emitter."""

class CraftError(BaseException):
Copy link
Member Author

Choose a reason for hiding this comment

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

These are slightly reworked doc strings, and the reportable field was removed as the only time it was ever accessed in downstream code was to set it to False.

Ok(())
}

fn error(&mut self, error: &Bound<'_, CraftError>) -> PyResult<()> {
Copy link
Member Author

Choose a reason for hiding this comment

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

&Bound<'_, CraftError> is a Python object that is "bound" to the GIL. The type argument of CraftError asserts at runtime that the Python object is mappable to a CraftError. This is how to access types that were instantiated in Python therefore cannot become owned by Rust.

};

#[derive(PartialEq)]
#[pyclass(extends = PyBaseException, subclass, get_all, set_all, eq)]
Copy link
Member Author

Choose a reason for hiding this comment

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

This reads: "this class extends BaseException, is subclassable in Python, has getters & setters for each field, and is comparable with __eq__."

Comment on lines +59 to +60
// See https://pyo3.rs/main/class.html#initializer for an explanation on why this method
// needs to exist.
Copy link
Member Author

Choose a reason for hiding this comment

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

The TL;DR of this link is that because we extended a base Python type (PyBaseException) instead of a type we defined ourselves in Rust with #[pyclass], the __init__ method of the base class was fighting against us.

It seems like PyO3 was inserting the exact same arguments given to __new__ into BaseException.__init__(), which was failing since it takes no kwargs. This explicitly calls it with no arguments at all.

@bepri bepri marked this pull request as ready for review March 26, 2026 13:05
@bepri bepri requested a review from tigarmo as a code owner March 26, 2026 13:05
@bepri bepri requested review from medubelko and mr-cal March 26, 2026 13:06
/// Send a message to the `InnerPrinter` for displaying
pub fn send(&mut self, event: Event) -> PyResult<()> {
if self.stopped {
return Err(PyRuntimeError::new_err(
Copy link
Member Author

Choose a reason for hiding this comment

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

@medubelko look for this pattern of "return Err(" when reviewing these PRs, any instance of this in Rust has the potential to be shown to the user if not caught (and many aren't caught right now, heh).

But this is the only one in this PR anyways

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant