Better unexpected exception msgs

debian/po/pt_BR.po

@@ -7,7 +7,7 @@ msgid ""
 msgstr ""
 "Project-Id-Version: PACKAGE VERSION\n"
 "Report-Msgid-Bugs-To: \n"
-"POT-Creation-Date: 2024-01-05 17:38-0300\n"
+"POT-Creation-Date: 2024-01-16 08:56-0500\n"
 "PO-Revision-Date: 2023-09-25 12:29-0400\n"
 "Last-Translator: Lucas Moura <[email protected]>\n"
 "Language-Team: Brazilian Portuguese <[email protected]."
@@ -2593,10 +2593,11 @@ msgid ""
 msgstr ""
 
 #: ../../uaclient/messages/__init__.py:1559
+#, python-brace-format
 msgid ""
-"Unexpected error(s) occurred.\n"
-"For more details, see the log: /var/log/ubuntu-advantage.log\n"
-"To file a bug run: ubuntu-bug ubuntu-advantage-tools"
+"An unexpected error occurred: {error_msg}\n"
+"For more details, see the log: {log_path}\n"
+"If you think this is a bug, please run: ubuntu-bug ubuntu-advantage-tools"
 msgstr ""
 
 #: ../../uaclient/messages/__init__.py:1569

debian/po/ubuntu-pro.pot

@@ -8,7 +8,7 @@ msgid ""
 msgstr ""
 "Project-Id-Version: PACKAGE VERSION\n"
 "Report-Msgid-Bugs-To: \n"
-"POT-Creation-Date: 2024-01-05 17:38-0300\n"
+"POT-Creation-Date: 2024-01-16 08:56-0500\n"
 "PO-Revision-Date: YEAR-MO-DA HO:MI+ZONE\n"
 "Last-Translator: FULL NAME <EMAIL@ADDRESS>\n"
 "Language-Team: LANGUAGE <[email protected]>\n"
@@ -2258,10 +2258,11 @@ msgid ""
 msgstr ""
 
 #: ../../uaclient/messages/__init__.py:1559
+#, python-brace-format
 msgid ""
-"Unexpected error(s) occurred.\n"
-"For more details, see the log: /var/log/ubuntu-advantage.log\n"
-"To file a bug run: ubuntu-bug ubuntu-advantage-tools"
+"An unexpected error occurred: {error_msg}\n"
+"For more details, see the log: {log_path}\n"
+"If you think this is a bug, please run: ubuntu-bug ubuntu-advantage-tools"
 msgstr ""
 
 #: ../../uaclient/messages/__init__.py:1569

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
 
@@ -1766,7 +1766,11 @@ 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.msg, file_type=sys.stderr
+                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(
                 error_msg=getattr(e, "msg", str(e)), error_type="exception"

uaclient/cli/tests/test_cli.py

@@ -491,7 +491,10 @@ class TestMain:
         (
             (
                 TypeError("'NoneType' object is not subscriptable"),
-                messages.UNEXPECTED_ERROR.msg,
+                messages.UNEXPECTED_ERROR.format(
+                    error_msg="'NoneType' object is not subscriptable",
+                    log_path="/var/log/ubuntu-advantage.log",
+                ),
                 "Unhandled exception, please file a bug",
             ),
         ),
@@ -528,7 +531,6 @@ def test_errors_handled_gracefully(
 
         exc = excinfo.value
         assert 1 == exc.code
-
         assert [
             mock.call(info_msg=expected_error_msg, file_type=mock.ANY)
         ] == m_event_info.call_args_list

uaclient/cli/tests/test_cli_attach.py

@@ -515,7 +515,10 @@ def test_attach_config_enable_services(
             ),
             (
                 Exception("error"),
-                messages.UNEXPECTED_ERROR,
+                messages.UNEXPECTED_ERROR.format(
+                    error_msg="error",
+                    log_path="/var/log/ubuntu-advantage.log",
+                ),
                 messages.E_ATTACH_FAILURE_UNEXPECTED,
             ),
         ),

uaclient/contract.py

@@ -24,6 +24,7 @@
 )
 from uaclient.files.state_files import attachment_data_file
 from uaclient.http import serviceclient
+from uaclient.log import get_user_or_root_log_file_path
 
 # Here we describe every endpoint from the ua-contracts
 # service that is used by this client implementation.
@@ -492,7 +493,7 @@ def process_entitlements_delta(
     from uaclient.entitlements import entitlements_enable_order
 
     delta_error = False
-    unexpected_error = False
+    unexpected_errors = []
 
     # We need to sort our entitlements because some of them
     # depend on other service to be enable first.
@@ -523,7 +524,7 @@ def process_entitlements_delta(
             )
         except Exception as e:
             LOG.exception(e)
-            unexpected_error = True
+            unexpected_errors.append(e)
             failed_services.append(name)
             LOG.exception(
                 "Unexpected error processing contract delta for %s: %r",
@@ -536,10 +537,17 @@ def process_entitlements_delta(
             if service_enabled and deltas:
                 event.service_processed(name)
     event.services_failed(failed_services)
-    if unexpected_error:
+    if len(unexpected_errors) > 0:
         raise exceptions.AttachFailureUnknownError(
             failed_services=[
-                (name, messages.UNEXPECTED_ERROR) for name in failed_services
+                (
+                    name,
+                    messages.UNEXPECTED_ERROR.format(
+                        error_msg=str(exception),
+                        log_path=get_user_or_root_log_file_path(),
+                    ),
+                )
+                for name, exception in zip(failed_services, unexpected_errors)
             ]
         )
     elif delta_error:

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,17 @@ 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 path,
+    adjusting for whether the user is root or not.
+    """
+    if util.we_are_currently_root():
+        return UAConfig().log_file
+    else:
+        return get_user_log_file()
+
+
 def get_user_log_file() -> str:
     """Gets the correct user log_file storage location"""
     return system.get_user_cache_dir() + "/ubuntu-pro.log"

uaclient/messages/__init__.py

@@ -1553,13 +1553,13 @@ def __repr__(self):
 the --access-only flag.""",
 )
 
-UNEXPECTED_ERROR = NamedMessage(
+UNEXPECTED_ERROR = FormattedNamedMessage(
     "unexpected-error",
     t.gettext(
         """\
-Unexpected error(s) occurred.
-For more details, see the log: /var/log/ubuntu-advantage.log
-To file a bug run: 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"""
     ),
 )
 

uaclient/tests/test_log.py

@@ -2,6 +2,7 @@
 import logging
 from io import StringIO
 
+import mock
 import pytest
 
 from uaclient import log as pro_log
@@ -158,3 +159,49 @@ def test_valid_json_output(
             assert val[6].get("key") == extra.get("key")
         else:
             assert 7 == len(val)
+
+
+class TestLogHelpers:
+    @pytest.mark.parametrize(
+        [
+            "we_are_currently_root",
+            "expected",
+        ],
+        [
+            (True, "cfg_log_file"),
+            (False, "user_log_file"),
+        ],
+    )
+    @mock.patch(
+        "uaclient.log.get_user_log_file",
+        return_value="user_log_file",
+    )
+    @mock.patch(
+        "uaclient.config.UAConfig.log_file",
+        new_callable=mock.PropertyMock,
+        return_value="cfg_log_file",
+    )
+    @mock.patch("uaclient.util.we_are_currently_root")
+    def test_get_user_or_root_log_file_path(
+        self,
+        m_we_are_currently_root,
+        m_cfg_log_file,
+        m_get_user_log_file,
+        we_are_currently_root,
+        expected,
+    ):
+        """
+        Tests that the correct log_file path is retrieved
+        when the user is root and non-root
+        """
+        m_we_are_currently_root.return_value = we_are_currently_root
+        result = pro_log.get_user_or_root_log_file_path()
+        # ensure mocks are used properly
+        assert m_we_are_currently_root.call_count == 1
+        assert m_cfg_log_file.call_count + m_get_user_log_file.call_count == 1
+        if we_are_currently_root:
+            assert m_cfg_log_file.call_count == 1
+        else:
+            assert m_get_user_log_file.call_count == 1
+        # ensure correct log_file path is returned
+        assert expected == result