From fb7e6c1707eb523adba74950e9cb81e9131497a3 Mon Sep 17 00:00:00 2001 From: Imani Pelton Date: Thu, 26 Mar 2026 08:44:46 -0400 Subject: [PATCH 1/8] feat: send errors through the emitter --- craft_cli/_rs/emitter.pyi | 7 +++ craft_cli/_rs/errors.pyi | 46 +++++++++++++++++++ craft_cli/errors.py | 79 ++------------------------------ src/emitter.rs | 57 ++++++++++++++++++++++-- src/errors.rs | 94 +++++++++++++++++++++++++++++++++++++++ src/lib.rs | 4 ++ 6 files changed, 209 insertions(+), 78 deletions(-) create mode 100644 craft_cli/_rs/errors.pyi create mode 100644 src/errors.rs diff --git a/craft_cli/_rs/emitter.pyi b/craft_cli/_rs/emitter.pyi index d3646d34..75ad94e5 100644 --- a/craft_cli/_rs/emitter.pyi +++ b/craft_cli/_rs/emitter.pyi @@ -1,6 +1,7 @@ from enum import Enum from pathlib import Path +from craft_cli._rs.errors import CraftError from craft_cli._rs.progress import Progresser from .streams import StreamHandle @@ -119,6 +120,12 @@ class Emitter: can be provided via the ``prefix`` parameter. """ + def error(self, error: CraftError) -> None: + """Show an error and terminate this Emitter.""" + + def report_error(self, error: CraftError) -> None: + """Show an error, but continue execution.""" + def ended_ok(self) -> None: """Stop gracefully.""" diff --git a/craft_cli/_rs/errors.pyi b/craft_cli/_rs/errors.pyi new file mode 100644 index 00000000..ba3f7b8e --- /dev/null +++ b/craft_cli/_rs/errors.pyi @@ -0,0 +1,46 @@ +"""Base error type supported by an Emitter.""" + +class CraftError(BaseException): + """A program error to report with context.""" + + message: str + """The main message to the user, to be shown as the first line of the error.""" + + details: str | None + """Deeper, verbose error details that may help with diagnosing the deeper issue.""" + + docs_url: str | None + """A URL to point the user towards for troubleshooting. + + Supersedes `docs_slug`.""" + + docs_slug: str | None + """A slug to append to user documentation. + + This field is meant to be appended to a base URL provided by the method handling this error. + + Is superseded by `docs_url`.""" + + show_logpath: bool + """Whether to display the path to the log alongside this message. + + It is generally recommended to leave this on, but some extremely simple error cases may + display better without the log path.""" + + retcode: int + """The code to return when the application finishes. + + This error class does not exit the program itself. Instead, this field is meant to be used + by a consumer inspecting the error message.""" + + def __init__( + self, + message: str, + *, + details: str | None = None, + resolution: str | None = None, + docs_url: str | None = None, + docs_slug: str | None = None, + show_logpath: bool = True, + retcode: int = 1, + ) -> None: ... diff --git a/craft_cli/errors.py b/craft_cli/errors.py index 607192f6..3903164f 100644 --- a/craft_cli/errors.py +++ b/craft_cli/errors.py @@ -16,84 +16,13 @@ """Error classes.""" -__all__ = [ - "CraftError", -] - from typing import Any, cast +from craft_cli._rs.errors import CraftError -class CraftError(Exception): - """Signal a program error with a lot of information to report.""" - - message: str - """The main message to the user, to be shown as first line (and probably only that, - according to the different modes); note that in some cases the log location will be - attached to this message.""" - - details: str | None - """The full error details received from a third party which originated the error - situation.""" - - resolution: str | None - """An extra line indicating to the user how the error may be fixed or avoided (to be - shown together with ``message``).""" - - docs_url: str | None - """An URL to point the user to documentation (to be shown together with ``message``).""" - - doc_slug: str | None - """The slug to the user documentation. Needs a base url to form a full address. - Note that ``docs_url`` has preference if it is set.""" - - logpath_report: bool - """Whether the location of the log filepath should be presented in the screen as the - final message.""" - - reportable: bool - """If an error report should be sent to some error-handling backend (like Sentry).""" - - retcode: int - """The code to return when the application finishes.""" - - def __init__( - self, - message: str, - *, - details: str | None = None, - resolution: str | None = None, - docs_url: str | None = None, - logpath_report: bool = True, - reportable: bool = True, - retcode: int = 1, - doc_slug: str | None = None, - ) -> None: - super().__init__(message) - self.details = details - self.resolution = resolution - self.docs_url = docs_url - self.logpath_report = logpath_report - self.reportable = reportable - self.retcode = retcode - self.doc_slug = doc_slug - if doc_slug and not doc_slug.startswith("/"): - self.doc_slug = "/" + doc_slug - - def __eq__(self, other: object) -> bool: - if isinstance(other, CraftError): - return all( - [ - self.args == other.args, - self.details == other.details, - self.resolution == other.resolution, - self.docs_url == other.docs_url, - self.logpath_report == other.logpath_report, - self.reportable == other.reportable, - self.retcode == other.retcode, - self.doc_slug == other.doc_slug, - ] - ) - return NotImplemented +__all__ = [ + "CraftError", +] class CraftCommandError(CraftError): diff --git a/src/emitter.rs b/src/emitter.rs index 4f2092d7..122879e1 100644 --- a/src/emitter.rs +++ b/src/emitter.rs @@ -8,6 +8,7 @@ use pyo3::{ }; use crate::{ + errors::CraftError, logs::LogListener, printer::{Event, Target, Text}, progress::Progresser, @@ -57,9 +58,6 @@ struct Emitter { /// The original filepath of the log file. log_filepath: String, - // Used by `report_error` on the Python side, which was left in Python due to - // the retrieved errors all still being in Python. - #[expect(unused)] /// The base URL for error messages. docs_base_url: Option, @@ -337,6 +335,59 @@ impl Emitter { Ok(()) } + fn error(&mut self, error: &Bound<'_, CraftError>) -> PyResult<()> { + self.report_error(error)?; + // Stop the emitter + self.finish() + } + + fn report_error(&mut self, error: &Bound<'_, CraftError>) -> PyResult<()> { + if self.streaming_brief { + self.clear_prefix(); + } + + // Build the message + let message = { + let error = error.borrow(); + let mut message = error.message.clone(); + + if self.verbosity != Verbosity::Quiet + && let Some(ref details) = error.details + { + message.push_str(&format!("\n{details}")); + } + + if let Some(ref resolution) = error.resolution { + message.push_str(&format!("\nRecommended resolution: {resolution}")); + } + + // Give priority to `docs_url` in the event that it and `docs_slug` are both set. + if let Some(ref docs_url) = error.docs_url { + message.push_str(&format!("\nFor more information, visit {docs_url}")); + } else if let Some(ref docs_base_url) = self.docs_base_url + && let Some(ref doc_slug) = error.docs_slug + { + message.push_str(&format!( + "\nFor more information, visit {docs_base_url}{doc_slug}", + )); + } + + if error.show_logpath { + message.push_str(&format!("\nFull execution log: {}", self.log_filepath)); + } + + message + }; + + let event = Event::Text(Text { + message, + target: Some(Target::Stderr), + permanent: true, + }); + + crate::printer::printer().send(event) + } + /// Render an incremental progress bar. #[pyo3(signature = ( text, diff --git a/src/errors.rs b/src/errors.rs new file mode 100644 index 00000000..f750dfc1 --- /dev/null +++ b/src/errors.rs @@ -0,0 +1,94 @@ +use pyo3::{ + Bound, PyResult, PyTypeInfo as _, + exceptions::PyBaseException, + pyclass, pymethods, pymodule, + types::{PyAnyMethods, PyDict, PyString, PySuper, PyTypeMethods}, +}; + +#[derive(PartialEq)] +#[pyclass(extends = PyBaseException, subclass, get_all, set_all, eq)] +pub struct CraftError { + pub message: String, + pub details: Option, + pub resolution: Option, + pub docs_url: Option, + pub docs_slug: Option, + pub show_logpath: bool, + pub retcode: i8, +} + +#[pymethods] +impl CraftError { + #[new] + #[pyo3(signature = ( + message, + *, + details = None, + resolution = None, + docs_url = None, + docs_slug = None, + show_logpath = true, + retcode = 1 + ))] + fn new( + message: String, + details: Option, + resolution: Option, + docs_url: Option, + docs_slug: Option, + show_logpath: bool, + retcode: i8, + ) -> Self { + Self { + message, + details, + resolution, + docs_url, + docs_slug, + show_logpath, + retcode, + } + } + + #[pyo3(name = "__init__", signature = (_message, **_kwargs))] + fn init( + slf: &Bound<'_, Self>, + _message: &Bound<'_, PyString>, + _kwargs: Option<&Bound<'_, PyDict>>, + ) -> PyResult<()> { + // See https://pyo3.rs/main/class.html#initializer for an explanation on why this method + // needs to exist. + // Call "super(self.__class__, self).__init__()" + PySuper::new(&PyBaseException::type_object(slf.py()), slf)?.call_method0("__init__")?; + Ok(()) + } + + #[pyo3(name = "__repr__")] + fn repr(slf: &Bound<'_, Self>) -> PyResult { + Ok(format!( + "{}({:?})", + slf.get_type().qualname()?, + slf.borrow().message + )) + } + + #[pyo3(name = "__str__")] + fn str(&self) -> String { + self.message.clone() + } +} + +#[pymodule(submodule)] +pub mod errors { + use crate::utils::fix_imports; + use pyo3::types::PyModule; + use pyo3::{Bound, PyResult}; + + #[pymodule_export] + use super::CraftError; + + #[pymodule_init] + fn init(m: &Bound<'_, PyModule>) -> PyResult<()> { + fix_imports(m, "craft_cli._rs.errors") + } +} diff --git a/src/lib.rs b/src/lib.rs index e4f01b13..b51b8ecb 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -6,6 +6,7 @@ use pyo3::pymodule; mod craft_cli_utils; mod emitter; +mod errors; mod logs; mod printer; mod progress; @@ -24,4 +25,7 @@ mod craft_cli_extensions { #[pymodule_export] use crate::logs::LogListener; + + #[pymodule_export] + use crate::errors::errors; } From 85da4295866caa2c6cf2c31c185dd81d984ae987 Mon Sep 17 00:00:00 2001 From: Imani Pelton Date: Thu, 26 Mar 2026 08:52:04 -0400 Subject: [PATCH 2/8] fix: don't panic if the printer was already stopped --- src/printer.rs | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/src/printer.rs b/src/printer.rs index 00dabfcd..425ba0e9 100644 --- a/src/printer.rs +++ b/src/printer.rs @@ -349,6 +349,9 @@ pub struct Printer { /// Whether or not a progress bar is currently running. in_progress: bool, + + /// Whether this printer has been stopped. + stopped: bool, } impl Printer { @@ -400,11 +403,18 @@ impl Printer { return Err(*e.downcast::().unwrap()); } + self.stopped = true; + Ok(()) } /// Send a message to the `InnerPrinter` for displaying pub fn send(&mut self, event: Event) -> PyResult<()> { + if self.stopped { + return Err(PyRuntimeError::new_err( + "Attempted to print after the printer was stopped", + )); + } let prepared_event = self.prepare_event(event)?; match prepared_event { None => Ok(()), From e400044acd0be92cfb41e548806a143b530ca159 Mon Sep 17 00:00:00 2001 From: Imani Pelton Date: Fri, 27 Mar 2026 14:38:46 -0400 Subject: [PATCH 3/8] fix: unsigned int for retcodes --- src/errors.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/errors.rs b/src/errors.rs index f750dfc1..a0242650 100644 --- a/src/errors.rs +++ b/src/errors.rs @@ -14,7 +14,7 @@ pub struct CraftError { pub docs_url: Option, pub docs_slug: Option, pub show_logpath: bool, - pub retcode: i8, + pub retcode: u8, } #[pymethods] @@ -37,7 +37,7 @@ impl CraftError { docs_url: Option, docs_slug: Option, show_logpath: bool, - retcode: i8, + retcode: u8, ) -> Self { Self { message, From 8895133a7f17df62d10bf53c6e1c011217c994d1 Mon Sep 17 00:00:00 2001 From: Imani Pelton Date: Fri, 27 Mar 2026 14:55:52 -0400 Subject: [PATCH 4/8] fix: timestamp on errors in debuggy modes --- src/emitter.rs | 68 ++++++++++++++++++++++++++------------------------ 1 file changed, 35 insertions(+), 33 deletions(-) diff --git a/src/emitter.rs b/src/emitter.rs index 122879e1..41a99533 100644 --- a/src/emitter.rs +++ b/src/emitter.rs @@ -346,46 +346,48 @@ impl Emitter { self.clear_prefix(); } - // Build the message - let message = { - let error = error.borrow(); - let mut message = error.message.clone(); - - if self.verbosity != Verbosity::Quiet - && let Some(ref details) = error.details - { - message.push_str(&format!("\n{details}")); + let send_error_event = |mut message: String| -> PyResult<()> { + if self.verbosity >= Verbosity::Debug { + message = utils::apply_timestamp(&message).to_string(); } + let event = Event::Text(Text { + message, + target: Some(Target::Stderr), + permanent: true, + }); + crate::printer::printer().send(event) + }; - if let Some(ref resolution) = error.resolution { - message.push_str(&format!("\nRecommended resolution: {resolution}")); - } + let error = error.borrow(); - // Give priority to `docs_url` in the event that it and `docs_slug` are both set. - if let Some(ref docs_url) = error.docs_url { - message.push_str(&format!("\nFor more information, visit {docs_url}")); - } else if let Some(ref docs_base_url) = self.docs_base_url - && let Some(ref doc_slug) = error.docs_slug - { - message.push_str(&format!( - "\nFor more information, visit {docs_base_url}{doc_slug}", - )); - } + send_error_event(error.message.clone())?; - if error.show_logpath { - message.push_str(&format!("\nFull execution log: {}", self.log_filepath)); - } + if self.verbosity != Verbosity::Quiet + && let Some(ref details) = error.details + { + send_error_event(details.clone())?; + } - message - }; + if let Some(ref resolution) = error.resolution { + send_error_event(format!("Recommended resolution: {resolution}"))?; + } - let event = Event::Text(Text { - message, - target: Some(Target::Stderr), - permanent: true, - }); + // Give priority to `docs_url` in the event that it and `docs_slug` are both set. + if let Some(ref docs_url) = error.docs_url { + send_error_event(format!("For more information, visit {docs_url}"))?; + } else if let Some(ref docs_base_url) = self.docs_base_url + && let Some(ref docs_slug) = error.docs_slug + { + send_error_event(format!( + "For more information, visit {docs_base_url}{docs_slug}", + ))?; + } + + if error.show_logpath { + send_error_event(format!("Full execution log: {}", self.log_filepath))?; + } - crate::printer::printer().send(event) + Ok(()) } /// Render an incremental progress bar. From 85160757edab7c94e9d60a917c7a0848e359d309 Mon Sep 17 00:00:00 2001 From: Imani Pelton Date: Fri, 27 Mar 2026 15:25:28 -0400 Subject: [PATCH 5/8] fix: correct init behavior for super class --- src/errors.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/errors.rs b/src/errors.rs index a0242650..2745bd69 100644 --- a/src/errors.rs +++ b/src/errors.rs @@ -50,16 +50,16 @@ impl CraftError { } } - #[pyo3(name = "__init__", signature = (_message, **_kwargs))] + #[pyo3(name = "__init__", signature = (message, **_kwargs))] fn init( slf: &Bound<'_, Self>, - _message: &Bound<'_, PyString>, + message: &Bound<'_, PyString>, _kwargs: Option<&Bound<'_, PyDict>>, ) -> PyResult<()> { // See https://pyo3.rs/main/class.html#initializer for an explanation on why this method // needs to exist. // Call "super(self.__class__, self).__init__()" - PySuper::new(&PyBaseException::type_object(slf.py()), slf)?.call_method0("__init__")?; + PySuper::new(&Self::type_object(slf.py()), slf)?.call_method1("__init__", (message,))?; Ok(()) } From b1e48993d3486ee29bb382d9c407912dd9b1fad9 Mon Sep 17 00:00:00 2001 From: Imani Pelton Date: Fri, 27 Mar 2026 15:30:55 -0400 Subject: [PATCH 6/8] fix: inherit from Exception instead of BaseException --- craft_cli/_rs/errors.pyi | 2 +- src/errors.rs | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/craft_cli/_rs/errors.pyi b/craft_cli/_rs/errors.pyi index ba3f7b8e..15eb1be4 100644 --- a/craft_cli/_rs/errors.pyi +++ b/craft_cli/_rs/errors.pyi @@ -1,6 +1,6 @@ """Base error type supported by an Emitter.""" -class CraftError(BaseException): +class CraftError(Exception): """A program error to report with context.""" message: str diff --git a/src/errors.rs b/src/errors.rs index 2745bd69..7b5977ff 100644 --- a/src/errors.rs +++ b/src/errors.rs @@ -1,12 +1,12 @@ use pyo3::{ Bound, PyResult, PyTypeInfo as _, - exceptions::PyBaseException, + exceptions::PyException, pyclass, pymethods, pymodule, types::{PyAnyMethods, PyDict, PyString, PySuper, PyTypeMethods}, }; #[derive(PartialEq)] -#[pyclass(extends = PyBaseException, subclass, get_all, set_all, eq)] +#[pyclass(extends = PyException, subclass, get_all, set_all, eq)] pub struct CraftError { pub message: String, pub details: Option, From f311f20e5da6687a0a4c015f53acbbe0af6f0707 Mon Sep 17 00:00:00 2001 From: Imani Pelton Date: Fri, 27 Mar 2026 15:34:05 -0400 Subject: [PATCH 7/8] fix: always normalize url components --- src/emitter.rs | 2 +- src/errors.rs | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/src/emitter.rs b/src/emitter.rs index 41a99533..68d42b79 100644 --- a/src/emitter.rs +++ b/src/emitter.rs @@ -379,7 +379,7 @@ impl Emitter { && let Some(ref docs_slug) = error.docs_slug { send_error_event(format!( - "For more information, visit {docs_base_url}{docs_slug}", + "For more information, visit {docs_base_url}/{docs_slug}", ))?; } diff --git a/src/errors.rs b/src/errors.rs index 7b5977ff..e110bb14 100644 --- a/src/errors.rs +++ b/src/errors.rs @@ -39,6 +39,7 @@ impl CraftError { show_logpath: bool, retcode: u8, ) -> Self { + let docs_slug = docs_slug.map(|slug| slug.trim_start_matches('/').into()); Self { message, details, From 3bb1640acd73208a7c6febf05b6a47c5bb50d413 Mon Sep 17 00:00:00 2001 From: Imani Pelton Date: Fri, 27 Mar 2026 15:35:43 -0400 Subject: [PATCH 8/8] chore: facepalm --- craft_cli/_rs/errors.pyi | 3 +++ 1 file changed, 3 insertions(+) diff --git a/craft_cli/_rs/errors.pyi b/craft_cli/_rs/errors.pyi index 15eb1be4..5ab67a7f 100644 --- a/craft_cli/_rs/errors.pyi +++ b/craft_cli/_rs/errors.pyi @@ -9,6 +9,9 @@ class CraftError(Exception): details: str | None """Deeper, verbose error details that may help with diagnosing the deeper issue.""" + resolution: str | None + """A brief suggestion for how the user might resolve the error themselves.""" + docs_url: str | None """A URL to point the user towards for troubleshooting.