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

[iOS] Do not use the background check in the iOS handler. #2555

Merged
merged 1 commit into from
Apr 19, 2021

Conversation

mandel-macaque
Copy link
Contributor

The iOS NSUrlSessionHandler needed to use a workaround to ensure that
Http requests that were performed by the application did not end up in a
stale situation in which the completion would never be reached. This was
due to xamarin/xamarin-macios#5463

This workound will listen to the UIApplication notifications and would
cancel all the inflight requests. For reference, this happens here:

https://github.com/xamarin/xamarin-macios/blob/main/src/Foundation/NSUrlSessionHandler.cs#L174

As you can see, if we set the property to false, we will get the
notification. This is not longer needed as mono fixed the runtime
issue. The Xamairn.iOS team skips that workaround by default as per
this commit:

xamarin/xamarin-macios@b6fbae5#diff-1e7e2b8b4f4fe98a485a36c085a0e9cb94f53fa87a3ff11c7c54682cc3b9d514

This library should not be using the workaround because it will have
undesired side effects on applications because they will have
cancelation exceptions that are not expected AND the library does not
provide an API to reuse the HttpClient instance configured by the
client.

The iOS NSUrlSessionHandler needed to use a workaround to ensure that
Http requests that were performed by the application did not end up in a
stale situation in which the completion would never be reached. This was
due to xamarin/xamarin-macios#5463

This workound will listen to the UIApplication notifications and would
cancel all the inflight requests. For reference, this happens here:

https://github.com/xamarin/xamarin-macios/blob/main/src/Foundation/NSUrlSessionHandler.cs#L174

As you can see, if we set the property to false, we will get the
notification. This is **not longer needed** as mono fixed the runtime
issue. The Xamairn.iOS team skips that workaround by default as per
this commit:

xamarin/xamarin-macios@b6fbae5#diff-1e7e2b8b4f4fe98a485a36c085a0e9cb94f53fa87a3ff11c7c54682cc3b9d514

This library should not be using the workaround because it will have
undesired side effects on applications because they will have
cancelation exceptions that are not expected AND the library does not
provide an API to reuse the HttpClient instance configured by the
client.
@jennyf19
Copy link
Collaborator

#2556

@parkhk10
Copy link

We have confirmed that the issue originates from the HTTP client handler instance created by MSAL as the stack trace points to NSUrlSessionHandler even when we set our app's client to use a managed handler instead. Any changes to the client handler we make in our app code are not propagated to MSAL, which cancels our requests when the app is backgrounded hence the cancellation exception in the stack trace.

@trwalke trwalke closed this Apr 16, 2021
@trwalke trwalke reopened this Apr 16, 2021
@trwalke trwalke closed this Apr 16, 2021
@trwalke trwalke reopened this Apr 16, 2021
@bgavrilMS bgavrilMS merged commit d11f1bd into AzureAD:master Apr 19, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants