Merge UrlError and ConnectivityError exceptions

Since ConnectityError is only raised when we detect an UrlError
exception, we have decided to merge those exceptios. Additionally,
we have also updated the ConnectivityError message to be more explicit
on what the actual error is.

Fixes: #2647

features/unattached_status.feature

@@ -9,26 +9,28 @@ Feature: Unattached status
         When I run `sed -i 's/contracts.can/invalidurl.notcan/' /etc/ubuntu-advantage/uaclient.conf` with sudo
         And I verify that running `pro status --format json` `as non-root` exits `1`
         Then stdout is a json matching the `ua_status` schema
-        And I will see the following on stdout:
-            """
-            {"environment_vars": [], "errors": [{"message": "Failed to connect to authentication server\nCheck your Internet connection and try again.", "message_code": "connectivity-error", "service": null, "type": "system"}], "result": "failure", "services": [], "warnings": []}
-            """
+        And stdout matches regexp:
+        """
+        {"environment_vars": \[\], "errors": \[{"message": "Failed to connect to .*\\n\[Errno -2\] Name or service not known\\n", "message_code": "connectivity-error", "service": null, "type": "system"}\], "result": "failure", "services": \[\], "warnings": \[\]}
+        """
         And I verify that running `pro status --format yaml` `as non-root` exits `1`
         Then stdout is a yaml matching the `ua_status` schema
-        And I will see the following on stdout:
-            """
-            environment_vars: []
-            errors:
-            - message: 'Failed to connect to authentication server
+        And stdout matches regexp:
+        """
+        environment_vars: \[\]
+        errors:
+        - message: 'Failed to connect to https://invalidurl.notcanonical.com/v1/resources(.*)
 
-                Check your Internet connection and try again.'
-              message_code: connectivity-error
-              service: null
-              type: system
-            result: failure
-            services: []
-            warnings: []
-            """
+            \[Errno -2\] Name or service not known
+
+            '
+          message_code: connectivity-error
+          service: null
+          type: system
+        result: failure
+        services: \[\]
+        warnings: \[\]
+        """
 
         Examples: ubuntu release
            | release | machine_type  |

lib/reboot_cmds.py

@@ -66,7 +66,7 @@ def fix_pro_pkg_holds(cfg: config.UAConfig):
 def refresh_contract(cfg: config.UAConfig):
     try:
         contract.refresh(cfg)
-    except exceptions.UrlError:
+    except exceptions.ConnectivityError:
         LOG.warning("Failed to refresh contract")
         raise
 

uaclient/actions.py

@@ -50,7 +50,7 @@ def attach_with_token(
 ) -> None:
     """
     Common functionality to take a token and attach via contract backend
-    :raise UrlError: On unexpected connectivity issues to contract
+    :raise ConnectivityError: On unexpected connectivity issues to contract
         server or inability to access identity doc from metadata service.
     :raise ContractAPIError: On unexpected errors when talking to the contract
         server.
@@ -59,14 +59,9 @@ def attach_with_token(
 
     contract_client = contract.UAContractClient(cfg)
     attached_at = datetime.datetime.now(tz=datetime.timezone.utc)
-
-    try:
-        new_machine_token = contract_client.add_contract_machine(
-            contract_token=token, attachment_dt=attached_at
-        )
-    except exceptions.UrlError as e:
-        LOG.exception(str(e))
-        raise exceptions.ConnectivityError()
+    new_machine_token = contract_client.add_contract_machine(
+        contract_token=token, attachment_dt=attached_at
+    )
 
     cfg.machine_token_file.write(new_machine_token)
 
@@ -86,7 +81,7 @@ def attach_with_token(
             cfg.machine_token_file.entitlements,
             allow_enable,
         )
-    except (exceptions.UrlError, exceptions.UbuntuProError) as exc:
+    except (exceptions.ConnectivityError, exceptions.UbuntuProError) as exc:
         # Persist updated status in the event of partial attach
         attachment_data_file.write(AttachmentData(attached_at=attached_at))
         ua_status.status(cfg=cfg)
@@ -109,7 +104,7 @@ def auto_attach(
     allow_enable=True,
 ) -> None:
     """
-    :raise UrlError: On unexpected connectivity issues to contract
+    :raise ConnectivityError: On unexpected connectivity issues to contract
         server or inability to access identity doc from metadata service.
     :raise ContractAPIError: On unexpected errors when talking to the contract
         server.

uaclient/api/tests/test_api_u_pro_attach_magic_wait_v1.py

@@ -67,10 +67,10 @@ def test_wait_fails_after_number_of_connectiviry_errors(
     ):
         magic_token = "test-id"
         m_attach_token_info.side_effect = [
-            exceptions.ConnectivityError(),
-            exceptions.ConnectivityError(),
-            exceptions.ConnectivityError(),
-            exceptions.ConnectivityError(),
+            exceptions.ConnectivityError(url="url", cause="cause"),
+            exceptions.ConnectivityError(url="url", cause="cause"),
+            exceptions.ConnectivityError(url="url", cause="cause"),
+            exceptions.ConnectivityError(url="url", cause="cause"),
         ]
 
         options = MagicAttachWaitOptions(magic_token=magic_token)
@@ -86,9 +86,9 @@ def test_wait_succeeds_after_number_of_connectivity_errors(
     ):
         magic_token = "test-id"
         m_attach_token_info.side_effect = [
-            exceptions.ConnectivityError(),
-            exceptions.ConnectivityError(),
-            exceptions.ConnectivityError(),
+            exceptions.ConnectivityError(url="url", cause="cause"),
+            exceptions.ConnectivityError(url="url", cause="cause"),
+            exceptions.ConnectivityError(url="url", cause="cause"),
             {
                 "token": magic_token,
                 "expires": "2100-06-09T18:14:55.323733Z",
@@ -113,7 +113,7 @@ def test_wait_succeeds_after_unavailable_server(
     ):
         magic_token = "test-id"
         m_attach_token_info.side_effect = [
-            exceptions.ConnectivityError(),
+            exceptions.ConnectivityError(url="url", cause="cause"),
             exceptions.MagicAttachUnavailable(),
             exceptions.MagicAttachUnavailable(),
             {

uaclient/cli/__init__.py

@@ -1065,7 +1065,7 @@ def action_enable(args, *, cfg, **kwargs):
     event.info(messages.REFRESH_CONTRACT_ENABLE)
     try:
         contract.refresh(cfg)
-    except (exceptions.UrlError, exceptions.UbuntuProError):
+    except (exceptions.ConnectivityError, exceptions.UbuntuProError):
         # Inability to refresh is not a critical issue during enable
         LOG.warning("Failed to refresh contract", exc_info=True)
         event.warning(warning_msg=messages.E_REFRESH_CONTRACT_FAILURE)
@@ -1231,7 +1231,7 @@ def action_auto_attach(args, *, cfg: config.UAConfig, **kwargs) -> int:
             cfg=cfg,
             mode=event_logger.EventLoggerMode.CLI,
         )
-    except exceptions.UrlError:
+    except exceptions.ConnectivityError:
         event.info(messages.E_ATTACH_FAILURE.msg)
         return 1
     else:
@@ -1301,7 +1301,7 @@ def action_attach(args, *, cfg, **kwargs):
 
     try:
         actions.attach_with_token(cfg, token=token, allow_enable=allow_enable)
-    except exceptions.UrlError:
+    except exceptions.ConnectivityError:
         raise exceptions.AttachError()
     else:
         ret = 0
@@ -1527,8 +1527,7 @@ def _action_refresh_config(args, cfg: config.UAConfig):
 def _action_refresh_contract(_args, cfg: config.UAConfig):
     try:
         contract.refresh(cfg)
-    except exceptions.UrlError as exc:
-        LOG.exception(exc)
+    except exceptions.ConnectivityError:
         raise exceptions.RefreshContractFailure()
     print(messages.REFRESH_CONTRACT_SUCCESS)
 
@@ -1707,7 +1706,7 @@ def wrapper(*args, **kwargs):
             print(messages.CLI_INTERRUPT_RECEIVED, file=sys.stderr)
             lock.clear_lock_file_if_present()
             sys.exit(1)
-        except exceptions.UrlError as exc:
+        except exceptions.ConnectivityError as exc:
             if "CERTIFICATE_VERIFY_FAILED" in str(exc):
                 tmpl = messages.SSL_VERIFICATION_ERROR_CA_CERTIFICATES
                 if apt.is_installed("ca-certificates"):
@@ -1720,7 +1719,10 @@ def wrapper(*args, **kwargs):
                     "Failed to access URL: %s", exc.url, exc_info=exc
                 )
 
-                msg = messages.E_CONNECTIVITY_ERROR
+                msg = messages.E_CONNECTIVITY_ERROR.format(
+                    url=exc.url,
+                    cause_error=exc.cause_error,
+                )
                 event.error(error_msg=msg.msg, error_code=msg.name)
                 event.info(info_msg=msg.msg, file_type=sys.stderr)
 

uaclient/cli/tests/test_cli.py

@@ -645,8 +645,9 @@ def test_url_error_handled_gracefully(
         expected_log_call,
     ):
         m_args = m_get_parser.return_value.parse_args.return_value
-        m_args.action.side_effect = exceptions.UrlError(
-            socket.gaierror(-2, "Name or service not known"), url=error_url
+        m_args.action.side_effect = exceptions.ConnectivityError(
+            cause=socket.gaierror(-2, "Name or service not known"),
+            url=error_url,
         )
 
         with pytest.raises(SystemExit) as excinfo:
@@ -657,7 +658,11 @@ def test_url_error_handled_gracefully(
 
         assert [
             mock.call(
-                info_msg=messages.E_CONNECTIVITY_ERROR.msg, file_type=mock.ANY
+                info_msg=messages.E_CONNECTIVITY_ERROR.format(
+                    url=error_url,
+                    cause_error="[Errno -2] Name or service not known",
+                ).msg,
+                file_type=mock.ANY,
             )
         ] == m_event_info.call_args_list
         assert [expected_log_call] == m_log_exception.call_args_list

uaclient/cli/tests/test_cli_auto_attach.py

@@ -85,7 +85,9 @@ def test_happy_path(
         "api_side_effect,expected_err,expected_ret",
         (
             (
-                exceptions.UrlError("does-not-matter", "url"),
+                exceptions.ConnectivityError(
+                    cause=Exception("does-not-matter"), url="url"
+                ),
                 messages.E_ATTACH_FAILURE.msg,
                 1,
             ),

uaclient/cli/tests/test_cli_refresh.py

@@ -100,7 +100,9 @@ def test_refresh_contract_error_on_failure_to_update_contract(
         FakeConfig,
     ):
         """On failure in request_updates_contract emit an error."""
-        refresh.side_effect = exceptions.UrlError(mock.MagicMock(), "url")
+        refresh.side_effect = exceptions.ConnectivityError(
+            mock.MagicMock(), "url"
+        )
 
         cfg = FakeConfig.for_attached_machine()
 

uaclient/cli/tests/test_cli_status.py

@@ -958,14 +958,14 @@ def test_error_on_connectivity_errors(
         _m_on_supported_kernel,
         FakeConfig,
     ):
-        """Raise UrlError on connectivity issues"""
-        m_get_avail_resources.side_effect = exceptions.UrlError(
-            socket.gaierror(-2, "Name or service not known"), "url"
+        """Raise ConnectivityError on connectivity issues"""
+        m_get_avail_resources.side_effect = exceptions.ConnectivityError(
+            cause=socket.gaierror(-2, "Name or service not known"), url="url"
         )
 
         cfg = FakeConfig()
 
-        with pytest.raises(exceptions.UrlError):
+        with pytest.raises(exceptions.ConnectivityError):
             action_status(
                 mock.MagicMock(all=False, simulate_with_token=None), cfg=cfg
             )
@@ -1023,8 +1023,8 @@ def test_unicode_dash_replacement_when_unprintable(
         "exception_to_throw,exception_type,exception_message",
         (
             (
-                exceptions.UrlError("Not found", "url"),
-                exceptions.UrlError,
+                exceptions.ConnectivityError(Exception("Not found"), "url"),
+                exceptions.ConnectivityError,
                 "Not found",
             ),
             (

uaclient/contract.py

@@ -259,14 +259,10 @@ def get_magic_attach_token_info(self, magic_token: str) -> Dict[str, Any]:
         """
         headers = self.headers()
         headers.update({"Authorization": "Bearer {}".format(magic_token)})
+        response = self.request_url(
+            API_V1_GET_MAGIC_ATTACH_TOKEN_INFO, headers=headers
+        )
 
-        try:
-            response = self.request_url(
-                API_V1_GET_MAGIC_ATTACH_TOKEN_INFO, headers=headers
-            )
-        except exceptions.UrlError as e:
-            LOG.exception(e)
-            raise exceptions.ConnectivityError()
         if response.code == 401:
             raise exceptions.MagicAttachTokenError()
         if response.code == 503:
@@ -283,16 +279,12 @@ def get_magic_attach_token_info(self, magic_token: str) -> Dict[str, Any]:
     def new_magic_attach_token(self) -> Dict[str, Any]:
         """Create a magic attach token for the user."""
         headers = self.headers()
+        response = self.request_url(
+            API_V1_NEW_MAGIC_ATTACH,
+            headers=headers,
+            method="POST",
+        )
 
-        try:
-            response = self.request_url(
-                API_V1_NEW_MAGIC_ATTACH,
-                headers=headers,
-                method="POST",
-            )
-        except exceptions.UrlError as e:
-            LOG.exception(e)
-            raise exceptions.ConnectivityError()
         if response.code == 503:
             raise exceptions.MagicAttachUnavailable()
         if response.code != 200:
@@ -308,16 +300,12 @@ def revoke_magic_attach_token(self, magic_token: str):
         """Revoke a magic attach token for the user."""
         headers = self.headers()
         headers.update({"Authorization": "Bearer {}".format(magic_token)})
+        response = self.request_url(
+            API_V1_REVOKE_MAGIC_ATTACH,
+            headers=headers,
+            method="DELETE",
+        )
 
-        try:
-            response = self.request_url(
-                API_V1_REVOKE_MAGIC_ATTACH,
-                headers=headers,
-                method="DELETE",
-            )
-        except exceptions.UrlError as e:
-            LOG.exception(e)
-            raise exceptions.ConnectivityError()
         if response.code == 400:
             raise exceptions.MagicAttachTokenAlreadyActivated()
         if response.code == 401:
@@ -659,7 +647,7 @@ def refresh(cfg):
 
     :raise UbuntuProError: on failure to update contract or error processing
         contract deltas
-    :raise UrlError: On failure during a connection
+    :raise ConnectivityError: On failure during a connection
     """
     orig_entitlements = cfg.machine_token_file.entitlements
     orig_token = cfg.machine_token

uaclient/daemon/retry_auto_attach.py

@@ -52,11 +52,9 @@ def full_auto_attach_exception_to_failure_reason(e: Exception) -> str:
             error_msg=e.body
         )
     elif isinstance(e, api_exceptions.ConnectivityError):
-        return messages.RETRY_ERROR_DETAIL_CONNECTIVITY_ERROR
-    elif isinstance(e, api_exceptions.UrlError):
         return messages.RETRY_ERROR_DETAIL_URL_ERROR_URL.format(
             url=e.url
-        ) + ': "{}"'.format(str(e))
+        ) + ': "{}"'.format(str(e.cause_error))
     elif isinstance(e, api_exceptions.UbuntuProError):
         return '"{}"'.format(e.msg)
     else:

uaclient/daemon/tests/test_retry_auto_attach.py

@@ -43,11 +43,9 @@ class TestFullAutoAttachToFailureReason:
                 'an error from Canonical servers: "response"',
             ),
             (
-                exceptions.ConnectivityError(),
-                "a connectivity error",
-            ),
-            (
-                exceptions.UrlError(error.URLError("urlerror"), "url"),
+                exceptions.ConnectivityError(
+                    cause=error.URLError("urlerror"), url="url"
+                ),
                 'an error while reaching url: "urlerror"',
             ),
             (fakes.FakeUbuntuProError(), '"This is a test"'),