From d940f67f6fa7a60144d5cc5665607b582498d26b Mon Sep 17 00:00:00 2001 From: Arun Babu Neelicattu Date: Sat, 18 Jan 2025 19:07:54 +0100 Subject: [PATCH 1/3] feat(ux): add framework for runtime error handling --- src/poetry/console/application.py | 9 +- src/poetry/console/exceptions.py | 213 ++++++++++++++++++ .../test_exceptions_console_message.py | 68 ++++++ .../test_exections_poetry_runtime_error.py | 141 ++++++++++++ 4 files changed, 430 insertions(+), 1 deletion(-) create mode 100644 tests/console/test_exceptions_console_message.py create mode 100644 tests/console/test_exections_poetry_runtime_error.py diff --git a/src/poetry/console/application.py b/src/poetry/console/application.py index e217ec56f2f..5f866a31259 100644 --- a/src/poetry/console/application.py +++ b/src/poetry/console/application.py @@ -21,6 +21,7 @@ from poetry.__version__ import __version__ from poetry.console.command_loader import CommandLoader from poetry.console.commands.command import Command +from poetry.console.exceptions import PoetryRuntimeError from poetry.utils.helpers import directory from poetry.utils.helpers import ensure_path @@ -246,9 +247,15 @@ def _run(self, io: IO) -> int: self._load_plugins(io) + exit_code: int = 1 + with directory(self._working_directory): try: - exit_code: int = super()._run(io) + exit_code = super()._run(io) + except PoetryRuntimeError as e: + io.write_error_line("") + e.write(io) + io.write_error_line("") except CleoCommandNotFoundError as e: command = self._get_command_name(io) diff --git a/src/poetry/console/exceptions.py b/src/poetry/console/exceptions.py index d45c38dd3df..6a35c8f97c8 100644 --- a/src/poetry/console/exceptions.py +++ b/src/poetry/console/exceptions.py @@ -1,7 +1,20 @@ from __future__ import annotations +import dataclasses +import shlex + +from dataclasses import InitVar +from subprocess import CalledProcessError +from typing import TYPE_CHECKING + from cleo.exceptions import CleoError +from poetry.utils._compat import decode + + +if TYPE_CHECKING: + from cleo.io.io import IO + class PoetryConsoleError(CleoError): pass @@ -9,3 +22,203 @@ class PoetryConsoleError(CleoError): class GroupNotFoundError(PoetryConsoleError): pass + + +@dataclasses.dataclass +class ConsoleMessage: + """ + Representation of a console message, providing utilities for formatting text + with tags, indentation, and sections. + + The ConsoleMessage class is designed to represent text messages that might be + displayed in a console or terminal output. It provides features for managing + formatted text, such as stripping tags, wrapping text with specific tags, + indenting, and creating structured message sections. + """ + + text: str + debug: bool = False + + @property + def stripped(self) -> str: + from cleo._utils import strip_tags + + return strip_tags(self.text) + + def wrap(self, tag: str) -> ConsoleMessage: + if self.text: + self.text = f"<{tag}>{self.text}" + return self + + def indent(self, indent: str) -> ConsoleMessage: + if self.text: + self.text = f"\n{indent}".join(self.text.splitlines()).strip() + self.text = f"{indent}{self.text}" + return self + + def make_section( + self, + title: str, + indent: str = "", + ) -> ConsoleMessage: + if not self.text: + return self + + if self.text: + section = [f"{title}:"] if title else [] + section.extend(self.text.splitlines()) + self.text = f"\n{indent}".join(section).strip() + + return self + + +@dataclasses.dataclass +class PrettyCalledProcessError: + """ + Represents a formatted and decorated error object for a subprocess call. + + This class is used to encapsulate information about a `CalledProcessError`, + providing additional context such as command output, errors, and helpful + debugging messages. It is particularly useful for wrapping and decorating + subprocess-related exceptions in a more user-friendly format. + + Attributes: + message: A string representation of the exception. + output: A section formatted representation of the exception stdout. + errors: A section formatted representation of the exception stderr. + command_message: Formatted message including a hint on retrying the original command. + command: A `shelex` quoted string representation of the original command. + exception: The original `CalledProcessError` instance. + indent: Indent prefix to use for inner content per section. + """ + + message: ConsoleMessage = dataclasses.field(init=False) + output: ConsoleMessage = dataclasses.field(init=False) + errors: ConsoleMessage = dataclasses.field(init=False) + command_message: ConsoleMessage = dataclasses.field(init=False) + command: str = dataclasses.field(init=False) + exception: InitVar[CalledProcessError] = dataclasses.field(init=True) + indent: InitVar[str] = dataclasses.field(default="") + + def __post_init__(self, exception: CalledProcessError, indent: str = "") -> None: + self.message = ConsoleMessage(str(exception).strip(), debug=True).make_section( + "Exception", indent + ) + self.output = ConsoleMessage(decode(exception.stdout), debug=True).make_section( + "Output", indent + ) + self.errors = ConsoleMessage(decode(exception.stderr), debug=True).make_section( + "Errors", indent + ) + self.command = ( + shlex.join(exception.cmd) + if isinstance(exception.cmd, list) + else exception.cmd + ) + self.command_message = ConsoleMessage( + f"You can test the failed command by executing:\n\n {self.command}", + debug=False, + ) + + +class PoetryRuntimeError(PoetryConsoleError): + """ + Represents a runtime error in the Poetry console application. + """ + + def __init__( + self, + reason: str, + messages: list[ConsoleMessage] | None = None, + exit_code: int = 1, + ) -> None: + super().__init__(reason) + self.exit_code = exit_code + self._messages = messages or [] + self._messages.insert(0, ConsoleMessage(reason)) + + def write(self, io: IO) -> None: + """ + Write the error text to the provided IO iff there is any text + to write. + """ + if text := self.get_text(debug=io.is_verbose(), strip=False): + io.write_error_line(text) + + def get_text( + self, debug: bool = False, indent: str = "", strip: bool = False + ) -> str: + """ + Convert the error messages to a formatted string. All empty messages + are ignored along with debug level messages if `debug` is `False`. + """ + text = "" + has_skipped_debug = False + + for message in self._messages: + if message.debug and not debug: + has_skipped_debug = True + continue + + message_text = message.stripped if strip else message.text + if not message_text: + continue + + if indent: + message_text = f"\n{indent}".join(message_text.splitlines()) + + text += f"{indent}{message_text}\n{indent}\n" + + if has_skipped_debug: + text += f"{indent}You can also run your poetry command with -v to see more information.\n{indent}\n" + + return text.rstrip(f"{indent}\n") + + def __str__(self) -> str: + return self._messages[0].stripped.strip() + + @classmethod + def create( + cls, + reason: str, + exception: CalledProcessError | Exception | None = None, + info: list[str] | str | None = None, + ) -> PoetryRuntimeError: + """ + Create an instance of this class using the provided reason. If + an exception is provided, this is also injected as a debug + `ConsoleMessage`. + + There is specific handling for known exception types. For example, + if exception is of type `subprocess.CalledProcessError`, the following + sections are additionally added when available - stdout, stderr and + command for testing. + """ + if isinstance(info, str): + info = [info] + + messages: list[ConsoleMessage] = [ + ConsoleMessage( + "\n".join(info or []), + debug=False, + ).wrap("info"), + ] + + if isinstance(exception, CalledProcessError): + error = PrettyCalledProcessError(exception, indent=" | ") + messages = [ + error.message.wrap("warning"), + error.output.wrap("warning"), + error.errors.wrap("warning"), + *messages, + error.command_message, + ] + elif exception is not None and isinstance(exception, Exception): + messages.insert( + 0, + ConsoleMessage(str(exception), debug=True).make_section( + "Exception", indent=" | " + ), + ) + + return cls(reason, messages) diff --git a/tests/console/test_exceptions_console_message.py b/tests/console/test_exceptions_console_message.py new file mode 100644 index 00000000000..6c302445459 --- /dev/null +++ b/tests/console/test_exceptions_console_message.py @@ -0,0 +1,68 @@ +from __future__ import annotations + +import pytest + +from poetry.console.exceptions import ConsoleMessage + + +@pytest.mark.parametrize( + ("text", "expected_stripped"), + [ + ("Hello, World!", "Hello, World!"), + ("Bold", "Bold"), + ("Italic", "Italic"), + ], +) +def test_stripped_property(text: str, expected_stripped: str) -> None: + """Test the stripped property with various tagged inputs.""" + message = ConsoleMessage(text) + assert message.stripped == expected_stripped + + +@pytest.mark.parametrize( + ("text", "tag", "expected"), + [ + ("Hello, World!", "info", "Hello, World!"), + ("Error occurred", "error", "Error occurred"), + ("", "info", ""), # Test with empty input + ], +) +def test_wrap(text: str, tag: str, expected: str) -> None: + """Test the wrap method with various inputs.""" + message = ConsoleMessage(text) + assert message.wrap(tag).text == expected + + +@pytest.mark.parametrize( + ("text", "indent", "expected"), + [ + ("Hello, World!", " ", " Hello, World!"), + ("Line 1\nLine 2", ">>", ">>Line 1\n>>Line 2"), + ("", " ", ""), # Test with empty input + (" ", " ", " "), # Test with whitespace input + ], +) +def test_indent(text: str, indent: str, expected: str) -> None: + """Test the indent method with various inputs.""" + message = ConsoleMessage(text) + assert message.indent(indent).text == expected + + +@pytest.mark.parametrize( + ("text", "title", "indent", "expected"), + [ + ("Hello, World!", "Greeting", "", "Greeting:\nHello, World!"), + ( + "This is a message.", + "Section Title", + " ", + "Section Title:\n This is a message.", + ), + ("", "Title", "", ""), # Test with empty text + ("Multi-line\nText", "Title", ">>>", "Title:\n>>>Multi-line\n>>>Text"), + ], +) +def test_make_section(text: str, title: str, indent: str, expected: str) -> None: + """Test the make_section method with various inputs.""" + message = ConsoleMessage(text) + assert message.make_section(title, indent).text == expected diff --git a/tests/console/test_exections_poetry_runtime_error.py b/tests/console/test_exections_poetry_runtime_error.py new file mode 100644 index 00000000000..4b3899b176a --- /dev/null +++ b/tests/console/test_exections_poetry_runtime_error.py @@ -0,0 +1,141 @@ +from __future__ import annotations + +from subprocess import CalledProcessError + +import pytest + +from poetry.console.exceptions import ConsoleMessage +from poetry.console.exceptions import PoetryRuntimeError + + +@pytest.mark.parametrize( + ("reason", "messages", "exit_code", "expected_reason"), + [ + ("Error occurred!", None, 1, "Error occurred!"), # Default scenario + ( + "Specific error", + [ConsoleMessage("Additional details.")], + 2, + "Specific error", + ), # Custom exit code and messages + ("Minimal error", [], 0, "Minimal error"), # No additional messages + ], +) +def test_poetry_runtime_error_init( + reason: str, + messages: list[ConsoleMessage] | None, + exit_code: int, + expected_reason: str, +) -> None: + """Test the basic initialization of the PoetryRuntimeError class.""" + error = PoetryRuntimeError(reason, messages, exit_code) + assert error.exit_code == exit_code + assert str(error) == expected_reason + assert isinstance(error._messages[0], ConsoleMessage) + assert error._messages[0].text == reason + + +@pytest.mark.parametrize( + ("debug", "strip", "indent", "messages", "expected_text"), + [ + ( + False, + False, + "", + [ + ConsoleMessage("Basic message"), + ConsoleMessage("Debug message", debug=True), + ], + "Error\n\nBasic message\n\nYou can also run your poetry command with -v to see more information.", + ), # Debug message ignored + ( + True, + False, + "", + [ + ConsoleMessage("Info message"), + ConsoleMessage("Debug message", debug=True), + ], + "Error\n\nInfo message\n\nDebug message", + ), # Debug message included in verbose mode + ( + True, + True, + "", + [ + ConsoleMessage("Bolded message"), + ConsoleMessage("Debug Italics Message", debug=True), + ], + "Error\n\nBolded message\n\nDebug Italics Message", + ), # Stripped tags and debug message + ( + False, + False, + " ", + [ConsoleMessage("Error occurred!")], + " Error\n \n Error occurred!", + ), # Indented message + ], +) +def test_poetry_runtime_error_get_text( + debug: bool, + strip: bool, + indent: str, + messages: list[ConsoleMessage], + expected_text: str, +) -> None: + """Test the get_text method of PoetryRuntimeError.""" + error = PoetryRuntimeError("Error", messages) + text = error.get_text(debug=debug, strip=strip, indent=indent) + assert text == expected_text + + +@pytest.mark.parametrize( + ("reason", "exception", "info", "expected_message_texts"), + [ + ( + "Command failed", + None, + None, + ["Command failed", ""], # No exception or additional info + ), + ( + "Command failure", + Exception("An exception occurred"), + None, + [ + "Command failure", + "Exception:\n | An exception occurred", + "", + ], # Exception message included + ), + ( + "Subprocess error", + CalledProcessError(1, ["cmd"], b"stdout", b"stderr"), + ["Additional info"], + [ + "Subprocess error", + "Exception:\n" + " | Command '['cmd']' returned non-zero exit status 1.", + "Output:\n" " | stdout", + "Errors:\n" " | stderr", + "Additional info", + "You can test the failed command by executing:\n\n cmd", + ], + ), + ], +) +def test_poetry_runtime_error_create( + reason: str, + exception: Exception, + info: list[str], + expected_message_texts: list[str], +) -> None: + """Test the create class method of PoetryRuntimeError.""" + error = PoetryRuntimeError.create(reason, exception, info) + + assert isinstance(error, PoetryRuntimeError) + assert all(isinstance(msg, ConsoleMessage) for msg in error._messages) + + actual_texts = [msg.text for msg in error._messages] + assert actual_texts == expected_message_texts From e78f434a72ddf234f291a81ab1097c590db32d76 Mon Sep 17 00:00:00 2001 From: Arun Babu Neelicattu Date: Sat, 18 Jan 2025 19:15:48 +0100 Subject: [PATCH 2/3] feat(ux): improve ux for hash not found errors This changes allows better error information propagation to facilitate improved ux for users encountering errors. Resolves: #10057 --- src/poetry/installation/chooser.py | 29 ++++++++++++++++++++++++----- src/poetry/installation/executor.py | 5 +++++ tests/installation/test_chooser.py | 11 +++++++++-- 3 files changed, 38 insertions(+), 7 deletions(-) diff --git a/src/poetry/installation/chooser.py b/src/poetry/installation/chooser.py index 8805c0d2ac3..0beea8774b2 100644 --- a/src/poetry/installation/chooser.py +++ b/src/poetry/installation/chooser.py @@ -8,6 +8,8 @@ from poetry.config.config import Config from poetry.config.config import PackageFilterPolicy +from poetry.console.exceptions import ConsoleMessage +from poetry.console.exceptions import PoetryRuntimeError from poetry.repositories.http_repository import HTTPRepository from poetry.utils.helpers import get_highest_priority_hash_type from poetry.utils.wheel import Wheel @@ -134,11 +136,28 @@ def _get_links(self, package: Package) -> list[Link]: selected_links.append(link) if links and not selected_links: - links_str = ", ".join(f"{link}({h})" for link, h in skipped) - raise RuntimeError( - f"Retrieved digests for links {links_str} not in poetry.lock" - f" metadata {locked_hashes}" - ) + reason = f"Downloaded distributions for {package.pretty_name} ({package.pretty_version}) did not match any known checksums in your lock file." + link_hashes = "\n".join(f" - {link}({h})" for link, h in skipped) + known_hashes = "\n".join(f" - {h}" for h in locked_hashes) + messages = [ + ConsoleMessage( + "Causes:\n" + " - invalid or corrupt cache either during locking or installation\n" + " - network interruptions or errors causing corrupted downloads\n\n" + "Solutions:\n" + " 1. Try running your command again using the --no-cache global option enabled.\n" + " 2. Try regenerating your lock file using (poetry lock --no-cache --regenerate).\n\n" + "If any of those solutions worked, you will have to clear your caches using (poetry cache clear --all CACHE_NAME)." + ), + ConsoleMessage( + f"Poetry retrieved the following links:\n" + f"{link_hashes}\n\n" + f"The lockfile contained only the following hashes:\n" + f"{known_hashes}", + debug=True, + ), + ] + raise PoetryRuntimeError(reason, messages) return selected_links diff --git a/src/poetry/installation/executor.py b/src/poetry/installation/executor.py index 8e4ff0f31d4..8f28e1a9677 100644 --- a/src/poetry/installation/executor.py +++ b/src/poetry/installation/executor.py @@ -16,6 +16,7 @@ from poetry.core.packages.utils.link import Link +from poetry.console.exceptions import PoetryRuntimeError from poetry.installation.chef import Chef from poetry.installation.chooser import Chooser from poetry.installation.operations import Install @@ -333,6 +334,10 @@ def _execute_operation(self, operation: Operation) -> None: f" for {pkg.pretty_name}." "" ) + elif isinstance(e, PoetryRuntimeError): + message = e.get_text(io.is_verbose(), indent=" | ").rstrip() + message = f"{message}" + with_trace = False else: message = f"Cannot install {pkg.pretty_name}." diff --git a/tests/installation/test_chooser.py b/tests/installation/test_chooser.py index dd0aa7baa97..cfb5a74d8d7 100644 --- a/tests/installation/test_chooser.py +++ b/tests/installation/test_chooser.py @@ -8,6 +8,7 @@ from packaging.tags import Tag from poetry.core.packages.package import Package +from poetry.console.exceptions import PoetryRuntimeError from poetry.installation.chooser import Chooser from poetry.repositories.legacy_repository import LegacyRepository from poetry.repositories.pypi_repository import PyPiRepository @@ -366,9 +367,15 @@ def test_chooser_throws_an_error_if_package_hashes_do_not_match( package.files = files - with pytest.raises(RuntimeError) as e: + with pytest.raises(PoetryRuntimeError) as e: chooser.choose_for(package) - assert files[0]["hash"] in str(e) + + reason = f"Downloaded distributions for {package.name} ({package.version}) did not match any known checksums in your lock file." + assert str(e.value) == reason + + text = e.value.get_text(debug=True, strip=True) + assert reason in text + assert files[0]["hash"] in text def test_chooser_md5_remote_fallback_to_sha256_inline_calculation( From bccd8431141a375d3a7ca1facb31d825a4f5fb9c Mon Sep 17 00:00:00 2001 From: Arun Babu Neelicattu Date: Sat, 18 Jan 2025 19:17:04 +0100 Subject: [PATCH 3/3] feat(ux): improve ux for git specific errors --- src/poetry/vcs/git/backend.py | 87 +++++++++++++++++++++++++++++------ src/poetry/vcs/git/system.py | 7 +-- 2 files changed, 76 insertions(+), 18 deletions(-) diff --git a/src/poetry/vcs/git/backend.py b/src/poetry/vcs/git/backend.py index 5f20b3888f9..79ba3f7473d 100644 --- a/src/poetry/vcs/git/backend.py +++ b/src/poetry/vcs/git/backend.py @@ -21,7 +21,7 @@ from dulwich.refs import ANNOTATED_TAG_SUFFIX from dulwich.repo import Repo -from poetry.console.exceptions import PoetryConsoleError +from poetry.console.exceptions import PoetryRuntimeError from poetry.utils.authenticator import get_default_authenticator from poetry.utils.helpers import remove_directory @@ -36,6 +36,30 @@ # A relative URL by definition starts with ../ or ./ RELATIVE_SUBMODULE_REGEX = re.compile(r"^\.{1,2}/") +# Common error messages +ERROR_MESSAGE_NOTE = ( + "Note: This error arises from interacting with " + "the specified vcs source and is likely not a " + "Poetry issue." +) +ERROR_MESSAGE_PROBLEMS_SECTION_START = ( + "This issue could be caused by any of the following;\n\n" + "- there are network issues in this environment" +) +ERROR_MESSAGE_BAD_REVISION = ( + "- the revision ({revision}) you have specified\n" + " - was misspelled\n" + " - is invalid (must be a sha or symref)\n" + " - is not present on remote" +) +ERROR_MESSAGE_BAD_REMOTE = ( + "- the remote ({remote}) you have specified\n" + " - was misspelled\n" + " - does not exist\n" + " - requires credentials that were either not configured or is incorrect\n" + " - is in accessible due to network issues" +) + def is_revision_sha(revision: str | None) -> bool: return re.match(r"^\b[0-9a-f]{5,40}\b$", revision or "") is not None @@ -236,10 +260,15 @@ def _clone_legacy(url: str, refspec: GitRefSpec, target: Path) -> Repo: try: SystemGit.clone(url, target) - except CalledProcessError: - raise PoetryConsoleError( - f"Failed to clone {url}, check your git configuration and permissions" - " for this repository." + except CalledProcessError as e: + raise PoetryRuntimeError.create( + reason=f"Failed to clone {url}, check your git configuration and permissions for this repository.", + exception=e, + info=[ + ERROR_MESSAGE_NOTE, + ERROR_MESSAGE_PROBLEMS_SECTION_START, + ERROR_MESSAGE_BAD_REMOTE.format(remote=url), + ], ) if revision: @@ -248,8 +277,16 @@ def _clone_legacy(url: str, refspec: GitRefSpec, target: Path) -> Repo: try: SystemGit.checkout(revision, target) - except CalledProcessError: - raise PoetryConsoleError(f"Failed to checkout {url} at '{revision}'") + except CalledProcessError as e: + raise PoetryRuntimeError.create( + reason=f"Failed to checkout {url} at '{revision}'.", + exception=e, + info=[ + ERROR_MESSAGE_NOTE, + ERROR_MESSAGE_PROBLEMS_SECTION_START, + ERROR_MESSAGE_BAD_REVISION.format(revision=revision), + ], + ) repo = Repo(str(target)) return repo @@ -276,13 +313,28 @@ def _clone(cls, url: str, refspec: GitRefSpec, target: Path) -> Repo: try: refspec.resolve(remote_refs=remote_refs, repo=local) except KeyError: # branch / ref does not exist - raise PoetryConsoleError( - f"Failed to clone {url} at '{refspec.key}', verify ref exists on" - " remote." + raise PoetryRuntimeError.create( + reason=f"Failed to clone {url} at '{refspec.key}', verify ref exists on remote.", + info=[ + ERROR_MESSAGE_NOTE, + ERROR_MESSAGE_PROBLEMS_SECTION_START, + ERROR_MESSAGE_BAD_REVISION.format(revision=refspec.key), + ], ) - # ensure local HEAD matches remote - local.refs[b"HEAD"] = remote_refs.refs[b"HEAD"] + try: + # ensure local HEAD matches remote + local.refs[b"HEAD"] = remote_refs.refs[b"HEAD"] + except ValueError: + raise PoetryRuntimeError.create( + reason=f"Failed to clone {url} at '{refspec.key}', verify ref exists on remote.", + info=[ + ERROR_MESSAGE_NOTE, + ERROR_MESSAGE_PROBLEMS_SECTION_START, + ERROR_MESSAGE_BAD_REVISION.format(revision=refspec.key), + f"\nThis particular error is prevalent when {refspec.key} could not be resolved to a specific commit sha.", + ], + ) if refspec.is_ref: # set ref to current HEAD @@ -318,9 +370,14 @@ def _clone(cls, url: str, refspec: GitRefSpec, target: Path) -> Repo: if isinstance(e, AssertionError) and "Invalid object name" not in str(e): raise - raise PoetryConsoleError( - f"Failed to clone {url} at '{refspec.key}', verify ref exists on" - " remote." + raise PoetryRuntimeError.create( + reason=f"Failed to clone {url} at '{refspec.key}', verify ref exists on remote.", + info=[ + ERROR_MESSAGE_NOTE, + ERROR_MESSAGE_PROBLEMS_SECTION_START, + ERROR_MESSAGE_BAD_REVISION.format(revision=refspec.key), + ], + exception=e, ) return local diff --git a/src/poetry/vcs/git/system.py b/src/poetry/vcs/git/system.py index 2d0ffc761f3..9a36c4fef74 100644 --- a/src/poetry/vcs/git/system.py +++ b/src/poetry/vcs/git/system.py @@ -40,13 +40,14 @@ def run(*args: Any, **kwargs: Any) -> None: git_command = find_git_command() env = os.environ.copy() env["GIT_TERMINAL_PROMPT"] = "0" - subprocess.check_call( + + subprocess.run( git_command + list(args), - stderr=subprocess.DEVNULL, - stdout=subprocess.DEVNULL, + capture_output=True, env=env, text=True, encoding="utf-8", + check=True, ) @staticmethod