diff --git a/changelog/890.bugfix.rst b/changelog/890.bugfix.rst new file mode 100644 index 00000000..d4dfd1bf --- /dev/null +++ b/changelog/890.bugfix.rst @@ -0,0 +1 @@ +Improve logging when keyring fails. diff --git a/tests/test_auth.py b/tests/test_auth.py index d9dde51b..d5abfec7 100644 --- a/tests/test_auth.py +++ b/tests/test_auth.py @@ -1,5 +1,6 @@ import getpass import logging +import re import pytest @@ -13,23 +14,23 @@ def config() -> utils.RepositoryConfig: return dict(repository="system") -def test_get_password_keyring_overrides_prompt(monkeypatch, config): +def test_get_username_keyring_defers_to_prompt(monkeypatch, entered_username, config): class MockKeyring: @staticmethod - def get_password(system, user): - return f"{user}@{system} sekure pa55word" + def get_credential(system, user): + return None monkeypatch.setattr(auth, "keyring", MockKeyring) - pw = auth.Resolver(config, auth.CredentialInput("user")).password - assert pw == "user@system sekure pa55word" + username = auth.Resolver(config, auth.CredentialInput()).username + assert username == "entered user" def test_get_password_keyring_defers_to_prompt(monkeypatch, entered_password, config): class MockKeyring: @staticmethod def get_password(system, user): - return + return None monkeypatch.setattr(auth, "keyring", MockKeyring) @@ -51,7 +52,7 @@ def test_empty_password_bypasses_prompt(monkeypatch, entered_password, config): def test_no_username_non_interactive_aborts(config): with pytest.raises(exceptions.NonInteractive): - auth.Private(config, auth.CredentialInput("user")).password + auth.Private(config, auth.CredentialInput()).username def test_no_password_non_interactive_aborts(config): @@ -124,55 +125,99 @@ def test_get_password_keyring_missing_non_interactive_aborts( auth.Private(config, auth.CredentialInput("user")).password -@pytest.fixture -def keyring_no_backends(monkeypatch): - """Simulate missing keyring backend raising RuntimeError on get_password.""" - +def test_get_username_keyring_runtime_error_logged( + entered_username, monkeypatch, config, caplog +): class FailKeyring: + """Simulate missing keyring backend raising RuntimeError on get_credential.""" + @staticmethod - def get_password(system, username): + def get_credential(system, username): raise RuntimeError("fail!") - monkeypatch.setattr(auth, "keyring", FailKeyring()) + monkeypatch.setattr(auth, "keyring", FailKeyring) + assert auth.Resolver(config, auth.CredentialInput()).username == "entered user" -@pytest.fixture -def keyring_no_backends_get_credential(monkeypatch): - """Simulate missing keyring backend raising RuntimeError on get_credential.""" + assert re.search( + r"Error getting username from keyring.+Traceback.+RuntimeError: fail!", + caplog.text, + re.DOTALL, + ) + +def test_get_password_keyring_runtime_error_logged( + entered_username, entered_password, monkeypatch, config, caplog +): class FailKeyring: + """Simulate missing keyring backend raising RuntimeError on get_password.""" + @staticmethod - def get_credential(system, username): + def get_password(system, username): raise RuntimeError("fail!") - monkeypatch.setattr(auth, "keyring", FailKeyring()) + monkeypatch.setattr(auth, "keyring", FailKeyring) + assert auth.Resolver(config, auth.CredentialInput()).password == "entered pw" -def test_get_username_runtime_error_suppressed( - entered_username, keyring_no_backends_get_credential, caplog, config -): - assert auth.Resolver(config, auth.CredentialInput()).username == "entered user" - assert caplog.messages == ["fail!"] + assert re.search( + r"Error getting password from keyring.+Traceback.+RuntimeError: fail!", + caplog.text, + re.DOTALL, + ) -def test_get_password_runtime_error_suppressed( - entered_password, keyring_no_backends, caplog, config -): - assert auth.Resolver(config, auth.CredentialInput("user")).password == "entered pw" - assert caplog.messages == ["fail!"] - +def _raise_home_key_error(): + """Simulate environment from https://github.com/pypa/twine/issues/889.""" + try: + raise KeyError("HOME") + except KeyError: + raise KeyError("uid not found: 999") -def test_get_username_return_none(entered_username, monkeypatch, config): - """Prompt for username when it's not in keyring.""" +def test_get_username_keyring_key_error_logged( + entered_username, monkeypatch, config, caplog +): class FailKeyring: @staticmethod def get_credential(system, username): - return None + _raise_home_key_error() + + monkeypatch.setattr(auth, "keyring", FailKeyring) - monkeypatch.setattr(auth, "keyring", FailKeyring()) assert auth.Resolver(config, auth.CredentialInput()).username == "entered user" + assert re.search( + r"Error getting username from keyring" + r".+Traceback" + r".+KeyError: 'HOME'" + r".+KeyError: 'uid not found: 999'", + caplog.text, + re.DOTALL, + ) + + +def test_get_password_keyring_key_error_logged( + entered_username, entered_password, monkeypatch, config, caplog +): + class FailKeyring: + @staticmethod + def get_password(system, username): + _raise_home_key_error() + + monkeypatch.setattr(auth, "keyring", FailKeyring) + + assert auth.Resolver(config, auth.CredentialInput()).password == "entered pw" + + assert re.search( + r"Error getting password from keyring" + r".+Traceback" + r".+KeyError: 'HOME'" + r".+KeyError: 'uid not found: 999'", + caplog.text, + re.DOTALL, + ) + def test_logs_cli_values(caplog): caplog.set_level(logging.INFO, "twine") diff --git a/twine/auth.py b/twine/auth.py index a873bf7c..83fa6181 100644 --- a/twine/auth.py +++ b/twine/auth.py @@ -63,7 +63,7 @@ def get_username_from_keyring(self) -> Optional[str]: # To support keyring prior to 15.2 pass except Exception as exc: - logger.warning(str(exc)) + logger.warning("Error getting username from keyring", exc_info=exc) return None def get_password_from_keyring(self) -> Optional[str]: @@ -73,7 +73,7 @@ def get_password_from_keyring(self) -> Optional[str]: logger.info("Querying keyring for password") return cast(str, keyring.get_password(system, username)) except Exception as exc: - logger.warning(str(exc)) + logger.warning("Error getting password from keyring", exc_info=exc) return None def username_from_keyring_or_prompt(self) -> str: