Skip to content

Commit

Permalink
fix(store): nicer error message when getting nonexistent libs (#2034)
Browse files Browse the repository at this point in the history
When a charm lib doesn't exist on charmhub, charmhub itself gives us a
hard to use error. This is replaced with a more useful message.

Fixes #1754
CRAFT-3353

---------

Signed-off-by: Alex Lowe <[email protected]>
Co-authored-by: Imani Pelton <[email protected]>
  • Loading branch information
lengau and bepri authored Dec 12, 2024
1 parent c8cef61 commit c9b5103
Show file tree
Hide file tree
Showing 2 changed files with 35 additions and 3 deletions.
18 changes: 17 additions & 1 deletion charmcraft/services/store.py
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
import craft_store
from craft_cli import emit
from craft_store import models
from craft_store.errors import StoreServerError
from overrides import override

from charmcraft import const, env, errors, store
Expand Down Expand Up @@ -244,7 +245,22 @@ def get_libraries_metadata(
store_lib["patch"] = patch_version
store_libs.append(store_lib)

return self.anonymous_client.fetch_libraries_metadata(store_libs)
try:
return self.anonymous_client.fetch_libraries_metadata(store_libs)
except StoreServerError as exc:
lib_names = [lib.lib for lib in libraries]
# Type ignore here because error_list is supposed to have string keys, but
# for whatever reason the store returns a null code for this one.
# https://bugs.launchpad.net/snapstore-server/+bug/1925065
if exc.error_list[None]["message"] == ( # type: ignore[index]
"Items need to include 'library_id' or 'package_id'"
):
raise errors.LibraryError(
"One or more declared charm-libs could not be found in the store.",
details="Declared charm-libs: " + ", ".join(lib_names),
resolution="Check the charm and library names in charmcraft.yaml",
) from exc
raise

def get_libraries_metadata_by_name(
self, libraries: Sequence[CharmLib]
Expand Down
20 changes: 18 additions & 2 deletions tests/unit/services/test_store.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,10 @@
from unittest import mock

import craft_store
import craft_store.errors
import distro
import pytest
import requests
from craft_cli.pytest_plugin import RecordingEmitter
from craft_store import models
from hypothesis import given, strategies
Expand All @@ -34,12 +36,12 @@


@pytest.fixture
def store(service_factory) -> services.StoreService:
def store(service_factory, mock_store_anonymous_client) -> services.StoreService:
store = services.StoreService(
app=application.APP_METADATA, services=service_factory
)
store.client = mock.Mock(spec_set=client.Client)
store.anonymous_client = mock.Mock(spec_set=client.AnonymousClient)
store.anonymous_client = mock_store_anonymous_client
return store


Expand Down Expand Up @@ -305,3 +307,17 @@ def test_fetch_libraries_metadata(monkeypatch, store, libs, expected_call):
store.anonymous_client.fetch_libraries_metadata.assert_called_once_with(
expected_call
)


def test_get_libraries_metadata_name_error(
monkeypatch, store: services.StoreService, mock_store_anonymous_client: mock.Mock
) -> None:
bad_response = requests.Response()
bad_response.status_code = 400
bad_response._content = b'{"error-list": [{"code": null, "message": "Items need to include \'library_id\' or \'package_id\'"}]}'
mock_store_anonymous_client.fetch_libraries_metadata.side_effect = (
craft_store.errors.StoreServerError(bad_response)
)

with pytest.raises(errors.LibraryError, match="One or more declared"):
store.get_libraries_metadata([CharmLib(lib="boop.snoot", version="-1")])

0 comments on commit c9b5103

Please sign in to comment.