Skip to content
This repository has been archived by the owner on May 15, 2024. It is now read-only.

Exceptions #19

Closed
Redth opened this issue Feb 24, 2018 · 6 comments
Closed

Exceptions #19

Redth opened this issue Feb 24, 2018 · 6 comments

Comments

@Redth
Copy link
Member

Redth commented Feb 24, 2018

There are some patterns in several API's that we might want to have some common exception types for.

Feature Availability

Things like GPS, Cameras, Phone, SMS, etc are not always available on a given device. When a method is called which tries to make use of such a device, we should throw a consistent exception to let the developer know the feature is unsupported on the current device:

NotSupportedOnDeviceException : NotSupportedException

There may also be the case where a feature is technically supported by the device, but not enabled:

NotEnabledOnDeviceException : InvalidOperationException

Open to ideas on naming, and other common exceptions we may require...

@Redth Redth added this to the Preview-1 milestone Feb 24, 2018
@mattleibow
Copy link
Contributor

mattleibow commented Feb 24, 2018

What if you use the system exceptions as the base types:

  • NotSupportedOnDeviceException : NotSupportedException
  • NotEnabledOnDeviceException : InvalidOperationException

The benefit of this is that it allows for some "typical" usage as well as using what the user already knows:

try {
    var result = await api.DoSomethingAsync();
} catch (NotSupportedException) {
    ShowAlert("This device does not do that.");
}

There is no need to force the user to use the custom exceptions, when the base, existing exception may cover more cases in one.

@Redth
Copy link
Member Author

Redth commented Feb 24, 2018

Yes!

@mattleibow
Copy link
Contributor

I think we also need to investigate the name of the exceptions: NotSupportedOnDevice vs DeviceNotSupported vs FeatureNotSupported.

This was referenced Feb 25, 2018
@f1nzer
Copy link
Contributor

f1nzer commented Feb 27, 2018

We might use capability prefix for that exceptions:
CapabilityNotSupportedException and CapabilityNotEnabledException.

@mattleibow mattleibow removed their assignment Mar 1, 2018
@Redth Redth added this to New in Triage Mar 7, 2018
@Redth Redth added this to In Progress in v0.5.0-preview Mar 7, 2018
Triage automation moved this from New to Closed Mar 26, 2018
@Redth Redth moved this from In Progress to Done in v0.5.0-preview Mar 27, 2018
@mattleibow mattleibow moved this from Done to Closed in v0.5.0-preview Apr 26, 2018
@mattleibow mattleibow reopened this Dec 22, 2018
@mattleibow
Copy link
Contributor

@jamesmontemagno @Redth we need to properly decide what exceptions we throw in cases, and when we just do nothing.

We currently have different ways of indicating that nothing can be done:

  • do nothing
  • throw FeatureNotSupportedException
  • throw PlatformNotSupportedException
  • throw FeatureNotEnabledException

One case where we do nothing is in Android's energy saver API: if we are not Lollipop, then we just fall through:
https://github.com/xamarin/Essentials/blob/1.0.0/Xamarin.Essentials/Battery/Battery.android.cs#L12-L48

One case where we throw FeatureNotSupportedException is with the sensors: if there is no sensor X, then we throw.

One case (and the only case so far) where we throw PlatformNotSupportedException is in Android's text-to-speech API: if we try and speak, but we couldn't initialize, then we throw.

So far, I was able to determine that we throw FeatureNotSupportedException for:

  • the sensors on all platforms if we aren't able to access the hardware
    • we throw in the start and the stop (this one may be overkill, we can probably first check to see if it is started, and if not then just do nothing)
  • the Android external browser if there was no browser installed
  • the email API
    • Android: if there is no message/rfc822 intent handler
    • iOS: (if the mail VC can't send, or if the mailto: doesn't have an app, or if trying to send HTML over the mailto: protocol
    • UWP: if the EmailManager is not available, or if trying to send HTML
  • the flashlight API on all platforms if there is no camera flash hardware
  • the phone dialler
    • Android / iOS: if the OS can't handle the tel: protocol
    • UWP: the PhoneCallManager is missing
  • the sms API
    • Android: if there is no smsto: intent handler
    • iOS: (if the message VC can't send
    • UWP: if the ChatMessageManager is not available
  • the vibration API on UWP if the VibrationDevice is not available or if no hardware was found

We throw a PlatformNotSupportedException for:

  • Android when we aren't able to initialize the text-to-speech engine

We throw a FeatureNotEnabledException for:

  • Geolocation if no providers are found

We do "nothing":

  • the Vibration API on iOS and android never actually checks, it just starts it
  • the Map API on Android and UWP just starts the URI, assuming that something will be there
  • the Geolocation API always assumes that there is a GPS and throws a FeatureNotEnabledException if there was no way to get the hardware
  • the KeepScreenOn feature just assumes the window flag will be honoured (probably is, but is there an api level/hardware limit?)
  • the energy saver API on android pre-Lollipop

@Redth Redth added this to Needs triage in Triage Dec 23, 2018
@Mrnikbobjeff
Copy link
Contributor

I am currently working on #178 where a variety of things can go wrong depending on the Platform. I would love to have a general PlatformException or similar to be able to throw a consistent Exception across Platforms with a corresponding inner exception for certain failures. This may be due to exceeding the recording limit on uwp or the failure to set a category for an iOS audio stream. I would love some input into what exception throwing strategy to follow there.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
Triage
  
Closed
Triage
  
Closed
v0.5.0-preview
  
Closed
Development

No branches or pull requests

5 participants