Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Emit a warning if AndroidHttpClientHandlerType is invalid #7326

Closed
jonpryor opened this issue Sep 1, 2022 · 0 comments · Fixed by #7448
Closed

Emit a warning if AndroidHttpClientHandlerType is invalid #7326

jonpryor opened this issue Sep 1, 2022 · 0 comments · Fixed by #7448
Assignees
@dellis1972 @jonathanpeppers
Labels
Area: App+Library Build Issues when building Library projects or Application projects. enhancement Proposed change to current functionality.
Milestone
.NET 8

Comments

@jonpryor
Copy link
Member

jonpryor commented Sep 1, 2022

Android application type

Android for .NET (net6.0-android, etc.)

Affected platform version

.NET 6.0+

Description

See also: PR #7214.

If the $(AndroidHttpClientHandlerType) property is set to an "invalid" value, which can be an "unresolvable" type (Type.GetType() returns null) or a type which has HttpClientHandler as a base class, then the build should emit a warning.

Steps to Reproduce

Did you find any workaround?

PR #7214 has a runtime workaround. However, the warning message it produces is a runtime warning message.

A build-time warning should also be generated.

Relevant log output

No response
@jonpryor jonpryor added Area: App+Library Build Issues when building Library projects or Application projects. needs-triage Issues that need to be assigned. labels Sep 1, 2022
jonpryor pushed a commit that referenced this issue Sep 1, 2022
@simonrozsival
[Mono.Android] .NET 6+ & GetHttpMessageHandler & null, oh my! (#7214)
d3e8767 
Fixes: #6961
Fixes: #6949

If a .NET SDK for Android app is built with the
`$(AndroidHttpClientHandlerType)` property set to a type which does
not exist, such as the value `Xamarin.Android.Net.AndroidClientHandler`
used by *Classic* Xamarin.Android apps (!), then the app may crash
during startup:

	I MonoDroid: UNHANDLED EXCEPTION:
	I MonoDroid: Android.Runtime.JavaProxyThrowable: Exception of type 'Android.Runtime.JavaProxyThrowable' was thrown.
	I MonoDroid: 
	I MonoDroid:   --- End of managed Android.Runtime.JavaProxyThrowable stack trace ---
	I MonoDroid: android.runtime.JavaProxyThrowable: System.NullReferenceException: Object reference not set to an instance of an object.
	I MonoDroid:    at System.Net.Http.HttpClientHandler.SendAsync(HttpRequestMessage request, CancellationToken cancellationToken)
	I MonoDroid:    at System.Net.Http.HttpMessageInvoker.SendAsync(HttpRequestMessage request, CancellationToken cancellationToken)
	I MonoDroid:    at System.Net.Http.HttpClient.<>n__0(HttpRequestMessage request, CancellationToken cancellationToken)
	I MonoDroid:    at System.Net.Http.HttpClient.<SendAsync>g__Core|83_0(HttpRequestMessage request, HttpCompletionOption completionOption, CancellationTokenSource cts, Boolean disposeCts, CancellationTokenSource pendingRequestsCts, CancellationToken originalCancellationToken)
	I MonoDroid:    at NetworkTest.MainPage.MakeRequest()
	I MonoDroid:    at NetworkTest.MainPage.OnCounterClicked(Object sender, EventArgs e)
	I MonoDroid:    at System.Threading.Tasks.Task.<>c.<ThrowAsync>b__128_0(Object state)
	I MonoDroid:    at Android.App.SyncContext.<>c__DisplayClass2_0.<Post>b__0()
	I MonoDroid:    at Java.Lang.Thread.RunnableImplementor.Run()
	I MonoDroid:    at Java.Lang.IRunnableInvoker.n_Run(IntPtr jnienv, IntPtr native__this)
	I MonoDroid:    at Android.Runtime.JNINativeWrapper.Wrap_JniMarshal_PP_V(_JniMarshal_PP_V callback, IntPtr jnienv, IntPtr klazz)
	I MonoDroid:     at mono.java.lang.RunnableImplementor.n_run(Native Method)
	I MonoDroid:     at mono.java.lang.RunnableImplementor.run(RunnableImplementor.java:30)
	I MonoDroid:     at android.os.Handler.handleCallback(Handler.java:938)
	I MonoDroid:     at android.os.Handler.dispatchMessage(Handler.java:99)
	I MonoDroid:     at android.os.Looper.loopOnce(Looper.java:201)
	I MonoDroid:     at android.os.Looper.loop(Looper.java:288)
	I MonoDroid:     at android.app.ActivityThread.main(ActivityThread.java:7870)
	I MonoDroid:     at java.lang.reflect.Method.invoke(Native Method)
	I MonoDroid:     at com.android.internal.os.RuntimeInit$MethodAndArgsCaller.run(RuntimeInit.java:548)
	I MonoDroid:     at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:1003)
	I MonoDroid: 
	I MonoDroid:   --- End of managed Android.Runtime.JavaProxyThrowable stack trace ---
	I MonoDroid: android.runtime.JavaProxyThrowable: System.NullReferenceException: Object reference not set to an instance of an object.
	I MonoDroid:    at System.Net.Http.HttpClientHandler.SendAsync(HttpRequestMessage request, CancellationToken cancellationToken)
	I MonoDroid:    at System.Net.Http.HttpMessageInvoker.SendAsync(HttpRequestMessage request, CancellationToken cancellationToken)
	I MonoDroid:    at System.Net.Http.HttpClient.<>n__0(HttpRequestMessage request, CancellationToken cancellationToken)
	I MonoDroid:    at System.Net.Http.HttpClient.<SendAsync>g__Core|83_0(HttpRequestMessage request, HttpCompletionOption completionOption, CancellationTokenSource cts, Boolean disposeCts, CancellationTokenSource pendingRequestsCts, CancellationToken originalCancellationToken)
	I MonoDroid:    at NetworkTest.MainPage.MakeRequest()
	I MonoDroid:    at NetworkTest.MainPage.OnCounterClicked(Object sender, EventArgs e)
	I MonoDroid:    at System.Threading.Tasks.Task.<>c.<ThrowAsync>b__128_0(Object state)
	I MonoDroid:    at Android.App.SyncContext.<>c__DisplayClass2_0.<Post>b__0()
	I MonoDroid:    at Java.Lang.Thread.RunnableImplementor.Run()
	I MonoDroid:    at Java.Lang.IRunnableInvoker.n_Run(IntPtr jnienv, IntPtr native__this)
	I MonoDroid:    at Android.Runtime.JNINativeWrapper.Wrap_JniMarshal_PP_V(_JniMarshal_PP_V callback, IntPtr jnienv, IntPtr klazz)
	I MonoDroid:     at mono.java.lang.RunnableImplementor.n_run(Native Method)
	I MonoDroid:     at mono.java.lang.RunnableImplementor.run(RunnableImplementor.java:30)
	I MonoDroid:     at android.os.Handler.handleCallback(Handler.java:938)
	I MonoDroid:     at android.os.Handler.dispatchMessage(Handler.java:99)
	I MonoDroid:     at android.os.Looper.loopOnce(Looper.java:201)
	I MonoDroid:     at android.os.Looper.loop(Looper.java:288)
	I MonoDroid:     at android.app.ActivityThread.main(ActivityThread.java:7870)
	I MonoDroid:     at java.lang.reflect.Method.invoke(Native Method)
	I MonoDroid:     at com.android.internal.os.RuntimeInit$MethodAndArgsCaller.run(RuntimeInit.java:548)
	I MonoDroid:     at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:1003)

The cause of the crash is that the
[`System.Net.Http.HttpClientHandler` default constructor][0] calls
[`CreateNativeHandler()`][1], which uses Reflection to call
`AndroidEnvironment.GetHttpMessageHandler()`, in order to obtain the
native HTTP handler (when `$(UseNativeHttpHandler)`=true).

The object that is returned depends on the value of the
`$(AndroidHttpClientHandlerType)` property.

When the type mentioned by `$(AndroidHttpClientHandlerType)` can't be
found, then `GetHttpMessageHandler()` will return `null`.  This isn't
immediately caught by `HttpClientHandler`, leading to a confusing
`NullReferenceException` later on.

When the type mentioned by `$(AndroidHttpClientHandlerType)` is a
type which inherits `HttpClientHandler` -- which includes the Classic
default value of `AndroidClientHandler`! -- then the app enters
infinite indirect recursion because the `HttpClientHandler`'s
constructor calls the `GetHttpMessageHandler` function which calls
the `HttpClientHandler` constructor which calls…

I base this PR on the implementation of Xamarin.iOS where we never
return `null`, and when the value is invalid we use the default handler
type as a fallback.  Whenever the type is invalid (`null` or derived
from `HttpClientHandler`), the `AndroidMessageHandler` class is used
instead and a warning is logged.

The existing docs for `$(AndroidHttpClientHandlerType)` are not
applicable for .NET SDK for Android, as they suggest setting the
property to `AndroidClientHandler` or `HttpClientHandler`.

Update the documentation for `$(AndroidHttpClientHandlerType)` to
explicitly call out .NET 6+ behavior, and that
`Xamarin.Android.Net.AndroidMessageHandler` (1e5bfa3) or
`System.Net.Http.SocketsHttpHandler, System.Net.Http` should be  used
instead.

TODO: [Emit a build-time warning][2] if
`$(AndroidHttpClientHandlerType)` is set to an invalid value

[0]: https://github.com/dotnet/runtime/blob/7a45201181d37329b7ab0bee541ed7c42b5d94b6/src/libraries/System.Net.Http/src/System/Net/Http/HttpClientHandler.AnyMobile.cs#L41
[1]: https://github.com/dotnet/runtime/blob/7a45201181d37329b7ab0bee541ed7c42b5d94b6/src/libraries/System.Net.Http/src/System/Net/Http/HttpClientHandler.AnyMobile.InvokeNativeHandler.cs#L149-L158
[2]: #7326
@jonpryor jonpryor added this to the .NET 7 milestone Sep 1, 2022
@jpobst jpobst added enhancement Proposed change to current functionality. and removed needs-triage Issues that need to be assigned. labels Sep 6, 2022
@jonpryor jonpryor modified the milestones: .NET 7, .NET 8 Sep 26, 2022
dellis1972 added a commit to dellis1972/xamarin-android that referenced this issue Oct 10, 2022
@dellis1972
[Xamarin.Android.Build.Tasks] Add XA1031 error for `AndroidHttpClient…
ed9f7fc 
…HandlerType`

Fixes dotnet#7326

We should check the value of the `AndroidHttpClientHandlerType` property
during the build process. This property can have a number of
different values depending on what version of Xamarin.Android is being
used.

Under Classic a valid value is `Xamarin.Android.Net.AndroidClientHandler`,
however under .NET 6+ the valid values are

- `Xamarin.Android.Net.AndroidMessageHandler`.
- `System.Net.Http.SocketsHttpHandler, System.Net.Http`.

We should raise an error if the user is using an invalid
value.
@dellis1972 dellis1972 mentioned this issue Oct 10, 2022
Merged
dellis1972 added a commit to dellis1972/xamarin-android that referenced this issue Oct 11, 2022
@dellis1972
[Xamarin.Android.Build.Tasks] Add XA1031 error for `AndroidHttpClient…
983568e 
…HandlerType`

Fixes dotnet#7326

We should check the value of the `AndroidHttpClientHandlerType` property
during the build process. This property can have a number of
different values depending on what version of Xamarin.Android is being
used.

Under Classic a valid value is `Xamarin.Android.Net.AndroidClientHandler`,
however under .NET 6+ the valid values are

- `Xamarin.Android.Net.AndroidMessageHandler`.
- `System.Net.Http.SocketsHttpHandler, System.Net.Http`.

We should raise an error if the user is using an invalid
value.
dellis1972 added a commit to dellis1972/xamarin-android that referenced this issue Oct 17, 2022
@dellis1972
[Xamarin.Android.Build.Tasks] Add XA1031 error for `AndroidHttpClient…
6181e5b 
…HandlerType`

Fixes dotnet#7326

We should check the value of the `AndroidHttpClientHandlerType` property
during the build process. This property can have a number of
different values depending on what version of Xamarin.Android is being
used.

Under Classic a valid value is `Xamarin.Android.Net.AndroidClientHandler`,
however under .NET 6+ the valid values are

- `Xamarin.Android.Net.AndroidMessageHandler`.
- `System.Net.Http.SocketsHttpHandler, System.Net.Http`.

We should raise an error if the user is using an invalid
value.
dellis1972 added a commit to dellis1972/xamarin-android that referenced this issue Nov 4, 2022
@dellis1972
[Xamarin.Android.Build.Tasks] Add XA1031 error for `AndroidHttpClient…
671735a 
…HandlerType`

Fixes dotnet#7326

We should check the value of the `AndroidHttpClientHandlerType` property
during the build process. This property can have a number of
different values depending on what version of Xamarin.Android is being
used.

Under Classic a valid value is `Xamarin.Android.Net.AndroidClientHandler`,
however under .NET 6+ the valid values are

- `Xamarin.Android.Net.AndroidMessageHandler`.
- `System.Net.Http.SocketsHttpHandler, System.Net.Http`.

We should raise an error if the user is using an invalid
value.
dellis1972 added a commit to dellis1972/xamarin-android that referenced this issue Nov 7, 2022
@dellis1972
[Xamarin.Android.Build.Tasks] Add XA1031 error for `AndroidHttpClient…
7cb9456 
…HandlerType`

Fixes dotnet#7326

We should check the value of the `AndroidHttpClientHandlerType` property
during the build process. This property can have a number of
different values depending on what version of Xamarin.Android is being
used.

Under Classic a valid value is `Xamarin.Android.Net.AndroidClientHandler`,
however under .NET 6+ the valid values are

- `Xamarin.Android.Net.AndroidMessageHandler`.
- `System.Net.Http.SocketsHttpHandler, System.Net.Http`.

We should raise an error if the user is using an invalid
value.
dellis1972 added a commit to dellis1972/xamarin-android that referenced this issue Nov 8, 2022
@dellis1972
[Xamarin.Android.Build.Tasks] Add XA1031 error for `AndroidHttpClient…
e4bf1af 
…HandlerType`

Fixes dotnet#7326

We should check the value of the `AndroidHttpClientHandlerType` property
during the build process. This property can have a number of
different values depending on what version of Xamarin.Android is being
used.

Under Classic a valid value is `Xamarin.Android.Net.AndroidClientHandler`,
however under .NET 6+ the valid values are

- `Xamarin.Android.Net.AndroidMessageHandler`.
- `System.Net.Http.SocketsHttpHandler, System.Net.Http`.

We should raise an error if the user is using an invalid
value.
dellis1972 added a commit to dellis1972/xamarin-android that referenced this issue Nov 11, 2022
@dellis1972
[Xamarin.Android.Build.Tasks] Add XA1031 error for `AndroidHttpClient…
6b31fed 
…HandlerType`

Fixes dotnet#7326

We should check the value of the `AndroidHttpClientHandlerType` property
during the build process. This property can have a number of
different values depending on what version of Xamarin.Android is being
used.

Under Classic a valid value is `Xamarin.Android.Net.AndroidClientHandler`,
however under .NET 6+ the valid values are

- `Xamarin.Android.Net.AndroidMessageHandler`.
- `System.Net.Http.SocketsHttpHandler, System.Net.Http`.

We should raise an error if the user is using an invalid
value.
dellis1972 added a commit to dellis1972/xamarin-android that referenced this issue Nov 14, 2022
@dellis1972
[Xamarin.Android.Build.Tasks] Add XA1031 error for `AndroidHttpClient…
469ada9 
…HandlerType`

Fixes dotnet#7326

We should check the value of the `AndroidHttpClientHandlerType` property
during the build process. This property can have a number of
different values depending on what version of Xamarin.Android is being
used.

Under Classic a valid value is `Xamarin.Android.Net.AndroidClientHandler`,
however under .NET 6+ the valid values are

- `Xamarin.Android.Net.AndroidMessageHandler`.
- `System.Net.Http.SocketsHttpHandler, System.Net.Http`.

We should raise an error if the user is using an invalid
value.
dellis1972 added a commit to dellis1972/xamarin-android that referenced this issue Nov 15, 2022
@dellis1972
[Xamarin.Android.Build.Tasks] Add XA1031 error for `AndroidHttpClient…
1b5ca05 
…HandlerType`

Fixes dotnet#7326

We should check the value of the `AndroidHttpClientHandlerType` property
during the build process. This property can have a number of
different values depending on what version of Xamarin.Android is being
used.

Under Classic a valid value is `Xamarin.Android.Net.AndroidClientHandler`,
however under .NET 6+ the valid values are

- `Xamarin.Android.Net.AndroidMessageHandler`.
- `System.Net.Http.SocketsHttpHandler, System.Net.Http`.

We should raise an error if the user is using an invalid
value.
dellis1972 added a commit to dellis1972/xamarin-android that referenced this issue Nov 16, 2022
@dellis1972
[Xamarin.Android.Build.Tasks] Add XA1031 error for `AndroidHttpClient…
31a5275 
…HandlerType`

Fixes dotnet#7326

We should check the value of the `AndroidHttpClientHandlerType` property
during the build process. This property can have a number of
different values depending on what version of Xamarin.Android is being
used.

Under Classic a valid value is `Xamarin.Android.Net.AndroidClientHandler`,
however under .NET 6+ the valid values are

- `Xamarin.Android.Net.AndroidMessageHandler`.
- `System.Net.Http.SocketsHttpHandler, System.Net.Http`.

We should raise an error if the user is using an invalid
value.
dellis1972 added a commit to dellis1972/xamarin-android that referenced this issue Nov 17, 2022
@dellis1972
[Xamarin.Android.Build.Tasks] Add XA1031 error for `AndroidHttpClient…
4124d42 
…HandlerType`

Fixes dotnet#7326

We should check the value of the `AndroidHttpClientHandlerType` property
during the build process. This property can have a number of
different values depending on what version of Xamarin.Android is being
used.

Under Classic a valid value is `Xamarin.Android.Net.AndroidClientHandler`,
however under .NET 6+ the valid values are

- `Xamarin.Android.Net.AndroidMessageHandler`.
- `System.Net.Http.SocketsHttpHandler, System.Net.Http`.

We should raise an error if the user is using an invalid
value.
dellis1972 added a commit to dellis1972/xamarin-android that referenced this issue Nov 22, 2022
@dellis1972
[Xamarin.Android.Build.Tasks] Add XA1031 error for `AndroidHttpClient…
ca2ecd0 
…HandlerType`

Fixes dotnet#7326

We should check the value of the `AndroidHttpClientHandlerType` property
during the build process. This property can have a number of
different values depending on what version of Xamarin.Android is being
used.

Under Classic a valid value is `Xamarin.Android.Net.AndroidClientHandler`,
however under .NET 6+ the valid values are

- `Xamarin.Android.Net.AndroidMessageHandler`.
- `System.Net.Http.SocketsHttpHandler, System.Net.Http`.

We should raise an error if the user is using an invalid
value.
dellis1972 added a commit to dellis1972/xamarin-android that referenced this issue Nov 22, 2022
@dellis1972
[Xamarin.Android.Build.Tasks] Add XA1031 error for `AndroidHttpClient…
1843712 
…HandlerType`

Fixes dotnet#7326

We should check the value of the `AndroidHttpClientHandlerType` property
during the build process. This property can have a number of
different values depending on what version of Xamarin.Android is being
used.

Under Classic a valid value is `Xamarin.Android.Net.AndroidClientHandler`,
however under .NET 6+ the valid values are

- `Xamarin.Android.Net.AndroidMessageHandler`.
- `System.Net.Http.SocketsHttpHandler, System.Net.Http`.

We should raise an error if the user is using an invalid
value.
dellis1972 added a commit to dellis1972/xamarin-android that referenced this issue Nov 30, 2022
@dellis1972
[Xamarin.Android.Build.Tasks] Add XA1031 error for `AndroidHttpClient…
43d7f89 
…HandlerType`

Fixes dotnet#7326

We should check the value of the `AndroidHttpClientHandlerType` property
during the build process. This property can have a number of
different values depending on what version of Xamarin.Android is being
used.

Under Classic a valid value is `Xamarin.Android.Net.AndroidClientHandler`,
however under .NET 6+ the valid values are

- `Xamarin.Android.Net.AndroidMessageHandler`.
- `System.Net.Http.SocketsHttpHandler, System.Net.Http`.

We should raise an error if the user is using an invalid
value.
dellis1972 added a commit to dellis1972/xamarin-android that referenced this issue Dec 7, 2022
@dellis1972
[Xamarin.Android.Build.Tasks] Add XA1031 error for `AndroidHttpClient…
389ac6e 
…HandlerType`

Fixes dotnet#7326

We should check the value of the `AndroidHttpClientHandlerType` property
during the build process. This property can have a number of
different values depending on what version of Xamarin.Android is being
used.

Under Classic a valid value is `Xamarin.Android.Net.AndroidClientHandler`,
however under .NET 6+ the valid values are

- `Xamarin.Android.Net.AndroidMessageHandler`.
- `System.Net.Http.SocketsHttpHandler, System.Net.Http`.

We should raise an error if the user is using an invalid
value.
jonpryor pushed a commit that referenced this issue Dec 12, 2022
@dellis1972
[Xamarin.Android.Build.Tasks] Add XA1031 error (#7448)
311b41e 
Fixes: #7326

Commit d3e8767 added a requirement that, under .NET 6+, the
`$(AndroidHttpClientHandlerType)` MUST inherit from
[`System.Net.Http.HttpMessageHandler`][0].  This was unfortunately a
change from Classic, which allowed/required that
[`System.Net.Http.HttpClientHandler`][1] be the base class.
Allowing `HttpClientHandler` to be in the inheritance hierarchy for
`$(AndroidHttpClientHandlerType)` would result in a
`NullReferenceException` at runtime under .NET 6.

Commit d3e8767 contained a TODO:

> TODO: [Emit a build-time warning][2] if
> `$(AndroidHttpClientHandlerType)` is set to an invalid value

Implement this TODO: add a build-time check that verifies that
`$(AndroidHttpClientHandlerType)` is a valid type for the target,
e.g. is an `HttpMessageHandler` subclass on .NET 6.

If `$(AndroidHttpClientHandlerType)` is not a valid type, then an
XA1031 error is raised:

	error XA1031: The 'AndroidHttpClientHandlerType' has an invalid value of '{0}' please check your project settings.

Additionally, if `$(AndroidHttpClientHandlerType)` is
`System.Net.Http.SocketsHttpHandler, System.Net.Http`, *and*
`$(UseNativeHttpHandler)` isn't set, then
set `$(UseNativeHttpHandler)` to false.  This is necessary to ensure
that `SocketsHttpHandler` is preserved by the linker.

[0]: https://learn.microsoft.com/en-us/dotnet/api/system.net.http.httpmessagehandler?view=net-6.0
[1]: https://learn.microsoft.com/en-us/dotnet/api/system.net.http.httpclienthandler?view=net-6.0
[2]: #7326
@ghost ghost locked as resolved and limited conversation to collaborators Jan 12, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees

@dellis1972 dellis1972

@jonathanpeppers jonathanpeppers

Labels
Area: App+Library Build Issues when building Library projects or Application projects. enhancement Proposed change to current functionality.
Projects
None yet
Milestone
.NET 8
4 participants
@jonpryor @jpobst @dellis1972 @jonathanpeppers