From 1e4503369d0128679e9b7116c62bd1d71eea3710 Mon Sep 17 00:00:00 2001 From: Marek Habersack Date: Sat, 28 Jan 2023 02:39:24 +0100 Subject: [PATCH] [Mono.Android] Wrap connection exceptions in HttpRequestException (#7661) Fixes: https://github.com/xamarin/xamarin-android/issues/7629 Whenever a Java backend connection fails for any reason, wrap the thrown exception in `HttpRequestException` as described in the [`HttpClient.SendAsync()` documentation][0]. [0]: https://learn.microsoft.com/en-us/dotnet/api/system.net.http.httpclient.sendasync?view=net-7.0#system-net-http-httpclient-sendasync(system-net-http-httprequestmessage) --- .../Xamarin.Android.Net/AndroidMessageHandler.cs | 6 +++++- .../Xamarin.Android.Net/AndroidMessageHandlerTests.cs | 6 +++++- 2 files changed, 10 insertions(+), 2 deletions(-) diff --git a/src/Mono.Android/Xamarin.Android.Net/AndroidMessageHandler.cs b/src/Mono.Android/Xamarin.Android.Net/AndroidMessageHandler.cs index 71d972d0a87..8971220b68b 100644 --- a/src/Mono.Android/Xamarin.Android.Net/AndroidMessageHandler.cs +++ b/src/Mono.Android/Xamarin.Android.Net/AndroidMessageHandler.cs @@ -494,7 +494,11 @@ Task ConnectAsync (HttpURLConnection httpConnection, CancellationToken ct) Logger.Log (LogLevel.Info, LOG_APP, $"Exception caught while cancelling connection: {ex}"); ct.ThrowIfCancellationRequested (); } - throw; + + // All exceptions related to connectivity should be wrapped in HttpRequestException. In theory it is possible that the exception will be + // thrown for another reason above, but it's OK to wrap it in HttpRequestException anyway, since we're working in the context of + // `ConnectAsync` which, from the end user's point of view, is 100% related to connectivity. + throw new HttpRequestException ("Connection failure", ex); } }, ct); } diff --git a/tests/Mono.Android-Tests/Xamarin.Android.Net/AndroidMessageHandlerTests.cs b/tests/Mono.Android-Tests/Xamarin.Android.Net/AndroidMessageHandlerTests.cs index 2f1517d57c6..c15b8292c36 100644 --- a/tests/Mono.Android-Tests/Xamarin.Android.Net/AndroidMessageHandlerTests.cs +++ b/tests/Mono.Android-Tests/Xamarin.Android.Net/AndroidMessageHandlerTests.cs @@ -139,10 +139,14 @@ private async Task AssertRejectsRemoteCertificate (Func makeRequest) Assert.Fail ("The request wasn't rejected"); } #if NET - catch (System.Net.WebException ex) {} + // While technically we should be throwing only HttpRequestException (as per HttpClient.SendAsync docs), in reality + // we need to consider legacy code that migrated to .NET and may still expect WebException. Thus, we throw both + // of these and we need to catch both here + catch (System.Net.WebException) {} #else catch (Java.IO.IOException) {} #endif + catch (System.Net.Http.HttpRequestException) {} } } }