diff --git a/features/unattached_status.feature b/features/unattached_status.feature index 00306ef145..d04609e2df 100644 --- a/features/unattached_status.feature +++ b/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 | diff --git a/lib/reboot_cmds.py b/lib/reboot_cmds.py index 8dc0569167..40e121e340 100644 --- a/lib/reboot_cmds.py +++ b/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 diff --git a/uaclient/actions.py b/uaclient/actions.py index 7f4deaff90..16e056923d 100644 --- a/uaclient/actions.py +++ b/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. diff --git a/uaclient/api/tests/test_api_u_pro_attach_magic_wait_v1.py b/uaclient/api/tests/test_api_u_pro_attach_magic_wait_v1.py index aa65d04c45..c6f22f4cdf 100644 --- a/uaclient/api/tests/test_api_u_pro_attach_magic_wait_v1.py +++ b/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(), { diff --git a/uaclient/cli/__init__.py b/uaclient/cli/__init__.py index 1d02e01ad5..976f5a5cd1 100644 --- a/uaclient/cli/__init__.py +++ b/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) diff --git a/uaclient/cli/tests/test_cli.py b/uaclient/cli/tests/test_cli.py index 79d84ff99a..489fd5b84d 100644 --- a/uaclient/cli/tests/test_cli.py +++ b/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 diff --git a/uaclient/cli/tests/test_cli_auto_attach.py b/uaclient/cli/tests/test_cli_auto_attach.py index 5c72539ce9..a56517c68e 100644 --- a/uaclient/cli/tests/test_cli_auto_attach.py +++ b/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, ), diff --git a/uaclient/cli/tests/test_cli_refresh.py b/uaclient/cli/tests/test_cli_refresh.py index 32facff7c7..b595318fc8 100644 --- a/uaclient/cli/tests/test_cli_refresh.py +++ b/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() diff --git a/uaclient/cli/tests/test_cli_status.py b/uaclient/cli/tests/test_cli_status.py index 9142179b7b..4085cb8b88 100644 --- a/uaclient/cli/tests/test_cli_status.py +++ b/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", ), ( diff --git a/uaclient/contract.py b/uaclient/contract.py index 0418956d97..334387ab48 100644 --- a/uaclient/contract.py +++ b/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 diff --git a/uaclient/daemon/retry_auto_attach.py b/uaclient/daemon/retry_auto_attach.py index 5e4df97907..3960cf9de6 100644 --- a/uaclient/daemon/retry_auto_attach.py +++ b/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: diff --git a/uaclient/daemon/tests/test_retry_auto_attach.py b/uaclient/daemon/tests/test_retry_auto_attach.py index 5be81d687e..720495da08 100644 --- a/uaclient/daemon/tests/test_retry_auto_attach.py +++ b/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"'), diff --git a/uaclient/exceptions.py b/uaclient/exceptions.py index 38d7c899b5..9a250a974c 100644 --- a/uaclient/exceptions.py +++ b/uaclient/exceptions.py @@ -19,20 +19,6 @@ class DelayProLicensePolling(IsProLicensePresentError): pass -class UrlError(IOError): - def __init__( - self, - cause: Exception, - url: str, - ): - if getattr(cause, "reason", None): - cause_error = str(getattr(cause, "reason")) - else: - cause_error = str(cause) - super().__init__(cause_error) - self.url = url - - class ProcessExecutionError(IOError): def __init__( self, @@ -206,10 +192,6 @@ class ProxyAuthenticationFailed(UbuntuProError): _msg = messages.E_PROXY_AUTH_FAIL -class ConnectivityError(UbuntuProError): - _msg = messages.E_CONNECTIVITY_ERROR - - class ExternalAPIError(UbuntuProError): _formatted_msg = messages.E_EXTERNAL_API_ERROR code = None # type: int @@ -236,6 +218,31 @@ def __init__(self, url, **kwargs) -> None: self.url = url +class ConnectivityError(UbuntuProError, IOError): + _formatted_msg = messages.E_CONNECTIVITY_ERROR + + def __init__( + self, + cause: Exception, + url: str, + ): + if getattr(cause, "reason", None): + cause_error = str(getattr(cause, "reason")) + else: + cause_error = str(cause) + IOError.__init__(self, cause_error) + UbuntuProError.__init__(self, cause_error=cause_error, url=url) + + # Even though we already set those variable through UbuntuProError + # we need to set them again to avoid mypy warnings + self.cause_error = cause_error + self.url = url + + +# We are doing that just to keep backwards compatibility +# for our custom UrlError exception +UrlError = ConnectivityError + ############################################################################### # ATTACH/ENABLE # ############################################################################### diff --git a/uaclient/http/__init__.py b/uaclient/http/__init__.py index 9863ac033e..0ebfc1547d 100644 --- a/uaclient/http/__init__.py +++ b/uaclient/http/__init__.py @@ -168,7 +168,8 @@ def _readurl_urllib( except error.HTTPError as e: resp = e except error.URLError as e: - raise exceptions.UrlError(e, url=req.full_url) + LOG.exception(str(e.reason)) + raise exceptions.ConnectivityError(cause=e, url=req.full_url) body = resp.read().decode("utf-8") diff --git a/uaclient/messages/__init__.py b/uaclient/messages/__init__.py index af4b60ce32..8e1d0ff5c2 100644 --- a/uaclient/messages/__init__.py +++ b/uaclient/messages/__init__.py @@ -2031,12 +2031,13 @@ def __repr__(self): "proxy-auth-fail", t.gettext("Proxy authentication failed") ) -E_CONNECTIVITY_ERROR = NamedMessage( +E_CONNECTIVITY_ERROR = FormattedNamedMessage( "connectivity-error", t.gettext( """\ -Failed to connect to authentication server -Check your Internet connection and try again.""" +Failed to connect to {url} +{cause_error} +""" ), ) diff --git a/uaclient/tests/test_actions.py b/uaclient/tests/test_actions.py index cce0266672..25926653ae 100644 --- a/uaclient/tests/test_actions.py +++ b/uaclient/tests/test_actions.py @@ -43,7 +43,7 @@ class TestAttachWithToken: ( "token", True, - exceptions.UrlError(Exception(), "url"), + exceptions.ConnectivityError(cause=Exception(), url="url"), None, None, None, @@ -67,7 +67,7 @@ class TestAttachWithToken: [{"machineTokenInfo": {"machineId": "machine-id"}}], "get-machine-id-result", mock.sentinel.entitlements, - exceptions.UrlError(Exception(), "url"), + exceptions.ConnectivityError(cause=Exception(), url="url"), None, [mock.call(contract_token="token", attachment_dt=mock.ANY)], [mock.call({"machineTokenInfo": {"machineId": "machine-id"}})], @@ -80,7 +80,7 @@ class TestAttachWithToken: [mock.call()], [], [], - pytest.raises(exceptions.UrlError), + pytest.raises(exceptions.ConnectivityError), ), ( "token", diff --git a/uaclient/tests/test_contract.py b/uaclient/tests/test_contract.py index 8f7c6141e7..905610cd20 100644 --- a/uaclient/tests/test_contract.py +++ b/uaclient/tests/test_contract.py @@ -528,9 +528,12 @@ def test_new_magic_attach_token_successfull( "request_side_effect,expected_exception,message", ( ( - exceptions.UrlError("cause", "url"), + exceptions.ConnectivityError(cause="cause", url="url"), exceptions.ConnectivityError, - messages.E_CONNECTIVITY_ERROR, + messages.E_CONNECTIVITY_ERROR.format( + url="url", + cause_error="cause", + ), ), ( [ @@ -604,12 +607,20 @@ def test_request_magic_attach_id_info_fails( cfg = FakeConfig() client = UAContractClient(cfg) magic_token = "test-id" - request_url.side_effect = exceptions.UrlError("cause", "url") + request_url.side_effect = exceptions.ConnectivityError( + cause=Exception("cause"), url="url" + ) with pytest.raises(exceptions.ConnectivityError) as exc_error: client.get_magic_attach_token_info(magic_token=magic_token) - assert messages.E_CONNECTIVITY_ERROR.msg == exc_error.value.msg + assert ( + messages.E_CONNECTIVITY_ERROR.format( + url="url", + cause_error="cause", + ).msg + == exc_error.value.msg + ) assert messages.E_CONNECTIVITY_ERROR.name == exc_error.value.msg_code @pytest.mark.parametrize( @@ -651,12 +662,20 @@ def test_revoke_magic_attach_token_fails( cfg = FakeConfig() client = UAContractClient(cfg) magic_token = "test-id" - request_url.side_effect = exceptions.UrlError("cause", "url") + request_url.side_effect = exceptions.ConnectivityError( + cause=Exception("cause"), url="url" + ) with pytest.raises(exceptions.ConnectivityError) as exc_error: client.revoke_magic_attach_token(magic_token=magic_token) - assert messages.E_CONNECTIVITY_ERROR.msg == exc_error.value.msg + assert ( + messages.E_CONNECTIVITY_ERROR.format( + url="url", + cause_error="cause", + ).msg + == exc_error.value.msg + ) assert messages.E_CONNECTIVITY_ERROR.name == exc_error.value.msg_code @pytest.mark.parametrize( @@ -1161,12 +1180,12 @@ def test_available_resources_error_on_network_disconnected( """Raise error get_available_resources can't contact backend""" cfg = FakeConfig() - urlerror = exceptions.UrlError( + urlerror = exceptions.ConnectivityError( socket.gaierror(-2, "Name or service not known"), "url" ) m_available_resources.side_effect = urlerror - with pytest.raises(exceptions.UrlError) as exc: + with pytest.raises(exceptions.ConnectivityError) as exc: get_available_resources(cfg) assert urlerror == exc.value diff --git a/uaclient/tests/test_livepatch.py b/uaclient/tests/test_livepatch.py index 7713e0948d..a2ecc700fe 100644 --- a/uaclient/tests/test_livepatch.py +++ b/uaclient/tests/test_livepatch.py @@ -327,7 +327,12 @@ def test_is_kernel_supported_calls_api_with_correct_params( ], LivepatchSupport.UNSUPPORTED, ), - (exceptions.UrlError(mock.MagicMock(), "url"), None), + ( + exceptions.ConnectivityError( + cause=mock.MagicMock(), url="url" + ), + None, + ), (Exception(), None), ( [