Skip to content

Abstract API: add server transport capability request option#146

Open
nakulbajaj wants to merge 5 commits into
apple:mainfrom
nakulbajaj:request-options-server-http-version
Open

Abstract API: add server transport capability request option#146
nakulbajaj wants to merge 5 commits into
apple:mainfrom
nakulbajaj:request-options-server-http-version

Conversation

@nakulbajaj
Copy link
Copy Markdown
Member

No description provided.

Comment thread Sources/NetworkTypes/HTTPVersion.swift Outdated
///
/// ``HTTPVersion`` provides type-safe access to supported HTTP protocol versions,
/// allowing clients and servers to communicate version capabilities.
public enum HTTPVersion: UInt8, Sendable, Hashable {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
public enum HTTPVersion: UInt8, Sendable, Hashable {
@nonexhaustive
public enum HTTPVersion: UInt8, Sendable, Hashable {

/// Providing server HTTP version information allows the client to optimize
/// connection establishment. For example, if a server is known to support HTTP/3,
/// the client can attempt a QUIC connection directly.
public protocol ServerHTTPVersionCapability: RequestOptions {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we include "Hint" in the name here

@nakulbajaj nakulbajaj force-pushed the request-options-server-http-version branch from 6746279 to 8a363cf Compare May 14, 2026 14:59
@nakulbajaj nakulbajaj changed the title Abstract API: add server HTTP version request option Abstract API: add server transport capability request option May 14, 2026
@nakulbajaj nakulbajaj marked this pull request as ready for review May 14, 2026 15:01
@nakulbajaj nakulbajaj added the 🆕 semver/minor Adds new public API. label May 14, 2026
case tcp

/// TCP with TLS transport.
case tcpWithTLS
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we don't need this case since whether the server supports TLS is already known to the user when they put http or https in the URL.

/// capabilities. New transports may be added in future releases, so client
/// code must handle unknown cases.
@nonexhaustive
public enum TransportVersion: Sendable, Hashable {
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's name it HTTPTransportProtocol since this is specific to HTTP

public var allowsExpensiveNetworkAccess: Bool = true
public var allowsConstrainedNetworkAccess: Bool = true
public var assumesHTTP3Capable: Bool = false
public var serverSupportedTransports: Set<TransportVersion> = []
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should default it to [.tcp], maybe call it a hint in the property name as well?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated to serverSupportedTransportsHint and the protocol name to just ServerTransportHint, I realized it doesn't need Capability in the name

@nakulbajaj nakulbajaj requested a review from guoye-zhang May 18, 2026 22:47
public struct RequestOptions: HTTPClientCapability.RequestOptions {

public struct RequestOptions: HTTPClientCapability.ServerTransportHint {
public var serverSupportedTransportsHint: Set<NetworkTypes.HTTPTransportVersion> = []
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should AHC conform or should we keep it to URLSession? Also let's make this .tcp by default as well

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

🆕 semver/minor Adds new public API.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants