-
Notifications
You must be signed in to change notification settings - Fork 3
feat: smarter exception messaging — deduplicated chains, format_for_display, log_record #278
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.
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -13,18 +13,121 @@ | |
| from typing import TYPE_CHECKING | ||
|
|
||
| from codeweaver.core import CodeWeaverError | ||
| from codeweaver.core.exceptions import _BULLET | ||
| from codeweaver.core.utils import get_codeweaver_prefix | ||
|
|
||
|
|
||
| if TYPE_CHECKING: | ||
| from codeweaver.cli.ui.status_display import StatusDisplay | ||
|
|
||
|
|
||
| # --------------------------------------------------------------------------- | ||
| # Exception-chain helpers | ||
| # --------------------------------------------------------------------------- | ||
|
|
||
| _MAX_CHAIN_DEPTH = 10 | ||
|
|
||
|
|
||
| def _collect_codeweaver_chain(exc: BaseException) -> list[CodeWeaverError]: | ||
| """Walk ``__cause__`` / ``__context__`` and return all CodeWeaverError nodes. | ||
|
|
||
| Returns nodes in outermost-first order (the exception itself is first). | ||
| Stops following the chain when it hits a non-CodeWeaverError or exceeds | ||
| ``_MAX_CHAIN_DEPTH`` to guard against unexpectedly long or cyclic chains. | ||
|
|
||
| Args: | ||
| exc: The exception to start from. | ||
|
|
||
sourcery-ai[bot] marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| Returns: | ||
| List of CodeWeaverError instances from outermost to root. | ||
| """ | ||
| chain: list[CodeWeaverError] = [] | ||
| seen: set[int] = set() | ||
| current: BaseException | None = exc | ||
| while current is not None and len(chain) < _MAX_CHAIN_DEPTH: | ||
| exc_id = id(current) | ||
| if exc_id in seen: | ||
| break | ||
| seen.add(exc_id) | ||
| if not isinstance(current, CodeWeaverError): | ||
| # Stop walking once we leave the CodeWeaverError chain so we don't | ||
| # traverse an arbitrarily long external __cause__/__context__ chain. | ||
| break | ||
| chain.append(current) | ||
| # Explicit cause (raise X from Y) takes priority; fall back to implicit | ||
| # context only when it has not been suppressed by ``raise X from None``. | ||
| next_exc: BaseException | None = current.__cause__ or ( | ||
| current.__context__ if not current.__suppress_context__ else None | ||
| ) | ||
| current = next_exc | ||
| return chain | ||
|
|
||
|
|
||
| def _get_external_root(exc: BaseException) -> BaseException | None: | ||
| """Return the first non-CodeWeaverError exception at the root of the chain. | ||
|
|
||
| This surfaces the original third-party or built-in exception (e.g. | ||
| ``OSError``, ``ImportError``) that triggered the CodeWeaver chain so users | ||
| can see what actually went wrong at the lowest level. | ||
|
|
||
| Args: | ||
| exc: The outermost exception. | ||
|
|
||
|
Comment on lines
+66
to
+75
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. issue: The current logic returns the first non- Either:
|
||
| Returns: | ||
| The non-CodeWeaverError root exception, or ``None`` if the entire chain | ||
| is made up of CodeWeaverError instances. | ||
| """ | ||
| seen: set[int] = set() | ||
| current: BaseException | None = exc | ||
| while current is not None: | ||
| exc_id = id(current) | ||
| if exc_id in seen: | ||
| break | ||
| seen.add(exc_id) | ||
| next_exc: BaseException | None = current.__cause__ or ( | ||
| current.__context__ if not current.__suppress_context__ else None | ||
| ) | ||
| # We found an external exception in the chain (not the root of the walk) | ||
| if not isinstance(current, CodeWeaverError) and current is not exc: | ||
| return current | ||
| current = next_exc | ||
| return None | ||
|
|
||
|
|
||
| def _deduplicate_suggestions(suggestions: list[str]) -> list[str]: | ||
| """Return *suggestions* with duplicates removed, preserving original order. | ||
|
|
||
| Args: | ||
| suggestions: Possibly-duplicated suggestion strings. | ||
|
|
||
| Returns: | ||
| De-duplicated list in original order. | ||
| """ | ||
| seen: set[str] = set() | ||
| result: list[str] = [] | ||
| for s in suggestions: | ||
| if s not in seen: | ||
| seen.add(s) | ||
| result.append(s) | ||
| return result | ||
|
|
||
|
|
||
| # --------------------------------------------------------------------------- | ||
| # Error handler | ||
| # --------------------------------------------------------------------------- | ||
|
|
||
|
|
||
| class CLIErrorHandler: | ||
| """Unified error handling for CLI commands. | ||
|
|
||
| Provides consistent error display across all CLI commands with appropriate | ||
| detail levels based on error type and verbosity flags. | ||
|
|
||
| When a ``CodeWeaverError`` exception chain is displayed, each node in the | ||
| chain contributes its message and location, but suggestions are aggregated | ||
| and de-duplicated across the whole chain, and the issue-reporting | ||
| boilerplate is printed exactly once. This prevents walls of identical | ||
| advice when multiple CodeWeaver exceptions wrap one another. | ||
| """ | ||
|
|
||
| def __init__( | ||
|
|
@@ -66,22 +169,66 @@ def handle_error(self, error: Exception, context: str, *, exit_code: int = 1) -> | |
| sys.exit(exit_code) | ||
|
|
||
| def _handle_codeweaver_error(self, error: CodeWeaverError, context: str) -> None: | ||
| """Display CodeWeaver-specific errors. | ||
| """Display a CodeWeaver exception chain without repeating boilerplate. | ||
|
|
||
| Walks the full ``__cause__`` / ``__context__`` chain and: | ||
|
|
||
| * Shows the outermost error in full (message, location, details). | ||
| * Shows each deeper cause condensed to a single line. | ||
| * Surfaces the first non-CodeWeaverError root cause if present. | ||
| * Aggregates all suggestions across the chain, de-duplicates them, and | ||
| displays them once. | ||
| * Prints the issue-reporting boilerplate exactly once at the end. | ||
|
|
||
| Args: | ||
| error: CodeWeaverError to display | ||
| context: Context description | ||
| error: The outermost CodeWeaverError to display. | ||
| context: Human-readable context for the failure (e.g. "Indexing"). | ||
| """ | ||
| chain = _collect_codeweaver_chain(error) | ||
|
|
||
| self.display.console.print(f"\n{self.prefix}\n [bold red]✗ {context} failed[/bold red]\n") | ||
| self._print_primary_error(error) | ||
|
|
||
|
Comment on lines
+187
to
+191
|
||
| if len(chain) > 1: | ||
| self._print_cause_chain(chain[1:]) | ||
|
|
||
| ext_root = _get_external_root(error) | ||
| if ext_root: | ||
| self.display.console.print( | ||
| f"[dim]Underlying cause: {type(ext_root).__name__}: {ext_root}[/dim]\n" | ||
| ) | ||
|
|
||
| all_suggestions = _deduplicate_suggestions([s for exc in chain for s in exc.suggestions]) | ||
| if all_suggestions: | ||
| self.display.console.print("[yellow]Suggestions:[/yellow]") | ||
| for suggestion in all_suggestions: | ||
| self.display.console.print(f" {_BULLET} {suggestion}") | ||
| self.display.console.print() | ||
|
|
||
| for line in CodeWeaverError.issue_information: | ||
| self.display.console.print(line) | ||
|
|
||
| if self.verbose or self.debug: | ||
| self.display.console.print("\n[dim]Full traceback:[/dim]") | ||
| self.display.console.print_exception(show_locals=self.debug) | ||
|
|
||
| def _print_primary_error(self, error: CodeWeaverError) -> None: | ||
| """Print the outermost error with its message, location, and details. | ||
|
|
||
| Args: | ||
| error: The primary CodeWeaverError to render. | ||
| """ | ||
| from pydantic_core import to_json | ||
|
|
||
| # Print header with error context | ||
| self.display.console.print(f"\n{self.prefix} \n [bold red]✗ {context} failed[/bold red]\n") | ||
| from codeweaver.core.utils.environment import format_file_link | ||
|
|
||
| # Print error message | ||
| self.display.console.print(f"[bold red]Error:[/bold red] {error}\n") | ||
| self.display.console.print(f"[bold red]Error:[/bold red] {error.message}") | ||
| if error.location and error.location.filename: | ||
| link = format_file_link(error.location.filename, error.location.line_number) | ||
| self.display.console.print(f" [dim]in '{error.location.module_name}' at {link}[/dim]") | ||
| self.display.console.print() | ||
|
|
||
| # Show details if available | ||
| if hasattr(error, "details") and error.details: | ||
| if error.details: | ||
| self.display.console.print("[yellow]Details:[/yellow]") | ||
| if isinstance(error.details, dict): | ||
| self.display.console.print( | ||
|
|
@@ -91,17 +238,28 @@ def _handle_codeweaver_error(self, error: CodeWeaverError, context: str) -> None | |
| self.display.console.print(str(error.details)) | ||
| self.display.console.print() | ||
|
|
||
| # Show suggestions if available | ||
| if hasattr(error, "suggestions") and error.suggestions: | ||
| self.display.console.print("[yellow]Suggestions:[/yellow]") | ||
| for suggestion in error.suggestions: | ||
| self.display.console.print(f" • {suggestion}") | ||
| self.display.console.print() | ||
| def _print_cause_chain(self, causes: list[CodeWeaverError]) -> None: | ||
| """Print a condensed cause chain (all nodes except the outermost). | ||
|
|
||
| # Show full traceback in verbose/debug mode | ||
| if self.verbose or self.debug: | ||
| self.display.console.print("[dim]Full traceback:[/dim]") | ||
| self.display.console.print_exception(show_locals=self.debug) | ||
| Each cause is rendered on a single ``→ ExcType: message (location)`` | ||
| line so users can follow the chain without reading repeated boilerplate. | ||
|
|
||
| Args: | ||
| causes: Chain nodes in outermost-to-root order, excluding the | ||
| primary node already rendered by ``_print_primary_error``. | ||
| """ | ||
| self.display.console.print("[dim]Caused by:[/dim]") | ||
| for cause in causes: | ||
| location_str = "" | ||
| if cause.location and cause.location.filename: | ||
| location_str = ( | ||
| f" [dim](in '{cause.location.module_name}', " | ||
| f"line {cause.location.line_number})[/dim]" | ||
| ) | ||
| self.display.console.print( | ||
| f" [dim]→ {type(cause).__name__}: {cause.message}{location_str}[/dim]" | ||
| ) | ||
| self.display.console.print() | ||
|
|
||
| def _handle_unexpected_error(self, error: Exception, context: str) -> None: | ||
| """Display unexpected errors. | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -18,6 +18,9 @@ | |
| from typing import Any, ClassVar, NamedTuple | ||
|
|
||
|
|
||
| _BULLET = "\u2022" | ||
|
|
||
|
|
||
| class LocationInfo(NamedTuple): | ||
| """Location information for where an exception was raised. | ||
|
|
||
|
|
@@ -100,9 +103,14 @@ class CodeWeaverError(Exception): | |
|
|
||
| Provides structured error information including details and suggestions | ||
| for resolution. | ||
|
|
||
| Use ``format_for_display()`` to render a single exception node with full | ||
| context for the user. Use ``log_record()`` to get a structured dict for | ||
| structured-logging systems. ``__str__`` returns a concise message | ||
| appropriate for tracebacks and plain log lines. | ||
| """ | ||
|
|
||
| _issue_information: ClassVar[tuple[str, ...]] = _get_issue_information() | ||
| issue_information: ClassVar[tuple[str, ...]] = _get_issue_information() | ||
|
|
||
| def __init__( | ||
| self, | ||
|
|
@@ -118,6 +126,7 @@ def __init__( | |
| message: Human-readable error message | ||
| details: Additional context about the error | ||
| suggestions: Actionable suggestions for resolving the error | ||
| location: Where the exception was raised (auto-detected when omitted) | ||
| """ | ||
| super().__init__(message) | ||
| self.message = message | ||
|
|
@@ -126,24 +135,60 @@ def __init__( | |
| self.location = location or LocationInfo.from_frame(2) | ||
|
|
||
| def __str__(self) -> str: | ||
| """Return descriptive error message with context details.""" | ||
| """Return a concise error message suitable for logs and tracebacks. | ||
|
|
||
| Returns only the error message and a brief location hint so that | ||
| exception chains don't repeat boilerplate at every level. For the | ||
| full user-facing display (details, suggestions, issue links) call | ||
| ``format_for_display()`` instead. | ||
| """ | ||
| if self.location and self.location.filename: | ||
| return ( | ||
| f"{self.message} " | ||
| f"(in '{self.location.module_name}', line {self.location.line_number})" | ||
| ) | ||
| return self.message | ||
|
|
||
| def format_for_display( | ||
| self, | ||
| *, | ||
| include_suggestions: bool = True, | ||
| include_details: bool = True, | ||
| include_issue_info: bool = False, | ||
| ) -> str: | ||
| """Format this exception node for user-facing display. | ||
|
|
||
| Unlike ``__str__``, this produces richly formatted output with all | ||
| contextual information attached to *this* exception. When displaying | ||
| an exception chain, call this only on the node you want to show in | ||
| full; use ``include_issue_info=True`` only on the outermost display | ||
| call so that the reporting boilerplate appears exactly once. | ||
|
|
||
| Args: | ||
| include_suggestions: Include the suggestions list. | ||
| include_details: Include the details dict. | ||
| include_issue_info: Append the alpha/issue-reporting boilerplate. | ||
|
|
||
| Returns: | ||
| Formatted string for display to the user. | ||
| """ | ||
| from codeweaver.core.utils.environment import format_file_link | ||
| from codeweaver.core.utils.environment import is_tty as _is_tty | ||
|
|
||
| if _is_tty(): | ||
| location_info = ( | ||
| f"\n[bold red]Encountered error[/bold red] in '{self.location.module_name}' at {format_file_link(self.location.filename, self.location.line_number)}\n" | ||
| if self.location and self.location.filename | ||
| else "" | ||
| ) | ||
| else: | ||
| location_info = ( | ||
| f"\nEncountered error in '{self.location.module_name}' at {format_file_link(self.location.filename, self.location.line_number)}\n" | ||
| if self.location and self.location.filename | ||
| else "" | ||
| ) | ||
| parts: list[str] = [self.message, location_info] | ||
| if self.details: | ||
| tty = _is_tty() | ||
| parts: list[str] = [self.message] | ||
|
|
||
| if self.location and self.location.filename: | ||
| link = format_file_link(self.location.filename, self.location.line_number) | ||
| if tty: | ||
| parts.append( | ||
| f"[bold red]Encountered error[/bold red] in " | ||
| f"'{self.location.module_name}' at {link}" | ||
| ) | ||
| else: | ||
| parts.append(f"Encountered error in '{self.location.module_name}' at {link}") | ||
|
|
||
| if include_details and self.details: | ||
| detail_parts: list[str] = [] | ||
| if "file_path" in self.details: | ||
| detail_parts.append(f"file: {self.details['file_path']}") | ||
|
|
@@ -164,9 +209,44 @@ def __str__(self) -> str: | |
| ) | ||
| if detail_parts: | ||
| parts.append(_get_reporting_info(detail_parts)) | ||
| parts.extend(type(self)._issue_information) | ||
|
|
||
| if include_suggestions and self.suggestions: | ||
| parts.append("\n".join(f" {_BULLET} {s}" for s in self.suggestions)) | ||
|
|
||
| if include_issue_info: | ||
| parts.extend(type(self).issue_information) | ||
|
|
||
| return "\n".join(parts) | ||
|
|
||
| def log_record(self) -> dict[str, Any]: | ||
| """Return a structured record for use with structured logging systems. | ||
|
|
||
| Produces a plain ``dict`` containing all exception data so that | ||
| logging back-ends (structlog, Python logging with a JSON formatter, | ||
|
Comment on lines
+221
to
+225
|
||
| etc.) can emit fully-structured log lines without parsing strings. | ||
|
|
||
| Example:: | ||
|
|
||
| logger.error("Indexing failed", **error.log_record()) | ||
|
|
||
| Returns: | ||
| Dict with ``error_type``, ``message``, ``details``, | ||
| ``suggestions``, and ``location`` keys. | ||
| """ | ||
| return { | ||
| "error_type": type(self).__name__, | ||
| "message": self.message, | ||
| "details": dict(self.details), | ||
| "suggestions": list(self.suggestions), | ||
| "location": { | ||
| "filename": self.location.filename, | ||
| "line_number": self.location.line_number, | ||
| "module_name": self.location.module_name, | ||
| } | ||
| if self.location | ||
| else None, | ||
| } | ||
|
|
||
|
|
||
| class InitializationError(CodeWeaverError): | ||
| """Initialization and startup errors. | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
_BULLETis duplicated here and incodeweaver.core.exceptions(and the PR description implies it’s shared). If you want a single source of truth, import it from the core module or move it to a dedicated constants module; otherwise the two copies can drift and produce inconsistent output.