Closed themronion closed 2 years ago
jamesmontemagno
commented
3 years ago Hmmm it shouldn't crash like this, which is interesting, but the simulator doesn't have a Dialer so that API is not allowed to be called and this could cause a catastrophic failure, which is out of our control and is iOS itself crashing your app.
You can check the DeviceInfo.DeviceType to see if it is a simulator or not and only do it on a physical device.
themronion
commented
3 years ago @jamesmontemagno it is a simulator, yes. Well, if it is out of your control then ok. I just thought that this is indeed a strange behaviour because the exception is not being caught. You can close the issue if it is not a concern for the library)
jamesmontemagno
commented
3 years ago I need to look at it a bit closer to see if we can handle it more gracefully :)
SailorTurkey
commented
3 years ago @jamesmontemagno I am pretty sure Essentials does check if dialing is available, because exception is thrown by Essentials not IOS: Xamarin.Essentials.FeatureNotSupportedException: Specified method is not supported.
May be Essentials should introduce a property / method, so instead of try/catch ing we can also precheck the condition? Something like the following, it should be trivial to implement, PhoneDialer library already has an internal IsSupported property. making it public can solve this problem, what do you think?
if(PhoneDialer.IsSupported)
PhoneDialer.Open(phone);
Silic0nS0ldier
commented
3 years ago There is a flaw in the iOS specific code which is causing these exceptions to go uncaught. That is, PhoneDialer.Open calls the platform specific method PhoneDialer.PlatformOpen which for iOS is marked as async (and critically is not awaited). This change was introduced in https://github.com/xamarin/Essentials/pull/1542
In theory, this issue could result in crashes on real iOS devices.
Reverting the changes made to PhoneDialer on iOS would fix this. I think it would also be worthwhile creating some automated tests to guard against these kinds of regressions in the future (I could have missed something, but none seem to exist that covers platform specific code).
jamesmontemagno
commented
3 years ago Good research here @Silic0nS0ldier <3
I am fine with moving it back to UIApplication.SharedApplication.OpenUrl(nsUrl); @mattleibow thoughts?
sunkerGit
commented
3 years ago public static bool TryCallContact(string contactNumber) { if (DeviceInfo.DeviceType == DeviceType.Physical) { PhoneDialer.Open(contactNumber); return true; } else { return false; } }
this will resolve the crash problem, the current problem has come from the emulator whereas the PhoneDialer method was crashed on an emulator, not the physical devices.
pgilhuijs
commented
3 years ago On physical device it crashes also
Silic0nS0ldier
commented
3 years ago Yep, as I said before the implementation is an async method. While the iOS simulator will crash, a physical iOS device could also crash (e.g. iPad and iPod Touch may not have a provider for calling functionality, iPhone may have an internal error, iOS rejects the API call citing in response to some newly added permission requirement, etc).
Regardless a potential solution has been identified (reverting some changes), someone just has to have a go at implementing it. (and presumably approval from Xamarin Essentials code owners)
chillNZ
commented
3 years ago I've ran into this bug also after upgrading and can confirm the behaviour Silic0nS0ldier reported above.Also note the official documentation example shows catch blocks being used which won't actually work on ios (the catch blocks never catch the exception and the whole app crashes, which is contrary to the implication in the example) - not sure if this needs to be updated or not: https://docs.microsoft.com/en-us/xamarin/essentials/phone-dialer?tabs=ios.
maxchu92
commented
3 years ago I am having the same issue as well. I tried catching the exception but it threw it in the Application class. I made sure my async await is correct.
sbalkhair
commented
3 years ago I am having the same issue as well. I tried catching the exception but it threw it in the Application class. I made sure my async await is correct.
same issue
mackayn
commented
2 years ago Seeing the same :(
You can use the launcher though
var canLaunch = await _launcher.CanOpenAsync(SupportNumber);
if (canLaunch)
{
await _launcher.OpenAsync(SupportNumber);
angelru
commented
2 years ago How do we solve it? same issue
benlings
commented
2 years ago I'm seeing the same crash. Looks like this is because PhoneDialer.PlatformOpen is async void and not awaited when its called, so the exceptions thrown in ValidateOpen aren't passed back to the caller. I think the fix would be to remove the async from the method definition and the await from the last line of the function. https://github.com/xamarin/Essentials/blob/main/Xamarin.Essentials/PhoneDialer/PhoneDialer.ios.cs#L14-L20
TonyLugg
commented
2 years ago Any sign of a fix for this? I'm using Essentials 1.7.1 and it is still crashing.
trevoriancox
commented
2 years ago Another test case to reproduce this on a simulator or real iOS device:
PhoneDialer.Open("5555551212#hash");
It will throw FeatureNotSupportedException in a different thread.
jfversluis
commented
2 years ago Confirmed that @benlings solution surfaces the exception
Steps to Reproduce
I am aware the apple didnt implement phone calls and sms on their simulator but i dont understand why this code still crashes. The exception is being thrown and it must be catched in the catch block. Maybe there is something else going on there (maybe that has something to do with Hot Reload)? Give it a look please, maybe it is a bug?
Even though try catch is present is the code the app will crash with the fllowing stack trace: