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.

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

WASM: Use BrowserHttpMessageHandler when available #9941

Closed
SteveSandersonMS opened this issue Aug 7, 2018 · 8 comments

Comments

@SteveSandersonMS
Copy link

@SteveSandersonMS SteveSandersonMS commented Aug 7, 2018

As just discussed with @kumpera, @marek-safar, @kjpou1

Rodrigo's proposal is that the Mono WebAssembly BCL can use reflection to check at runtime whether the assembly Microsoft.AspNetCore.Blazor.Browser is loadable, and if so, load the type Microsoft.AspNetCore.Blazor.Browser.Http.BrowserHttpMessageHandler from it, and use that as the default System.Net.Http.HttpMessageHandler subclass for HttpClient. It doesn't take any constructor params, so you can just Activator.CreateInstance(thatType) it.

For reference, the sources for BrowserHttpMessageHandler are at https://github.com/aspnet/Blazor/blob/master/src/Microsoft.AspNetCore.Blazor.Browser/Http/BrowserHttpMessageHandler.cs

More difficult alternative: if you want, Mono could take the BrowserHttpMessageHandler sources and use them directly in the Mono WebAssembly BCL. However if you follow this approach, it will necessitate having a reference from the BCL to the Mono.WebAssembly.Interop package (or @kjpou1's version of that assembly, if it's now ready to use). This is fine with me, but it's a more involved solution than Rodrigo's proposal above to reference BrowserHttpMessageHandler dynamically via reflection. It also still would only work in the context of Blazor, unless you also extract the corresponding JS-side code and merge that into Mono WebAssembly too. Plus you'd want to get all the tests and have a way of running them too. So I think in the short term at least, the simple dynamic-reflection-loading technique may be much better!

cc @danroth @rynowak

@Suchiman

This comment has been minimized.

Copy link
Contributor

@Suchiman Suchiman commented Aug 8, 2018

Related to #7933

@jeromelaban

This comment has been minimized.

Copy link
Contributor

@jeromelaban jeromelaban commented Aug 10, 2018

This direction is a bit too tied to a specific framework/library implementation. Could the implementation be based on something like this instead:

MethodInfo method = type.GetMethod ("GetHttpMessageHandler", BindingFlags.Static | BindingFlags.NonPublic);

The source of the issue is still that the default HttpHandler cannot be changed globally in the .net (or .NET standard) API, and it should be possible to do so (/cc @terrajobst). Many .NET Standard compatible libraries don't provide a way to change the default handler used when making http calls, and this can complicate portability.

Also, Uno implements its own handler this way WasmHttpHandler.cs, used like this in apps.

@marek-safar marek-safar self-assigned this Aug 10, 2018
@marek-safar marek-safar added the task label Aug 10, 2018
@marek-safar marek-safar added this to To do in Web Assembly Support via automation Aug 10, 2018
@SteveSandersonMS

This comment has been minimized.

Copy link
Author

@SteveSandersonMS SteveSandersonMS commented Aug 10, 2018

@jeromelaban Yes, we know it's specific to one implementation. It's just a quick and simple tactical fix we already discussed with the Mono team (@kumpera etc.).

@carldebilly

This comment has been minimized.

Copy link

@carldebilly carldebilly commented Aug 10, 2018

A nicer solution would be to add a "default handler provider" in mono, allowing any wasm framework (blazor, Ooui, Uno, ...) to inject appropriate factory in its bootstrapping.

@kumpera

This comment has been minimized.

Copy link
Member

@kumpera kumpera commented Aug 10, 2018

We're working on a solution that would provide a HttpClient that works OOTB.
We can make this change in such a way that it would work for everyone else as well.

@kumpera

This comment has been minimized.

Copy link
Member

@kumpera kumpera commented Aug 10, 2018

cc @kjpou1

marek-safar added a commit that referenced this issue Aug 29, 2018
- Add a private static variable to HttpClient called `Func<HttpMessageHandler> GetHttpMessageHandler` that will be used to obtain the default message handler and can be set for example as follows:

```
		var httpMessageHandler = typeof(HttpClient).GetField("GetHttpMessageHandler", 
                            BindingFlags.Static | 
                            BindingFlags.NonPublic);

                httpMessageHandler.SetValue(null, (Func<HttpClientHandler>) (() => {
			return new MyHttpClientHandler ();
		}));
```

- If a handler is not set or returns null the default implementation HttpClientHandler will be returned.

- Add HttpClient.wasm.cs implementation to handle the default HttpClient
- Add simple test to the wasm test harness bindings-test.
- Reference #9941



<!--
Thank you for your Pull Request!

If you are new to contributing to Mono, please try to do your best at conforming to our coding guidelines http://www.mono-project.com/community/contributing/coding-guidelines/ but don't worry if you get something wrong. One of the project members will help you to get things landed.

Does your pull request fix any of the existing issues? Please use the following format: Fixes #issue-number
-->
EgorBo added a commit to EgorBo/mono that referenced this issue Sep 10, 2018
- Add a private static variable to HttpClient called `Func<HttpMessageHandler> GetHttpMessageHandler` that will be used to obtain the default message handler and can be set for example as follows:

```
		var httpMessageHandler = typeof(HttpClient).GetField("GetHttpMessageHandler", 
                            BindingFlags.Static | 
                            BindingFlags.NonPublic);

                httpMessageHandler.SetValue(null, (Func<HttpClientHandler>) (() => {
			return new MyHttpClientHandler ();
		}));
```

- If a handler is not set or returns null the default implementation HttpClientHandler will be returned.

- Add HttpClient.wasm.cs implementation to handle the default HttpClient
- Add simple test to the wasm test harness bindings-test.
- Reference mono#9941



<!--
Thank you for your Pull Request!

If you are new to contributing to Mono, please try to do your best at conforming to our coding guidelines http://www.mono-project.com/community/contributing/coding-guidelines/ but don't worry if you get something wrong. One of the project members will help you to get things landed.

Does your pull request fix any of the existing issues? Please use the following format: Fixes #issue-number
-->
@kjpou1

This comment has been minimized.

Copy link
Member

@kjpou1 kjpou1 commented Sep 12, 2018

@SteveSandersonMS and @kumpera can this issue be marked as resolved?

@SteveSandersonMS

This comment has been minimized.

Copy link
Author

@SteveSandersonMS SteveSandersonMS commented Sep 12, 2018

Yes, thanks!

Web Assembly Support automation moved this from To do to Done Sep 12, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
7 participants
You can’t perform that action at this time.