Skip to content

Commit

Permalink
update connectivity error message
Browse files Browse the repository at this point in the history
We want to make connectivity errors clearer and easier
to debug for the user. To achieve that, we are now displaying
the url that cause the error together with the cause behind
the error.

Fixes: #2647
  • Loading branch information
lucasmoura committed Oct 30, 2023
1 parent 4c5604b commit a9fe59a
Show file tree
Hide file tree
Showing 9 changed files with 55 additions and 22 deletions.
5 changes: 4 additions & 1 deletion uaclient/actions.py
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,10 @@ def attach_with_token(
)
except exceptions.UrlError as e:
LOG.exception(str(e))
raise exceptions.ConnectivityError()
raise exceptions.ConnectivityError(
url=e.url,
cause=e.cause_error,
)

cfg.machine_token_file.write(new_machine_token)

Expand Down
16 changes: 8 additions & 8 deletions uaclient/api/tests/test_api_u_pro_attach_magic_wait_v1.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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",
Expand All @@ -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(),
{
Expand Down
5 changes: 4 additions & 1 deletion uaclient/cli/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -1720,7 +1720,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=exc.cause_error,
)
event.error(error_msg=msg.msg, error_code=msg.name)
event.info(info_msg=msg.msg, file_type=sys.stderr)

Expand Down
6 changes: 5 additions & 1 deletion uaclient/cli/tests/test_cli.py
Original file line number Diff line number Diff line change
Expand Up @@ -657,7 +657,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="[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
Expand Down
12 changes: 9 additions & 3 deletions uaclient/contract.py
Original file line number Diff line number Diff line change
Expand Up @@ -266,7 +266,7 @@ def get_magic_attach_token_info(self, magic_token: str) -> Dict[str, Any]:
)
except exceptions.UrlError as e:
LOG.exception(e)
raise exceptions.ConnectivityError()
raise exceptions.ConnectivityError(url=e.url, cause=e.cause_error)
if response.code == 401:
raise exceptions.MagicAttachTokenError()
if response.code == 503:
Expand All @@ -292,7 +292,10 @@ def new_magic_attach_token(self) -> Dict[str, Any]:
)
except exceptions.UrlError as e:
LOG.exception(e)
raise exceptions.ConnectivityError()
raise exceptions.ConnectivityError(
url=e.url,
cause=e.cause_error,
)
if response.code == 503:
raise exceptions.MagicAttachUnavailable()
if response.code != 200:
Expand All @@ -317,7 +320,10 @@ def revoke_magic_attach_token(self, magic_token: str):
)
except exceptions.UrlError as e:
LOG.exception(e)
raise exceptions.ConnectivityError()
raise exceptions.ConnectivityError(
url=e.url,
cause=e.cause_error,
)
if response.code == 400:
raise exceptions.MagicAttachTokenAlreadyActivated()
if response.code == 401:
Expand Down
2 changes: 1 addition & 1 deletion uaclient/daemon/tests/test_retry_auto_attach.py
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ class TestFullAutoAttachToFailureReason:
'an error from Canonical servers: "response"',
),
(
exceptions.ConnectivityError(),
exceptions.ConnectivityError(url="test", cause="test"),
"a connectivity error",
),
(
Expand Down
3 changes: 2 additions & 1 deletion uaclient/exceptions.py
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ def __init__(
cause_error = str(cause)
super().__init__(cause_error)
self.url = url
self.cause_error = cause_error


class ProcessExecutionError(IOError):
Expand Down Expand Up @@ -207,7 +208,7 @@ class ProxyAuthenticationFailed(UbuntuProError):


class ConnectivityError(UbuntuProError):
_msg = messages.E_CONNECTIVITY_ERROR
_formatted_msg = messages.E_CONNECTIVITY_ERROR


class ExternalAPIError(UbuntuProError):
Expand Down
7 changes: 4 additions & 3 deletions uaclient/messages/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -2019,12 +2019,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}
"""
),
)

Expand Down
21 changes: 18 additions & 3 deletions uaclient/tests/test_contract.py
Original file line number Diff line number Diff line change
Expand Up @@ -530,7 +530,10 @@ def test_new_magic_attach_token_successfull(
(
exceptions.UrlError("cause", "url"),
exceptions.ConnectivityError,
messages.E_CONNECTIVITY_ERROR,
messages.E_CONNECTIVITY_ERROR.format(
url="url",
cause="cause",
),
),
(
[
Expand Down Expand Up @@ -609,7 +612,13 @@ def test_request_magic_attach_id_info_fails(
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="cause",
).msg
== exc_error.value.msg
)
assert messages.E_CONNECTIVITY_ERROR.name == exc_error.value.msg_code

@pytest.mark.parametrize(
Expand Down Expand Up @@ -656,7 +665,13 @@ def test_revoke_magic_attach_token_fails(
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="cause",
).msg
== exc_error.value.msg
)
assert messages.E_CONNECTIVITY_ERROR.name == exc_error.value.msg_code

@pytest.mark.parametrize(
Expand Down

0 comments on commit a9fe59a

Please sign in to comment.