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

Add SocketsHttpHandler.ConnectCallback #41949

Closed
geoffkizer opened this issue Sep 7, 2020 · 5 comments · Fixed by #41955
Closed

Add SocketsHttpHandler.ConnectCallback #41949

geoffkizer opened this issue Sep 7, 2020 · 5 comments · Fixed by #41955
Labels
api-approved API was approved in API review, it can be implemented area-System.Net.Http blocking-release
Milestone

Comments

@geoffkizer
Copy link
Contributor

Background and Motivation

We recently removed System.Net.Connections from 5.0. One feature we lost was the ConnectionFactory property on SocketsHttpHandler, which allowed you to plug in your own transport that SocketsHttpHandler would run over.

This new API provides the same capability to replace the transport without relying on System.Net.Connections. When you set this property, your callback will be invoked whenever SocketsHttpHandler needs to create a new connection. You can use this callback to create the Stream however you want -- e.g. use an in-memory stream, bind to a particular local address, etc.

Proposed API

public class SocketsHttpHandler
{
    public Func<DnsEndPoint, HttpRequestMessage, CancellationToken, ValueTask<Stream>>? ConnectCallback;
}

The DnsEndPoint argument identifies the server we are trying to connect to. The HttpRequestMessage argument allows you to inspect the initial HttpRequestMessage for this connection -- though you should be aware that the connection can be used for many requests, not just this initial one. We expect most users will only need the DnsEndPoint.

@geoffkizer geoffkizer added api-suggestion Early API idea and discussion, it is NOT ready for implementation area-System.Net.Http labels Sep 7, 2020
@geoffkizer geoffkizer added this to the 5.0.0 rc2 milestone Sep 7, 2020
@Dotnet-GitSync-Bot Dotnet-GitSync-Bot added the untriaged New issue has not been triaged by the area owner label Sep 7, 2020
@ghost
Copy link

ghost commented Sep 7, 2020

Tagging subscribers to this area: @dotnet/ncl
See info in area-owners.md if you want to be subscribed.

@geoffkizer geoffkizer added api-ready-for-review API is ready for review, it is NOT ready for implementation and removed api-suggestion Early API idea and discussion, it is NOT ready for implementation labels Sep 7, 2020
@scalablecory scalablecory added blocking Marks issues that we want to fast track in order to unblock other important work and removed untriaged New issue has not been triaged by the area owner labels Sep 8, 2020
@terrajobst
Copy link
Contributor

terrajobst commented Sep 8, 2020

Video

  • We should add a context class that passes in the parameters so that we can add more values over time
  • We originally had SocketsHttpConnectionContext as get/set POCO but that would mean that all properties would have to be nullable, which would be super annoying to use
namespace System.Net.Http
{
    public sealed class SocketsHttpConnectionContext
    {
        public DnsEndPoint DnsEndPoint { get; }
        public HttpRequestMessage RequestMessage { get; }
    }
    public sealed class SocketsHttpHandler : HttpMessageHandler
    {
        public Func<SocketsHttpConnectionContext, CancellationToken, ValueTask<Stream>>? ConnectCallback { get; set; }
    }
}

@terrajobst terrajobst added api-approved API was approved in API review, it can be implemented and removed api-ready-for-review API is ready for review, it is NOT ready for implementation blocking Marks issues that we want to fast track in order to unblock other important work labels Sep 8, 2020
@karelz karelz modified the milestones: 5.0.0 rc2, 5.0.0 Sep 9, 2020
@karelz
Copy link
Member

karelz commented Sep 10, 2020

Reopening this issue to wait for backport to 5.0 in PR #42075

@karelz karelz reopened this Sep 10, 2020
@geoffkizer
Copy link
Contributor Author

geoffkizer commented Sep 12, 2020

Backport to 5.0 RC2 is merged in PR #42075

@karelz
Copy link
Member

karelz commented Jan 26, 2021

Fixed in 6.0/master (PR #41955) and in 5.0 (PR #42075)

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
api-approved API was approved in API review, it can be implemented area-System.Net.Http blocking-release
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants