-
Notifications
You must be signed in to change notification settings - Fork 559
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
Add support to use externally provided HttpClient/HttpClientHandler #2400
Comments
I don't think setting a single handler is compatible with the current implementations which assume ownership of the handler. It might be easier if it were possible to supply instead a factory to create and customise |
@Tragedian, the difficulty with a factory is it should return an instance of HttpMessageHandler which then means we can't set anything like credentials or certificate validation callbacks. What do you think about instead having a mechanism which specifies a Type which must derive from DelegatingHandler? We could look for either a default constructor (in which case we construct an instance and then call the setter on InnerHandler with our HttpClientHandler) or a constructor which takes an HttpMessageHandler so we can pass our configured HttpClientHandler to the constructor. The implementation would be free to ignore the passed in HttpClientHandler if it wanted to use a completely different implementation or could modify it or simply modify the request in the SendAsync call path. |
Thinking about it a bit more, it could be a bit simpler than that. We could use a |
The problem with this (and any approach I could imagine) is where the HTTP abstractions are built to work on |
Assuming |
@Tragedian, WCF doesn't touch anything on
Is there anything about having a delegate which looks like |
My only questions:
|
It would still be a |
Under this, if there were two behaviours which attempted to modify the handler, the expected result will be to get an exception thrown when trying to add the second configure method to the binding parameters. That doesn't seem like an easy situation to diagnose if a user has just added two behaviours which conflict. Can we do better? Knowing to add a |
We have the constraint that we can't add any explicit api's as any new api's also need to be added to the desktop full framework. As the full framework doesn't use HttpClient, that's a problem. Do you have any other suggestions on how to do this? |
The constraint on not being able to add any explicit new APIs certainly makes anything other than some kind of The scenario-test-as-an-example sounds like a decent way to document the new behaviour with some "best practice". It wouldn't even have to do something as complex as chaining (I actually can't picture how that would work with the asymmetric |
We do normally have the option of adding an API, the problem we have though is with how different the inner workings are between .Net Core and the full framework for HTTP. If there were a parallel for NetTcp, we could probably add an API. But semantically binding parameters are the right way to do this anyway. It's how you provide parameters which a binding element would pick up to modify the behavior of the binding element. |
Nice guys, what about passing cancellation token to wcf call after HttpClient is bound ? |
@fsielimowicz, can you explain more about what you mean? We do pass a cancellation token to HttpClient.SendAsync which will be canceled when the send timeout/operation timeout duration has passed. |
API's involved
There are 2 existing API mechanisms which could be used to achieve this.
BindingParameterCollection
A developer could create a class which implements
IEndpointBehavior
. In theAddBindingParameters
method, they would add the relevant instance to theBindingParameterCollection
. This would look like this:This is a naïve implementation, e.g.
HttpClient
needs to have timeouts disabled to prevent contention on the global TimerManager, but it shows the general pattern.MessageProperties
A developer could add an outgoing message property for the relevant instance to the
OutgoingMessageProperties
of anOperationContext
. This could be done either with a class which implementsIClientMessageInspector
or with the use ofOperationContextScope
. The latter would look like this:Which class(es) to support
We have multiple options about which class(es) we could potentially support being added to a
BindingParameterCollection
orOutgoingMessageProperties
, each having their own limitations. We would be unable to modify any parameters on any passed in objects for 2 reasons. 1) Any property we might set could potentially be something which is intended to be overridden. 2) There are use cases where the passed in object might have already been used so is now immutable.System.Net.Http.HttpClient
WCF sets
HttpClient.Timeout
to infinite to prevent a timer being registered for each request. We have logic to coalesce CancellationToken timers to prevent high contention on the global timer queue. Without setting this value, HttpClient will cause a lot of contention.System.Net.Http.HttpMessageInvoker
This is the base class to
HttpClient
. It lacks a lot of the helper api's thatHttpClient
provides such as the verb specific request api's, e.g.GetAsync
andPostAsync
. WCF only needs to useSendAsync
so this shouldn't be an issue. Although we don't currently do this, WCF could make use ofHttpClient.DefaultRequestHeaders
for the headers which will always be the same for all requests from the sameHttpChannelFactory
. If we decide to support the use of a single instance ofHttpClient
with multipleChannelFactory
instances, then we can't useDefaultRequestHeaders
.System.Net.Http.HttpClientHandler
There are many properties on this class which WCF sets. One of the more important ones is
Credentials
. A developer would need to set the credentials onHttpClientHandler
themselves. We would still be able to set any properties on theHttpClient
instance we would create to wrap theHttpClientHandler
. Supporting allowing this class to be provided would limit developers to using client implementations which ship with the framework.System.Net.Http.HttpMessageHandler
This is the base class for
HttpClientHandler
and is the type that the constructor forHttpClient
accepts. This has none of the properties thatHttpClientHandler
exposes, but as we can't modify any properties as explained earlier, this does not present any additional problems. This would enable developers to provide any implementation of Http which works with HttpClient.System.Net.Http.DelegatingHandler
This is a special class which allows to provide an existing
HttpMessageHandler
to be wrapped and potentially provide custom behavior. There is a propertyInnerHandler
which would allow WCF to set theHttpClientHandler
that we create on theDelegatingHandler
. Presuming the constraint of only being able to give WCF a delegating handler which has not been used yet, we would be able to provide an unusedHttpClientHandler
to the delegating handler which would be able to modify any properties before first usage. This would allow WCF to set credentials etc on anHttpClientHandler
while also allowing a developer to modify everything as much as they wish.Decisions to be made
ChannelFactory
's?The text was updated successfully, but these errors were encountered: