Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Raise the original error when refreshing token fails #431

Closed
jiasli opened this issue Nov 1, 2021 · 1 comment · Fixed by #434
Closed

Raise the original error when refreshing token fails #431

jiasli opened this issue Nov 1, 2021 · 1 comment · Fixed by #434

Comments

@jiasli
Copy link
Contributor

jiasli commented Nov 1, 2021

Describe the bug

Currently when refreshing token fails, MSAL silences the exception and logs and error:

except: # The exact HTTP exception is transportation-layer dependent
logger.exception("Refresh token failed") # Potential AAD outage?

ERROR    msal.application:application.py:1152 Refresh token failed
Traceback (most recent call last):
  File "C:\Users\user1\Desktop\project\env\lib\site-packages\urllib3\connectionpool.py", line 699, in urlopen
    httplib_response = self._make_request(
...
  File "C:\Program Files\Python38\lib\ssl.py", line 1173, in send
    return self._sslobj.write(data)
ConnectionResetError: [WinError 10054] An existing connection was forcibly closed by the remote host

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "C:\Users\user1\Desktop\project\env\lib\site-packages\requests\adapters.py", line 439, in send
    resp = conn.urlopen(
...
  File "C:\Users\user1\Desktop\project\env\lib\site-packages\requests\adapters.py", line 498, in send
    raise ConnectionError(err, request=request)
requests.exceptions.ConnectionError: ('Connection aborted.', ConnectionResetError(10054, 'An existing connection was forcibly closed by the remote host', None, 10054, None))

Currently Azure CLI doesn't print MSAL's logs, the real cause will be hidden and all Azure CLI can get is a None result. Making it impossible for the end user to know what happened:

https://github.com/Azure/azure-cli/blob/925d5f93e9caff6fdc74c03b3755d8811d2baf69/src/azure-cli-core/azure/cli/core/auth/util.py#L119-L121

    if not result:
        raise AuthenticationError("Can't find token from MSAL cache.",
                                  recommendation="To re-authenticate, please run:\naz login")

This topic is very similar to the one we discussed in AzureAD/microsoft-authentication-extensions-for-python#92.

@jiasli
Copy link
Contributor Author

jiasli commented Nov 1, 2021

Even if Azure CLI prints MSAL's log, the content of the exception log is uncontrollable by Azure CLI.

It is certainly not user-friendly to dump the full traceback in the context of Azure CLI's user experience.

https://docs.python.org/3/library/logging.html#logging.Logger.exception

exception(msg, *args, **kwargs)
Logs a message with level ERROR on this logger. The arguments are interpreted as for debug(). Exception info is added to the logging message. This method should only be called from an exception handler.

A simple demo:

import logging

try:
    a = 3 / 0
except:
    logging.exception("calculation failed.")

Output:

ERROR:root:calculation failed.
Traceback (most recent call last):
  File "D:\cli\testproj\main.py", line 4, in <module>
    3 / 0
ZeroDivisionError: division by zero

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
1 participant