mirror of
https://github.com/pypa/pip
synced 2023-12-13 21:30:23 +01:00
Introduce DiagnosticPipError
This introduces an exception and presentation model, for providing better errors messages. The motivating idea is that the better error messages contain clear wording and provide additional context to users to better understand what is happening. The `DiagnosticPipError` class introduces a structured framework in our exception model, for code authors to write their error messages. The usage explicitly requires passing "context" and a "hint" (which accept None values). This should nudge code authors to explicitly think about what additional information can/should be presented to the user, and to provide relevant hints to them whenever possible. It also makes it straightforward to identify cases where we don't do this, which may serve as clear areas for improvement in the future. The initial implementation is intentionally basic and doesn't do much; however we should be able to introduce better usage of terminal colors and other features (eg: hyperlinks!) to further improve the presentation of these errors. It does improve the presentation style by a bit, even though there are significant presentation-related improvements to be made. Additionally, having a structured framework means that these would be improvements in presentation of *all* the errors that are within this framework -- increasing the benefits of investing in the presentation of these errors.
This commit is contained in:
parent
7b3c12299b
commit
85b43482bb
|
@ -22,6 +22,7 @@ from pip._internal.cli.status_codes import (
|
|||
from pip._internal.exceptions import (
|
||||
BadCommand,
|
||||
CommandError,
|
||||
DiagnosticPipError,
|
||||
InstallationError,
|
||||
NetworkConnectionError,
|
||||
PreviousBuildDirError,
|
||||
|
@ -169,6 +170,11 @@ class Command(CommandContextMixIn):
|
|||
logger.debug("Exception information:", exc_info=True)
|
||||
|
||||
return PREVIOUS_BUILD_DIR_ERROR
|
||||
except DiagnosticPipError as exc:
|
||||
logger.critical(str(exc))
|
||||
logger.debug("Exception information:", exc_info=True)
|
||||
|
||||
return ERROR
|
||||
except (
|
||||
InstallationError,
|
||||
UninstallationError,
|
||||
|
|
|
@ -1,23 +1,102 @@
|
|||
"""Exceptions used throughout package"""
|
||||
|
||||
import configparser
|
||||
import re
|
||||
from itertools import chain, groupby, repeat
|
||||
from typing import TYPE_CHECKING, Dict, List, Optional, Union
|
||||
from typing import TYPE_CHECKING, Dict, Iterator, List, Optional, Union
|
||||
|
||||
from pip._vendor.pkg_resources import Distribution
|
||||
from pip._vendor.requests.models import Request, Response
|
||||
|
||||
if TYPE_CHECKING:
|
||||
from hashlib import _Hash
|
||||
from typing import Literal
|
||||
|
||||
from pip._internal.metadata import BaseDistribution
|
||||
from pip._internal.req.req_install import InstallRequirement
|
||||
|
||||
|
||||
#
|
||||
# Scaffolding
|
||||
#
|
||||
def _is_kebab_case(s: str) -> bool:
|
||||
return re.match(r"^[a-z]+(-[a-z]+)*$", s) is not None
|
||||
|
||||
|
||||
def _prefix_with_indent(prefix: str, s: str, indent: Optional[str] = None) -> str:
|
||||
if indent is None:
|
||||
indent = " " * len(prefix)
|
||||
else:
|
||||
assert len(indent) == len(prefix)
|
||||
message = s.replace("\n", "\n" + indent)
|
||||
return f"{prefix}{message}\n"
|
||||
|
||||
|
||||
class PipError(Exception):
|
||||
"""Base pip exception"""
|
||||
"""The base pip error."""
|
||||
|
||||
|
||||
class DiagnosticPipError(PipError):
|
||||
"""A pip error, that presents diagnostic information to the user.
|
||||
|
||||
This contains a bunch of logic, to enable pretty presentation of our error
|
||||
messages. Each error gets a unique reference. Each error can also include
|
||||
additional context, a hint and/or a note -- which are presented with the
|
||||
main erorr message in a consistent style.
|
||||
"""
|
||||
|
||||
reference: str
|
||||
|
||||
def __init__(
|
||||
self,
|
||||
*,
|
||||
message: str,
|
||||
context: Optional[str],
|
||||
hint_stmt: Optional[str],
|
||||
attention_stmt: Optional[str] = None,
|
||||
reference: Optional[str] = None,
|
||||
kind: 'Literal["error", "warning"]' = "error",
|
||||
) -> None:
|
||||
|
||||
# Ensure a proper reference is provided.
|
||||
if reference is None:
|
||||
assert hasattr(self, "reference"), "error reference not provided!"
|
||||
reference = self.reference
|
||||
assert _is_kebab_case(reference), "error reference must be kebab-case!"
|
||||
|
||||
super().__init__(f"{reference}: {message}")
|
||||
|
||||
self.kind = kind
|
||||
self.message = message
|
||||
self.context = context
|
||||
|
||||
self.reference = reference
|
||||
self.attention_stmt = attention_stmt
|
||||
self.hint_stmt = hint_stmt
|
||||
|
||||
def __str__(self) -> str:
|
||||
return "".join(self._string_parts())
|
||||
|
||||
def _string_parts(self) -> Iterator[str]:
|
||||
# Present the main message, with relevant context indented.
|
||||
yield f"{self.message}\n"
|
||||
if self.context is not None:
|
||||
yield f"\n{self.context}\n"
|
||||
|
||||
# Space out the note/hint messages.
|
||||
if self.attention_stmt is not None or self.hint_stmt is not None:
|
||||
yield "\n"
|
||||
|
||||
if self.attention_stmt is not None:
|
||||
yield _prefix_with_indent("Note: ", self.attention_stmt)
|
||||
|
||||
if self.hint_stmt is not None:
|
||||
yield _prefix_with_indent("Hint: ", self.hint_stmt)
|
||||
|
||||
|
||||
#
|
||||
# Actual Errors
|
||||
#
|
||||
class ConfigurationError(PipError):
|
||||
"""General exception in configuration"""
|
||||
|
||||
|
|
213
tests/unit/test_exceptions.py
Normal file
213
tests/unit/test_exceptions.py
Normal file
|
@ -0,0 +1,213 @@
|
|||
"""Tests the presentation style of exceptions."""
|
||||
|
||||
import textwrap
|
||||
|
||||
import pytest
|
||||
|
||||
from pip._internal.exceptions import DiagnosticPipError
|
||||
|
||||
|
||||
class TestDiagnosticPipErrorCreation:
|
||||
def test_fails_without_reference(self) -> None:
|
||||
class DerivedError(DiagnosticPipError):
|
||||
pass
|
||||
|
||||
with pytest.raises(AssertionError) as exc_info:
|
||||
DerivedError(message="", context=None, hint_stmt=None)
|
||||
|
||||
assert str(exc_info.value) == "error reference not provided!"
|
||||
|
||||
def test_can_fetch_reference_from_subclass(self) -> None:
|
||||
class DerivedError(DiagnosticPipError):
|
||||
reference = "subclass-reference"
|
||||
|
||||
obj = DerivedError(message="", context=None, hint_stmt=None)
|
||||
assert obj.reference == "subclass-reference"
|
||||
|
||||
def test_can_fetch_reference_from_arguments(self) -> None:
|
||||
class DerivedError(DiagnosticPipError):
|
||||
pass
|
||||
|
||||
obj = DerivedError(
|
||||
message="", context=None, hint_stmt=None, reference="subclass-reference"
|
||||
)
|
||||
assert obj.reference == "subclass-reference"
|
||||
|
||||
@pytest.mark.parametrize(
|
||||
"name",
|
||||
[
|
||||
"BADNAME",
|
||||
"BadName",
|
||||
"bad_name",
|
||||
"BAD_NAME",
|
||||
"_bad",
|
||||
"bad-name-",
|
||||
"bad--name",
|
||||
"-bad-name",
|
||||
"bad-name-due-to-1-number",
|
||||
],
|
||||
)
|
||||
def test_rejects_non_kebab_case_names(self, name: str) -> None:
|
||||
class DerivedError(DiagnosticPipError):
|
||||
reference = name
|
||||
|
||||
with pytest.raises(AssertionError) as exc_info:
|
||||
DerivedError(message="", context=None, hint_stmt=None)
|
||||
|
||||
assert str(exc_info.value) == "error reference must be kebab-case!"
|
||||
|
||||
|
||||
class TestDiagnosticPipErrorPresentation_ASCII:
|
||||
def test_complete(self) -> None:
|
||||
err = DiagnosticPipError(
|
||||
reference="test-diagnostic",
|
||||
message="Oh no!\nIt broke. :(",
|
||||
context="Something went wrong\nvery wrong.",
|
||||
attention_stmt="You did something wrong, which is what caused this error.",
|
||||
hint_stmt="Do it better next time, by trying harder.",
|
||||
)
|
||||
|
||||
assert str(err) == textwrap.dedent(
|
||||
"""\
|
||||
Oh no!
|
||||
It broke. :(
|
||||
|
||||
Something went wrong
|
||||
very wrong.
|
||||
|
||||
Note: You did something wrong, which is what caused this error.
|
||||
Hint: Do it better next time, by trying harder.
|
||||
"""
|
||||
)
|
||||
|
||||
def test_no_context(self) -> None:
|
||||
err = DiagnosticPipError(
|
||||
reference="test-diagnostic",
|
||||
message="Oh no!\nIt broke. :(",
|
||||
context=None,
|
||||
attention_stmt="You did something wrong, which is what caused this error.",
|
||||
hint_stmt="Do it better next time, by trying harder.",
|
||||
)
|
||||
|
||||
assert str(err) == textwrap.dedent(
|
||||
"""\
|
||||
Oh no!
|
||||
It broke. :(
|
||||
|
||||
Note: You did something wrong, which is what caused this error.
|
||||
Hint: Do it better next time, by trying harder.
|
||||
"""
|
||||
)
|
||||
|
||||
def test_no_note(self) -> None:
|
||||
err = DiagnosticPipError(
|
||||
reference="test-diagnostic",
|
||||
message="Oh no!\nIt broke. :(",
|
||||
context="Something went wrong\nvery wrong.",
|
||||
attention_stmt=None,
|
||||
hint_stmt="Do it better next time, by trying harder.",
|
||||
)
|
||||
|
||||
assert str(err) == textwrap.dedent(
|
||||
"""\
|
||||
Oh no!
|
||||
It broke. :(
|
||||
|
||||
Something went wrong
|
||||
very wrong.
|
||||
|
||||
Hint: Do it better next time, by trying harder.
|
||||
"""
|
||||
)
|
||||
|
||||
def test_no_hint(self) -> None:
|
||||
err = DiagnosticPipError(
|
||||
reference="test-diagnostic",
|
||||
message="Oh no!\nIt broke. :(",
|
||||
context="Something went wrong\nvery wrong.",
|
||||
attention_stmt="You did something wrong, which is what caused this error.",
|
||||
hint_stmt=None,
|
||||
)
|
||||
|
||||
assert str(err) == textwrap.dedent(
|
||||
"""\
|
||||
Oh no!
|
||||
It broke. :(
|
||||
|
||||
Something went wrong
|
||||
very wrong.
|
||||
|
||||
Note: You did something wrong, which is what caused this error.
|
||||
"""
|
||||
)
|
||||
|
||||
def test_no_context_no_hint(self) -> None:
|
||||
err = DiagnosticPipError(
|
||||
reference="test-diagnostic",
|
||||
message="Oh no!\nIt broke. :(",
|
||||
context=None,
|
||||
attention_stmt="You did something wrong, which is what caused this error.",
|
||||
hint_stmt=None,
|
||||
)
|
||||
|
||||
assert str(err) == textwrap.dedent(
|
||||
"""\
|
||||
Oh no!
|
||||
It broke. :(
|
||||
|
||||
Note: You did something wrong, which is what caused this error.
|
||||
"""
|
||||
)
|
||||
|
||||
def test_no_context_no_note(self) -> None:
|
||||
err = DiagnosticPipError(
|
||||
reference="test-diagnostic",
|
||||
message="Oh no!\nIt broke. :(",
|
||||
context=None,
|
||||
attention_stmt=None,
|
||||
hint_stmt="Do it better next time, by trying harder.",
|
||||
)
|
||||
|
||||
assert str(err) == textwrap.dedent(
|
||||
"""\
|
||||
Oh no!
|
||||
It broke. :(
|
||||
|
||||
Hint: Do it better next time, by trying harder.
|
||||
"""
|
||||
)
|
||||
|
||||
def test_no_hint_no_note(self) -> None:
|
||||
err = DiagnosticPipError(
|
||||
reference="test-diagnostic",
|
||||
message="Oh no!\nIt broke. :(",
|
||||
context="Something went wrong\nvery wrong.",
|
||||
attention_stmt=None,
|
||||
hint_stmt=None,
|
||||
)
|
||||
|
||||
assert str(err) == textwrap.dedent(
|
||||
"""\
|
||||
Oh no!
|
||||
It broke. :(
|
||||
|
||||
Something went wrong
|
||||
very wrong.
|
||||
"""
|
||||
)
|
||||
|
||||
def test_no_hint_no_note_no_context(self) -> None:
|
||||
err = DiagnosticPipError(
|
||||
reference="test-diagnostic",
|
||||
message="Oh no!\nIt broke. :(",
|
||||
context=None,
|
||||
hint_stmt=None,
|
||||
attention_stmt=None,
|
||||
)
|
||||
|
||||
assert str(err) == textwrap.dedent(
|
||||
"""\
|
||||
Oh no!
|
||||
It broke. :(
|
||||
"""
|
||||
)
|
Loading…
Reference in a new issue