Skip to content

Commit

Permalink
fix(yaml): wrap error message for YAML errors (#427)
Browse files Browse the repository at this point in the history
  • Loading branch information
lengau authored Aug 23, 2024
1 parent 2f56f14 commit df906c3
Show file tree
Hide file tree
Showing 4 changed files with 78 additions and 6 deletions.
16 changes: 16 additions & 0 deletions craft_application/errors.py
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
from collections.abc import Sequence
from typing import TYPE_CHECKING

import yaml
from craft_cli import CraftError
from craft_providers import bases

Expand All @@ -42,6 +43,21 @@ class PathInvalidError(CraftError, OSError):
"""Error that the given path is not usable."""


class YamlError(CraftError, yaml.YAMLError):
"""Craft-cli friendly version of a YAML error."""

@classmethod
def from_yaml_error(cls, filename: str, error: yaml.YAMLError) -> Self:
"""Convert a pyyaml YAMLError to a craft-application YamlError."""
message = f"error parsing {filename!r}"
details = str(error)
return cls(
message,
details=details,
resolution=f"Ensure {filename} contains valid YAML",
)


class CraftValidationError(CraftError):
"""Error validating project yaml."""

Expand Down
13 changes: 10 additions & 3 deletions craft_application/util/yaml.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,10 +17,13 @@
from __future__ import annotations

import contextlib
import pathlib
from typing import TYPE_CHECKING, Any, TextIO, cast, overload

import yaml

from craft_application import errors

if TYPE_CHECKING: # pragma: no cover
from collections.abc import Hashable

Expand Down Expand Up @@ -99,9 +102,13 @@ def safe_yaml_load(stream: TextIO) -> Any: # noqa: ANN401 - The YAML could be a
:param stream: Any text-like IO object.
:returns: A dict object mapping the yaml.
"""
# Silencing S506 ("probable use of unsafe loader") because we override it by using
# our own safe loader.
return yaml.load(stream, Loader=_SafeYamlLoader) # noqa: S506
try:
# Silencing S506 ("probable use of unsafe loader") because we override it by
# using our own safe loader.
return yaml.load(stream, Loader=_SafeYamlLoader) # noqa: S506
except yaml.YAMLError as error:
filename = pathlib.Path(stream.name).name
raise errors.YamlError.from_yaml_error(filename, error) from error


@overload
Expand Down
44 changes: 43 additions & 1 deletion tests/unit/test_errors.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,11 +20,53 @@
import pydantic
import pytest
import pytest_check
from craft_application.errors import CraftValidationError, PartsLifecycleError
import yaml
from craft_application.errors import (
CraftValidationError,
PartsLifecycleError,
YamlError,
)
from pydantic import BaseModel
from typing_extensions import Self


@pytest.mark.parametrize(
("original", "expected"),
[
(
yaml.YAMLError("I am a thing"),
YamlError(
"error parsing 'something.yaml'",
details="I am a thing",
resolution="Ensure something.yaml contains valid YAML",
),
),
(
yaml.MarkedYAMLError(
problem="I am a thing",
problem_mark=yaml.error.Mark(
name="bork",
index=0,
line=0,
column=0,
buffer="Hello there",
pointer=0,
),
),
YamlError(
"error parsing 'something.yaml'",
details='I am a thing\n in "bork", line 1, column 1:\n Hello there\n ^',
resolution="Ensure something.yaml contains valid YAML",
),
),
],
)
def test_yaml_error_from_yaml_error(original, expected):
actual = YamlError.from_yaml_error("something.yaml", original)

assert actual == expected


@pytest.mark.parametrize(
"err",
[
Expand Down
11 changes: 9 additions & 2 deletions tests/unit/util/test_yaml.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,9 @@
import pathlib

import pytest
import pytest_check
from craft_application import errors
from craft_application.util import yaml
from yaml.error import YAMLError

TEST_DIR = pathlib.Path(__file__).parent

Expand All @@ -39,9 +40,15 @@ def test_safe_yaml_loader_valid(file):
)
def test_safe_yaml_loader_invalid(file):
with file.open() as f:
with pytest.raises(YAMLError):
with pytest.raises(
errors.YamlError, match=f"error parsing {file.name!r}"
) as exc_info:
yaml.safe_yaml_load(f)

pytest_check.is_in(file.name, exc_info.value.resolution)
pytest_check.is_true(str(exc_info.value.resolution).endswith("contains valid YAML"))
pytest_check.is_in("found", exc_info.value.details)


@pytest.mark.parametrize(
("data", "kwargs", "expected"),
Expand Down

0 comments on commit df906c3

Please sign in to comment.