From 808ffa92d686f59e466e497350e5588d809abdae Mon Sep 17 00:00:00 2001 From: a-dubs Date: Wed, 6 Dec 2023 15:03:46 -0500 Subject: [PATCH] feat: unxpected error messages point to correct log path if not root added util function to get proper log path based on if currently root or not --- uaclient/cli/__init__.py | 7 +++++-- uaclient/cli/tests/test_cli.py | 8 +++++++- uaclient/cli/tests/test_cli_attach.py | 10 +++++++++- uaclient/contract.py | 5 ++++- uaclient/log.py | 9 +++++++++ uaclient/messages/__init__.py | 7 +++---- uaclient/tests/test_log.py | 28 +++++++++++++++++++++++++++ uaclient/util.py | 7 +++++++ 8 files changed, 72 insertions(+), 9 deletions(-) diff --git a/uaclient/cli/__init__.py b/uaclient/cli/__init__.py index 9235f0573b..3844ad2729 100644 --- a/uaclient/cli/__init__.py +++ b/uaclient/cli/__init__.py @@ -66,7 +66,7 @@ ) from uaclient.files import notices, state_files from uaclient.files.notices import Notice -from uaclient.log import JsonArrayFormatter +from uaclient.log import JsonArrayFormatter, get_user_or_root_log_file_path from uaclient.timer.update_messaging import refresh_motd, update_motd_messages from uaclient.yaml import safe_dump, safe_load @@ -1764,7 +1764,10 @@ def wrapper(*args, **kwargs): LOG.exception("Unhandled exception, please file a bug") lock.clear_lock_file_if_present() event.info( - info_msg=messages.UNEXPECTED_ERROR.format(error_msg=str(e)), + info_msg=messages.UNEXPECTED_ERROR.format( + error_msg=str(e), + log_path=get_user_or_root_log_file_path(), + ), file_type=sys.stderr, ) event.error( diff --git a/uaclient/cli/tests/test_cli.py b/uaclient/cli/tests/test_cli.py index 7ba817097c..717b2cb02f 100644 --- a/uaclient/cli/tests/test_cli.py +++ b/uaclient/cli/tests/test_cli.py @@ -492,12 +492,17 @@ class TestMain: ( TypeError("'NoneType' object is not subscriptable"), messages.UNEXPECTED_ERROR.format( - error_msg="'NoneType' object is not subscriptable" + error_msg="'NoneType' object is not subscriptable", + log_path="fake_log_path", ), "Unhandled exception, please file a bug", ), ), ) + @mock.patch( + "uaclient.log.get_user_or_root_log_file_path", + return_value="fake_log_path", + ) @mock.patch(M_PATH_UACONFIG + "delete_cache_key") @mock.patch("uaclient.cli.event.info") @mock.patch("uaclient.cli.LOG.exception") @@ -510,6 +515,7 @@ def test_errors_handled_gracefully( m_log_exception, m_event_info, m_delete_cache_key, + _m_get_user_or_root_log_path, event, exception, expected_error_msg, diff --git a/uaclient/cli/tests/test_cli_attach.py b/uaclient/cli/tests/test_cli_attach.py index 8e355c0c82..f2a7b7bace 100644 --- a/uaclient/cli/tests/test_cli_attach.py +++ b/uaclient/cli/tests/test_cli_attach.py @@ -515,11 +515,18 @@ def test_attach_config_enable_services( ), ( Exception("error"), - messages.UNEXPECTED_ERROR.format(error_msg="error"), + messages.UNEXPECTED_ERROR.format( + error_msg="error", + log_path="fake_log_path", + ), messages.E_ATTACH_FAILURE_UNEXPECTED, ), ), ) + @mock.patch( + "uaclient.log.get_user_or_root_log_file_path", + return_value="fake_log_path", + ) @mock.patch("uaclient.files.state_files.attachment_data_file.write") @mock.patch("uaclient.entitlements.entitlements_enable_order") @mock.patch("uaclient.contract.process_entitlement_delta") @@ -534,6 +541,7 @@ def test_attach_when_one_service_fails_to_enable( m_process_entitlement_delta, m_enable_order, _m_attachment_data_file_write, + _m_get_user_or_root_log_path, expected_exception, expected_msg, expected_outer_msg, diff --git a/uaclient/contract.py b/uaclient/contract.py index c1d75b1b23..492b0c9275 100644 --- a/uaclient/contract.py +++ b/uaclient/contract.py @@ -553,7 +553,10 @@ def process_entitlements_delta( failed_services=[ ( name, - messages.UNEXPECTED_ERROR.format(error_msg=str(exception)), + messages.UNEXPECTED_ERROR.format( + error_msg=str(exception), + log_path=util.get_user_or_root_log_file_path(), + ), ) for name, exception in zip(failed_services, unexpected_errors) ] diff --git a/uaclient/log.py b/uaclient/log.py index 05e658af2b..8cf3a412d7 100644 --- a/uaclient/log.py +++ b/uaclient/log.py @@ -5,6 +5,7 @@ from typing import Any, Dict, List # noqa: F401 from uaclient import defaults, system, util +from uaclient.config import UAConfig class RedactionFilter(logging.Filter): @@ -61,6 +62,14 @@ def format(self, record: logging.LogRecord) -> str: return json.dumps(list(local_log_record.values())) +def get_user_or_root_log_file_path() -> str: + """Gets the correct log_file storage location adjusting for whether the user is root or not""" + if util.we_are_currently_root(): + return UAConfig().log_file + else: + return "~/.cache/" + defaults.USER_CACHE_SUBDIR + "/ubuntu-pro.log" + + def get_user_log_file() -> str: """Gets the correct user log_file storage location""" return system.get_user_cache_dir() + "/ubuntu-pro.log" diff --git a/uaclient/messages/__init__.py b/uaclient/messages/__init__.py index 85593868c8..c4294dcc03 100644 --- a/uaclient/messages/__init__.py +++ b/uaclient/messages/__init__.py @@ -1538,10 +1538,9 @@ def __repr__(self): "unexpected-error", t.gettext( """\ -Unexpected error(s) occurred: {error_msg} -For more details, see the log: ~/.cache/ubuntu-pro/ubuntu-pro.log -First, try searching online for solutions to this error. -Otherwise, file a bug using the command: ubuntu-bug ubuntu-advantage-tools""" +An unexpected error occurred: {error_msg} +For more details, see the log: {log_path} +If you think this is a bug, please run: ubuntu-bug ubuntu-advantage-tools""" ), ) diff --git a/uaclient/tests/test_log.py b/uaclient/tests/test_log.py index fbd6c481a8..f919e4d16a 100644 --- a/uaclient/tests/test_log.py +++ b/uaclient/tests/test_log.py @@ -1,11 +1,14 @@ import json import logging from io import StringIO +import mock import pytest from uaclient import log as pro_log from uaclient import util +from uaclient.util import we_are_currently_root +from uaclient.config import UAConfig LOG = logging.getLogger(util.replace_top_level_logger_name(__name__)) LOG_FMT = "%(asctime)s%(name)s%(funcName)s%(lineno)s\ @@ -158,3 +161,28 @@ def test_valid_json_output( assert val[6].get("key") == extra.get("key") else: assert 7 == len(val) + +def test_get_user_or_root_log_file_path(): + """Gets the correct log_file storage location adjusting for whether the user is root or not""" + # test root log file path + with mock.patch("uaclient.util.we_are_currently_root", return_value=True): + assert ( + pro_log.get_user_or_root_log_file_path() + == "/var/log/ubuntu-advantage.log" + ) + # test default user log file path + with mock.patch("uaclient.util.we_are_currently_root", return_value=False): + assert ( + pro_log.get_user_or_root_log_file_path() + == "~/.cache/ubuntu-pro/ubuntu-pro.log" + ) + # test custom user log file path + with mock.patch("uaclient.config.UAConfig.log_file", new_callable=mock.PropertyMock) as mock_log_file: + expected_log_file = "/tmp/foo.log" + mock_log_file.return_value = expected_log_file + with mock.patch("uaclient.util.we_are_currently_root", return_value=True) as mock_root: + result = pro_log.get_user_or_root_log_file_path() + mock_root.assert_called() + mock_log_file.assert_called() + assert expected_log_file == result + diff --git a/uaclient/util.py b/uaclient/util.py index 724d0adf7d..9dd8eef3c9 100644 --- a/uaclient/util.py +++ b/uaclient/util.py @@ -477,3 +477,10 @@ def print_package_list( ) ) print("") + + +def get_user_or_root_log_path() -> str: + if we_are_currently_root(): + return "/var/log/ubuntu-advantage.log" + else: + return "~/.cache/ubuntu-pro/ubuntu-pro.log"