Some of the new wrapped Java exceptions use BCL exceptions that differ from the related BCL types

[brendanzagaeski]

Context

60363ef added an option to wrap Java exception types in .NET exception types for certain Android APIs.

JavaDictionary.Get() is an example:

https://github.com/xamarin/xamarin-android/blob/8d7557aad23a3c0724e1996aade3072b1a09f298/src/Mono.Android/Android.Runtime/JavaDictionary.cs#L72-L86

Expected behavior

JavaDictionary inherits from Java.Lang.Object and implements IDictionary:

https://github.com/xamarin/xamarin-android/blob/8d7557aad23a3c0724e1996aade3072b1a09f298/src/Mono.Android/Android.Runtime/JavaDictionary.cs#L12

The documentation for the IDictionary.Item[Object] indexer property mentions two exceptions:

• ArgumentNullException
• NotSupportedException

Actual behavior

JavaDictionary.Get() can currently throw the following two different exceptions instead:

• InvalidCastException
• NullReferenceException

Depending on the goals of the Java exception wrapping, throwing different System exceptions from JavaDictionary.Get() than the ones documented for IDictionary.Item[Object] might be unintended. Maybe only the exception conditions documented for IDictionary.Item[Object] (like "key is null") need to be surfaced as System exceptions, and other exceptions, if any, can be left as Java exceptions?

Related scenarios

If this JavaDictionary.Get() scenario does look good to change, then there are probably a few additional similar cases to change too.

Another slightly different case is JavaCollection.Add(). I think it might have been wrapped because JavaCollection implements ICollection, but I just noticed today that the non-generic ICollection interface doesn't include an Add() method. Only the generic ICollection<T> includes an Add() method. So maybe JavaCollection.Add() doesn't need to wrap any exceptions?

[brendanzagaeski]

Additional background context: From a first quick chat with @jonpryor, it sounded like this might be unintentional, so I added an issue here to help with tracking potential changes to the behavior.

[brendanzagaeski]

Another related case is that with the new exception mode enabled Android.Runtime.InputStreamInvoker.Read() will at the moment throw things like:

• Java.Lang.IndexOutOfBoundsException instead of ArgumentException or ArgumentOutOfRangeException
• Java.Lang.NullPointerException instead of ArgumentNullException
• System.IO.IOException instead of ObjectDisposedException

[brendanzagaeski]

One more interesting case is AndroidClientHandler, which currently throws exceptions like Java.Net.UnknownHostException instead of System.Net.Http.HttpRequestException. I think that type might have been unintentionally overlooked in the first set of changes because it isn't a binding for an existing Android or Java type.

Adjusting AndroidClientHandler.SendAsync() to throw the documented excpetions for HttpClient.SendAsync() and HttpClientHandler.SendAsync() will fix #3216.