[Bug] System.Net.HttpListenerException when system browser flow is cancelled (sometimes)

[crmann1]

Which Version of MSAL are you using ?
MSAL 4.11

Platform
net45

What authentication flow has the issue?

• Desktop / Mobile
  System browser

Is this a new or existing app?
visual studio 2019

Repro
Cancel a system browser flow sign in by closing the account settings dialog when the system browser windows is up. This will set the cancellation token to cancelled which is then passed to the ExecuteAsync method.

When a system browser flow is cancelled
Cancel systemWebBrowserCancellation

builder = builder.WithUseEmbeddedWebView(false);
return await builder.ExecuteAsync(systemWebBrowserCancellation).ConfigureAwait(false);

we sometimes get an exception
System.Net.HttpListenerException

System.Net.HttpListenerException: 'The I/O operation has been aborted because of either a thread exit or an application request'

at System.Net.HttpListener.EndGetContext(IAsyncResult asyncResult)
at Microsoft.Identity.Client.Platforms.Shared.DefaultOSBrowser.HttpListenerInterceptor.d__14.MoveNext()
at Microsoft.Identity.Client.Platforms.Shared.Desktop.OsBrowser.DefaultOsBrowserWebUi.d__10.MoveNext()
at Microsoft.Identity.Client.Platforms.Shared.Desktop.OsBrowser.DefaultOsBrowserWebUi.d__7.MoveNext()
at Microsoft.Identity.Client.Internal.Requests.InteractiveRequest.d__11.MoveNext()
at Microsoft.Identity.Client.Internal.Requests.InteractiveRequest.d__10.MoveNext()
at Microsoft.Identity.Client.Internal.Requests.RequestBase.d__14.MoveNext()
at Microsoft.Identity.Client.ApiConfig.Executors.PublicClientExecutor.d__2.MoveN

Expected behavior
Could MSAL translate this into a operation cancelled exception if the cancellation token is cancelled.

An operation cancelled exception should be thrown instead if the cancellation token was cancelled and the httplistenerexception was due to that condition.

Actual behavior
A clear and concise description of what happens, e.g. exception is thrown, UI freezes

[crmann1]

Sometimes we see the exception not all the time.

[henrik-me]

@crmann1 : will catching System.Net.HttpListenerException be ok as a workaround?

[crmann1]

Catching it in the caller code? We can do that for now.

[henrik-me]

@bgavrilMS @jmprieur : could/should we catch all exceptions and translate into canceled exception or perhaps user canceled?
@crmann1 : Do you happen to have a repro where we can hit this issue at least relatively frequently? Also do you have a set of logs you can share from when this issue have occurred?

[jmprieur]

@henrik-me : I don't understand all exceptions? Is it what we've been doing (and should not be doing) ?

[bgavrilMS]

@jmprieur - the cancellation semantics in .NET state that if you cancel a long running operation, that operation should throw an OperationCancelledException. In this case, the exception is HttpListenerException.

The impact is that it prevents apps from correctly observing a system browser flow cancellation, potentially crashing the app.

As an aside: because libraries are not required to cancel an operation if they think there is no point in this. For example, if MSAL already fetched the tokens and saved them to the cache, there is no point in having all that work go to waste.
Also, the recommendation is for every async API you expose, to have an overload with a cancellation token (just saying for Identity.Web).

[henrik-me]

@bgavrilMS thought on how we would handle this. A wrapper inside all our public api points that will validate if the exception is ours or operationcanceled and if not throw the best possible exception? Where we would err towards OperationCanceledException as the default (unless it's already an MSAL exception type)? Something we should do only for system browser usage?