Introduce MsalRuntimeError

[jiasli]

Currently MSAL uses the generic RuntimeError. This error is difficult to be distinguished from other RuntimeError, especially Python itself's RuntimeError:

microsoft-authentication-library-for-python/msal/oauth2cli/oidc.py

Lines 75 to 77 in 9f78b6b

	if err:
	raise RuntimeError("%s Current epoch = %s. The id_token was: %s" % (
	err, _now, json.dumps(decoded, indent=2)))

MSAL can introduce its own MsalRuntimeError so that Azure CLI can know this error comes from MSAL and do error handling at a centralized location.

For example, Azure SDK defines

https://github.com/Azure/azure-sdk-for-python/blob/f843d16feb76a73d77e4fdee36cefc6646bd7c83/sdk/core/azure-core/azure/core/exceptions.py#L216

class AzureError(Exception):
    """Base exception for all errors.

Azure CLI handles it at

https://github.com/Azure/azure-cli-pr/blob/6751048cd85921081e3d033624f6109d7d6a01ec/src/azure-cli-core/azure/cli/core/util.py#L99-L103

    elif isinstance(ex, AzureError):
        if extract_common_error_message(ex):
            error_msg = extract_common_error_message(ex)
        AzCLIErrorType = get_error_type_by_azure_error(ex)
        az_error = AzCLIErrorType(error_msg)

[rayluo]

This seemingly straightforward topic requires more thoughts.

A little bit history here. When MSAL code base was started 5+ years ago, we did define a highest level base exception MsalError, mainly because MSAL's predecessor (ADAL) also did that. Many other software projects do that, too. However, later during the development of MSAL Python, we ended up using those MsalError sparsely. In recent years, we even gradually removed our usage of MsalError and today they are not raised from MSAL Python any more. That is all because of a design philosophy, detailed below.

Some people believe, all the observable exceptions arguably become part of the public API surface, therefore any new exception being raised, or an old exception no longer being raised, are both considered by them as a breaking change. We do not necessarily agree with that opinion, but we do want to minimize the need that our downstream consumers write code that unnecessarily couples with a lower-level library's exception.

The rare time that we did explicitly define our exception (in our another library MSAL EX), was that we expect that its downstream app developer would need to catch it and can fully recover from it.

Other than that, we consider it is less valuable to do a from msal import MsalError and later an except MsalError: do_some_translation().

Back to this specific case, how MSAL handles those ID token validation is really just an implementation detail. At one point we seriously considered - and we still won't rule out that possibility - to switch to use a lower-level 3rd-party JWT library for token validation. And that library would probably emit different exceptions. If that happens, your existing except MsalError: do_some_translation() would become moot AND you won't even notice that. Besides, there is no way for your app to recover from an ID token validation failure. The app would need to surface that error for the end user to adjust his/her computer's time. So, you might as well just let the raw exception bubble up. (Note: we do love to work with you on refining the error message in our exceptions, in that different conversation.)

[jiasli]

How about using MsalRuntimeError which inherent from RuntimeError?

If Azure CLI needs to do some additional recommendation for certain MSAL errors, catching RuntimeError and parsing the error message is obviously a more unreliable approach...

Also, Azure CLI reports all errors through client telemetry. A generic RuntimeError will make it very difficult to know whether it comes from MSAL or Python itself.

+@chasewilson