Make IAppLinks.RegisterLink etc awaitable to allow catching exceptions #852
Conversation
|
@dominik-weber, It will cover your contributions to all .NET Foundation-managed open source projects. |
|
@dominik-weber, thanks for signing the contribution license agreement. We will now validate the agreement and then the pull request. |
| { | ||
| RemoveFromIndexItemAsync(appLink.AppLinkUri.ToString()); | ||
| return Task.FromResult<object>(null); |
There was a problem hiding this comment.
It doesn't matter what result we use, we just need a completed Task to convert this synchronous method to an async one. The object is actually converted from Task<whatever> to Task so the result of the task is ignored anyway.
The same task/result is used here: https://github.com/xamarin/Xamarin.Forms/blob/master/Xamarin.Forms.Core/NavigationProxy.cs#L201
There was a problem hiding this comment.
If there is an existing guideline or practice how this should be handled in the Forms repo please let me know and I'll change it.
|
Hum, this is a breaking change.. .. not sure we can accept this. |
|
@rmarinho I can't see where this should break any apps - especially since the currrent implementation just crashes when the image can't be loaded. Could you please explain? |
|
@dominik-weber if anyone is implementing the IAppLinks interface they will have to change the code. Can we try fix the image loading only ? |
|
Closing for now. I will revise this and try to fix the loading image issue |
|
@dominik-weber, |
Description of Change
With the current implementation the app crashes when the thumbnail url can't be loaded on iOS. This change makes the RegisterLink etc. methods awaitable (return Task) so the dev can catch exceptions and avoid unhandled exception crashes.
Bugs Fixed
API Changes
Changed:
Behavioral Changes
None
PR Checklist