-
Notifications
You must be signed in to change notification settings - Fork 9.8k
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
WebTransport API #42490
Comments
Nit: interface members don't need a public access modifier. |
Remove the Stream Override APIs from WebTransportStream's API summary, those don't need to be reviewed. |
API Review Notes:
API Approved! namespace Microsoft.AspNetCore.Http.Features;
+ public interface IWebTransportSession
+ {
+ public long SessionId { get; }
+ public void Abort(int errorCode = (int)Http3ErrorCode.NoError);
+ public ValueTask<ConnectionContext?> AcceptStreamAsync(CancellationToken cancellationToken = default);
+ public ValueTask<ConnectionContext> OpenUnidirectionalStreamAsync(CancellationToken cancellationToken = default);
+ }
+ public interface IHttpWebTransportFeature
+ {
+ public bool IsWebTransportRequest { get; }
+ [RequiresPreviewFeatures("WebTransport is a preview feature")]
+ public ValueTask<IWebTransportSession> AcceptAsync(CancellationToken cancellationToken = default);
+ } |
Closing; this went in for Preview 7. |
Reopening; need to reevaluate where we put the |
These entire interfaces need to be marked preview. Given that you need to opt-in to preview features to use this anyway, we should go all the way. Down the road, we might want different interfaces. We might want to use namespace Microsoft.AspNetCore.Http.Features;
+ [RequiresPreviewFeatures("WebTransport is a preview feature")]
public interface IWebTransportSession
{
public long SessionId { get; }
public void Abort(int errorCode = (int)Http3ErrorCode.NoError);
public ValueTask<ConnectionContext?> AcceptStreamAsync(CancellationToken cancellationToken = default);
public ValueTask<ConnectionContext> OpenUnidirectionalStreamAsync(CancellationToken cancellationToken = default);
}
+ [RequiresPreviewFeatures("WebTransport is a preview feature")]
public interface IHttpWebTransportFeature
{
public bool IsWebTransportRequest { get; }
- [RequiresPreviewFeatures("WebTransport is a preview feature")]
public ValueTask<IWebTransportSession> AcceptAsync(CancellationToken cancellationToken = default);
} |
Thank you for submitting this for API review. This will be reviewed by @dotnet/aspnet-api-review at the next meeting of the ASP.NET Core API Review group. Please ensure you take a look at the API review process documentation and ensure that:
|
Done in #43419 |
Background and Motivation
I am working on adding WebTransport support to Kestrel (PR #42097). That PR adds a few new public APIs.
Proposed API
Usage Examples
For a nearly-exhaustive piecewise view of how this API can be used you can refer to this doc: https://github.com/dotnet/aspnetcore/blob/c5e66c255c0254a9be191ee53031cec6b79def48/docs/WebTransport.md
Example 1
This example waits for a bidirectional stream. Once it receives one, it will read the data from it, reverse it and then write it back to the stream.
Example 2
This example opens a new stream from the server side and then sends data.
Alternative Designs
There are many other possible designs such as using
Action
andFunc
parameters when accepting and openning streams and sessions. Then those could be called when a new session or stream is openned instead of theasync/await
design. However, most of ASP.NET seems to be usingasync/await
.I also looked into making the
WebTransportStream
extend from theHttp3Stream
class. However, as WebTransport streams don't need all the framing logic, it just complicated the design. Also, technically WebTransport is defined over HTTP/2 as well. So, this would be incorrect if we wanted to add WebTransport over HTTP/2 in the future.Lastly, I also looked into separating the
WebTransportStream
class into aWebTransportinputStream
,WebTransportOutputStream
and aWebTransportBidirectionalStream
. This caused a lot of code duplication and unnecessary complexity though. Moreover, it caused me to need to restructure the HTTP/3 stream handling logic as non-bidirectional HTTP/3 streams are assumed to be control streams. This new proposed solution avoids all this added complexity.Risks
As WebTransport is hidden behind an
AppContext
switch and behind aPreviewFeature
annotation, the risks are only enabled once the user specifically opts-in. Having said that, this API is only adding new functionality and trying to minimize the changes to existing functionality.The text was updated successfully, but these errors were encountered: