Skip to content
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

feat(ux): add error better ux for hash not found #10065

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
54 changes: 54 additions & 0 deletions src/poetry/console/exceptions.py
Original file line number Diff line number Diff line change
@@ -1,11 +1,65 @@
from __future__ import annotations

import dataclasses

from typing import TYPE_CHECKING

from cleo.exceptions import CleoError


if TYPE_CHECKING:
from cleo.io.io import IO


class PoetryConsoleError(CleoError):
pass


class GroupNotFoundError(PoetryConsoleError):
pass


@dataclasses.dataclass
class ConsoleMessage:
text: str
debug: bool = False

@property
def stripped(self) -> str:
from cleo._utils import strip_tags

return strip_tags(self.text)


class PoetryRuntimeError(PoetryConsoleError):
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 + ("\n" if messages else "")))

def write(self, io: IO) -> None:
io.write_error_line(self.get_text(debug=io.is_verbose(), strip=False))

def get_text(
self, debug: bool = False, indent: str = "", strip: bool = False
) -> str:
text = ""
for message in self._messages:
if message.debug and not debug:
continue

message_text = message.stripped if strip else message.text
if indent:
message_text = f"\n{indent}".join(message_text.splitlines())
text += f"{indent}{message_text}\n{indent}\n"

return text.rstrip(f"{indent}\n")

def __str__(self) -> str:
return self._messages[0].stripped.strip()
29 changes: 24 additions & 5 deletions src/poetry/installation/chooser.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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 <b>{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(
"<options=bold>Causes:</>\n"
" - invalid or corrupt cache either during locking or installation\n"
" - network interruptions or errors causing corrupted downloads\n\n"
"<b>Solutions:</>\n"
" 1. Try running your command again using the <c1>--no-cache</> global option enabled.\n"
" 2. Try regenerating your lock file using (<c1>poetry lock --no-cache --regenerate</>).\n\n"
"If any of those solutions worked, you will have to clear your caches using (<c1>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

Expand Down
5 changes: 5 additions & 0 deletions src/poetry/installation/executor.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -333,6 +334,10 @@ def _execute_operation(self, operation: Operation) -> None:
f" for {pkg.pretty_name}."
"</error>"
)
elif isinstance(e, PoetryRuntimeError):
sourcery-ai[bot] marked this conversation as resolved.
Show resolved Hide resolved
message = e.get_text(io.is_verbose(), indent=" | ").rstrip()
message = f"<warning>{message}</>"
with_trace = False
else:
message = f"<error>Cannot install {pkg.pretty_name}.</error>"

Expand Down
11 changes: 9 additions & 2 deletions tests/installation/test_chooser.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Comment on lines +373 to +378
Copy link

Choose a reason for hiding this comment

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

suggestion (testing): Test both debug and non-debug text.

It would be beneficial to also test the non-debug version of the error message to ensure that the correct information is displayed to users in both verbose and non-verbose modes.

Suggested change
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
reason = f"Downloaded distributions for {package.name} ({package.version}) did not match any known checksums in your lock file."
assert str(e.value) == reason
# Test non-debug error message
text = e.value.get_text(debug=False, strip=True)
assert reason in text
assert files[0]["hash"] not in text # Hash should not be included in non-debug output
# Test debug error message
debug_text = e.value.get_text(debug=True, strip=True)
assert reason in debug_text
assert files[0]["hash"] in debug_text # Hash should be included in debug output



Comment on lines +376 to 380
Copy link

Choose a reason for hiding this comment

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

suggestion (testing): Add tests for multiple mismatched hashes.

Consider adding a test case where multiple files have mismatched hashes to ensure the error message handles this scenario correctly. The current tests only cover a single mismatch.

Suggested change
text = e.value.get_text(debug=True, strip=True)
assert reason in text
assert files[0]["hash"] in text
text = e.value.get_text(debug=True, strip=True)
assert reason in text
assert files[0]["hash"] in text
def test_chooser_with_multiple_mismatched_hashes(package, chooser):
files = [
{
"name": "demo-0.1.0.tar.gz",
"hash": "incorrect_hash_1",
"url": "https://files.pythonhosted.org/demo-0.1.0.tar.gz",
},
{
"name": "demo-0.1.0-py2.py3-none-any.whl",
"hash": "incorrect_hash_2",
"url": "https://files.pythonhosted.org/demo-0.1.0-py2.py3-none-any.whl",
}
]
package.files = files
with pytest.raises(PoetryRuntimeError) as e:
chooser.choose_for(package)
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
# Verify both mismatched hashes are mentioned in the error
assert files[0]["hash"] in text
assert files[1]["hash"] in text

def test_chooser_md5_remote_fallback_to_sha256_inline_calculation(
Expand Down
Loading