Skip to content

Conversation

@fabianfett
Copy link
Member

@fabianfett fabianfett commented Jun 24, 2021

This PR adds http/2 support to the HTTPBin test utility.

Note regarding semver:

This PR adds swift-nio-http2 as a dependency.

Modifications

  • Removed obscure initialization options for HTTPBin
    • channelPromise: EventLoopPromise<Channel>? = nil was only used once (in testUploadStreamingBackpressure).
    • connectionDelay: TimeAmount = .seconds(0) was never used
    • maxChannelAge: TimeAmount? = nil was never used
  • The initialization options ssl: Bool = false, compress: Bool = false, refusesConnections: Bool = false have moved into a Mode enum. The mode might be .http1_1(ssl: Bool, compress: Bool), .http2(compress: Bool), or .refuse
  • Added support for http/2 (not used in any test yet, however I verified the support with curl locally.)
  • channel pipeline modifications are nearly all synchronous.
  • the HTTPBin's request handler can be changed. This allows testing of more esoteric cases without having to create a new server every time. The default HTTPBin handler continues to use the HTTPBinHandler.

Result

  • Easier test utils that can be used for http/2

Follow ups

  • I also wanted to add a ServerQuiescingHelper to ensure that all connections are closed when the test ends. However we often seem to leak connections when we cancel tasks. This would make our CI very flaky. I hope to land this at a later point once all connection and connection pool improvements have landed (See [RFC] ConnectionPool rewrite and http/2 support #376).

@fabianfett fabianfett added the 🆕 semver/minor Adds new public API. label Jun 24, 2021
@fabianfett fabianfett added this to the 1.5.0 milestone Jun 24, 2021
@fabianfett fabianfett requested a review from weissi June 24, 2021 15:15
Copy link
Collaborator

@glbrntt glbrntt left a comment

Choose a reason for hiding this comment

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

Looks good aside from a typo.

private func syncAddHTTPProxyHandlers(
to channel: Channel,
connectionID: Int,
expectedAuthroization: String?
Copy link
Collaborator

Choose a reason for hiding this comment

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

typo: expectedAuthroization -> expectedAuthorization

Copy link
Member Author

Choose a reason for hiding this comment

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

Fixed!

@fabianfett fabianfett force-pushed the ff-prepare-test-utils-for-h2 branch from 2207098 to c4c7212 Compare June 24, 2021 18:54
@Davidde94
Copy link
Collaborator

@swift-server-bot test this please

@fabianfett fabianfett force-pushed the ff-prepare-test-utils-for-h2 branch from 4653af5 to 3bf080c Compare June 25, 2021 09:45
@fabianfett fabianfett merged commit af837ed into swift-server:main Jun 25, 2021
@fabianfett fabianfett deleted the ff-prepare-test-utils-for-h2 branch June 25, 2021 09:54
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