Skip to content

errors: Centralize user-facing messages #183

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.

Sign up for GitHub

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

Draft
BenWestgate wants to merge 1 commit into from
Draft

errors: Centralize user-facing messages #183

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
82 changes: 11 additions & 71 deletions src/codex32/_cli_input.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@

from codex32.bip93 import (
MS,
TEXT_LENGTHS,
MasterSeed,
Secret,
Share,
Expand All @@ -30,17 +29,10 @@
InvalidChecksum,
InvalidCorrectionInput,
InvalidLength,
InvalidThreshold,
MismatchedHrp,
MismatchedIdentifier,
MismatchedPayloadLength,
MismatchedThreshold,
SecretInRecoverySet,
)

Artifact = Share | Secret
_MAX_INPUT = 9 * 1025
_MAX_CORRECTION_LENGTH_DELTA = 8

_line_editor: Any
try:
Expand Down Expand Up @@ -237,17 +229,16 @@ def _render_groups(
def _parse(value: str, hrp: str | None) -> Artifact:
try:
artifact = parse_codex32(value)
except InvalidChecksum as error:
# The parser checks checksums before master-seed lengths; explain a
# wrong length without changing that order.
try:
_validate_ms_length(value[: value.rfind("1")].lower(), len(value))
except InvalidLength as length_error:
raise InputError(str(length_error)) from error
raise InputError(str(error)) from error
except CodexError as error:
message = _FRIENDLY_SET_ERRORS.get(type(error), str(error))
if isinstance(error, InvalidChecksum):
# Parsing has already checked the container and generic length.
# Explain an unsupported master-seed length without validating input
# or changing the parser's checksum-before-length boundary.
try:
_validate_ms_length(value[: value.rfind("1")].lower(), len(value))
except InvalidLength as length_error:
message = str(length_error)
raise InputError(message) from error
raise InputError(str(error)) from error
if hrp is not None and artifact.hrp != hrp:
raise InputError("This command accepts only Bitcoin master seed input.")
return artifact
Expand Down Expand Up @@ -439,52 +430,6 @@ def allowed(candidate: CorrectionCandidate) -> bool:
return accepted


_FRIENDLY_SET_ERRORS: dict[type[Exception], str] = {
InvalidChecksum: "The checksum does not match.",
MismatchedHrp: "These strings are for different applications.",
MismatchedThreshold: "These strings require different numbers of shares.",
MismatchedIdentifier: "These strings have different identifiers.",
MismatchedPayloadLength: "These strings have different lengths.",
DuplicateShareIndex: "That share index was already entered.",
SecretInRecoverySet: "Enter ordinary shares rather than the shared secret.",
}


def _prefixed_input_error(
error: InputError,
value: str,
prefix: str,
accepted: list[Artifact],
) -> str:
cause = error.__cause__
if isinstance(cause, InvalidThreshold) and prefix.lower() == "ms1":
found = value[len(prefix) : len(prefix) + 1].lower()
if found:
return f"After the prefilled {prefix}, enter a threshold of 0 or 2 through 9; found {found!r}."
if not isinstance(cause, InvalidChecksum):
return str(error)
if accepted:
if accepted[0].hrp != MS:
return str(error)
target = len(accepted[0].text)
difference = len(value) - target
if abs(difference) <= _MAX_CORRECTION_LENGTH_DELTA:
return str(error)
direction = "too long for" if difference > 0 else "short of"
return (
f"The string is {abs(difference)} characters {direction} the required {target}-character length."
)
if prefix.lower() != "ms1":
return str(error)
if min(abs(len(value) - target) for target in TEXT_LENGTHS) <= _MAX_CORRECTION_LENGTH_DELTA:
return str(error)
lengths = ", ".join(str(length) for length in TEXT_LENGTHS[:-1]) + f", or {TEXT_LENGTHS[-1]}"
return (
f"The string has {len(value)} characters; valid Bitcoin master-seed codex32 lengths are "
f"{lengths} characters."
)


def _interactive(
*,
basis: bool,
Expand Down Expand Up @@ -552,19 +497,14 @@ def allowed(candidate: CorrectionCandidate) -> bool:
if confirmation is True:
artifact = candidates[0].artifact
else:
message = (
_prefixed_input_error(error, complete_value, prefix, accepted)
if supplied_prefix
else str(error)
)
_stderr(f"Rejected: {message}\n")
_stderr(f"Rejected: {error}\n")
prefill = _retry_text(entered, prefix)
prefix = _prefix_for_suffix(prefix, prefill)
continue
try:
validate(artifact)
except CodexError as error:
_stderr(f"Rejected: {_FRIENDLY_SET_ERRORS.get(type(error), str(error))}\n")
_stderr(f"Rejected: {error}\n")
duplicate = isinstance(error, (DuplicateShareIndex, ExistingTargetIndex))
prefill = "" if duplicate else _retry_text(entered, prefix)
continue
Expand Down
18 changes: 9 additions & 9 deletions src/codex32/bip93.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,7 @@ def _decode_codex32(text: str) -> tuple[str, tuple[int, ...], _Checksum]:
body = tuple(encoded[: -checksum.length])
Header._from_symbols(body[:6])
if not bech32_verify_checksum(hrp, encoded, checksum):
raise InvalidChecksum(f"invalid {checksum.kind} checksum")
raise InvalidChecksum("The checksum does not match.")
_validate_ms_length(hrp, len(text))
return hrp, tuple(encoded), checksum

Expand Down Expand Up @@ -294,31 +294,31 @@ def _validate_share_set(artifacts: Sequence[Share | Secret], *, require_exact: b
first = copied[0]
threshold = first.header.threshold
if threshold not in range(2, 10):
raise MismatchedThreshold("linear sharing requires threshold 2 through 9")
raise MismatchedThreshold("Only strings with a threshold of 2 through 9 can be combined.")
if len(copied) > threshold or (require_exact and len(copied) != threshold):
raise WrongShareCount(f"threshold is {threshold}, but {len(copied)} artifacts were supplied")
first_tail, checksum_length, encoded_length = _artifact_tail(first)
tails = [first_tail]
indices = [first.header.index]
for item in copied[1:]:
if item.hrp != first.hrp:
raise MismatchedHrp(f"{first.hrp} and {item.hrp} cannot be combined")
raise MismatchedHrp("These strings are for different applications.")
if item.header.threshold != threshold:
raise MismatchedThreshold("share thresholds do not match")
raise MismatchedThreshold("These strings require different numbers of shares.")
if item.header.identifier != first.header.identifier:
raise MismatchedIdentifier("share identifiers do not match")
raise MismatchedIdentifier("These strings have different identifiers.")
tail, item_checksum_length, item_encoded_length = _artifact_tail(item)
item_shape = (
len(item.payload_symbols),
item_checksum_length,
item_encoded_length,
)
if item_shape != (len(first.payload_symbols), checksum_length, encoded_length):
raise MismatchedPayloadLength("share payload/checksum lengths do not match")
raise MismatchedPayloadLength("These strings have different lengths.")
tails.append(tail)
indices.append(item.header.index)
if len(set(indices)) != len(indices):
raise DuplicateShareIndex("share indices must be distinct")
raise DuplicateShareIndex("That share index was already entered.")
return _ShareSet(
copied,
first.hrp,
Expand All @@ -332,7 +332,7 @@ def _validate_share_set(artifacts: Sequence[Share | Secret], *, require_exact: b
def _validate_recovery_prefix(shares: Sequence[Share | Secret]) -> None:
share_set = _validate_share_set(shares, require_exact=False)
if any(isinstance(item, Secret) for item in share_set.artifacts):
raise SecretInRecoverySet("recovery accepts ordinary shares only")
raise SecretInRecoverySet("Enter ordinary shares rather than the shared secret.")


def _interpolate_tail(share_set: _ShareSet, target: str) -> Share | Secret:
Expand All @@ -353,7 +353,7 @@ def recover_secret(shares: Sequence[Share]) -> Secret:
"""Recover S from exactly k compatible, distinct ordinary shares."""
share_set = _validate_share_set(shares)
if any(isinstance(item, Secret) for item in share_set.artifacts):
raise SecretInRecoverySet("recovery accepts ordinary shares only")
raise SecretInRecoverySet("Enter ordinary shares rather than the shared secret.")
recovered = _interpolate_tail(share_set, "s")
if not isinstance(recovered, Secret):
raise InvalidShareSet("interpolation did not produce a secret")
Expand Down
18 changes: 7 additions & 11 deletions tests/test_cli.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -579,9 +579,9 @@ def test_xprv_explains_prefilled_threshold_and_uncorrectable_length(monkeypatch,
assert ms_main(["xprv"]) == 0
rejected = [line for line in capsys.readouterr().err.splitlines() if line.startswith("Rejected:")]
assert rejected == [
"Rejected: After the prefilled MS1, enter a threshold of 0 or 2 through 9; found 'n'.",
"Rejected: The threshold must be 0 or a number from 2 through 9; found 'n'.",
(
"Rejected: The string has 39 characters; valid Bitcoin master-seed codex32 lengths are "
"Rejected: This input has 39 characters. A Bitcoin master-seed backup must have exactly "
"48, 54, 61, 67, 74, or 127 characters."
),
]
Expand All @@ -600,7 +600,7 @@ def test_xprv_reports_every_uncorrectable_prefixed_ms_length(monkeypatch, capsys
assert ms_main(["xprv"]) == 0
output = capsys.readouterr().err
for length in lengths:
assert f"The string has {length} characters; valid Bitcoin master-seed codex32 lengths are" in output
assert f"This input has {length} characters. A Bitcoin master-seed backup must have" in output


def test_prefixed_ms_lengths_at_correction_boundary_explain_length_and_still_search(
Expand Down Expand Up @@ -645,10 +645,8 @@ def test_complete_paste_keeps_ordinary_threshold_error(monkeypatch, capsys) -> N
assert "After the prefilled" not in output


@pytest.mark.parametrize(("difference", "word"), ((-9, "short of"), (9, "too long for")))
def test_subsequent_prefixed_ms_input_reports_required_set_length(
monkeypatch, capsys, difference, word
) -> None:
@pytest.mark.parametrize("difference", (-9, 9))
def test_subsequent_prefixed_ms_input_reports_required_set_length(monkeypatch, capsys, difference) -> None:
input_module = importlib.import_module("codex32._cli_input")
prefix = "MS12NAME"
suffix = VECTOR_2["share_C"][len(prefix) :]
Expand All @@ -662,10 +660,8 @@ def test_subsequent_prefixed_ms_input_reports_required_set_length(
assert main(["secret", "--plain"]) == 0
output = capsys.readouterr()
assert output.out.strip() == VECTOR_2["secret_S"]
assert (
f"Rejected: The string is 9 characters {word} the required {len(VECTOR_2['share_C'])}-character "
"length."
) in output.err
length = len(VECTOR_2["share_C"]) + difference
assert f"Rejected: This input has {length} characters." in output.err


@pytest.mark.parametrize(
Expand Down
7 changes: 2 additions & 5 deletions tests/test_cli_lengths.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

import pytest

from codex32._cli_input import InputError, _parse, _prefixed_input_error
from codex32._cli_input import InputError, _parse
from codex32.errors import InvalidChecksum, InvalidLength


Expand Down Expand Up @@ -30,15 +30,12 @@ def test_generic_length_profile_length_and_checksum_diagnostics(length, message,
_parse(value, "ms")
assert str(failure.value) == message
assert isinstance(failure.value.__cause__, cause)
if cause is InvalidLength:
assert _prefixed_input_error(failure.value, value, "MS1", []) == message


def test_reported_invalid_length_vector_in_neutral_entry():
value = "ms10fauxs" + "x" * 73 + "r335l5tv88js3"
with pytest.raises(InputError, match="This input has 95 characters") as failure:
with pytest.raises(InputError, match="This input has 95 characters"):
_parse(value, "ms")
assert _prefixed_input_error(failure.value, value, "", []) == str(failure.value)


def test_unknown_profile_does_not_replace_checksum_failure_with_a_length_guess():
Expand Down
Loading