Repository navigation
Conversation
`ConnectionManager` gains an optional `clientSettings`, passed to `ClientTransportConnection.viaSocket` for every connection. `null` keeps `package:http2`'s defaults, so existing behavior is unchanged. `ClientSettings` is exported so callers need no direct `http2` dependency. Closes cfug#2606
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #2606.
ConnectionManagergains an optionalclientSettings, passed toClientTransportConnection.viaSocketfor every connection it creates.null(the default) keepspackage:http2's defaults, so existing behavior is unchanged.ClientSettingsis re-exported fromdio_http2_adapterso callers need no directhttp2dependency.Why one
ClientSettingsrather thanstreamWindowSize/concurrentStreamLimitints as sketched in #2606: the stream window alone does not lift the cap. Inpackage:http2up to 3.1.0 the connection receive window is fixed at 65535 bytes, so at most 64 KiB is in flight per round trip whateverstreamWindowSizeis. dart-lang/http#1999 (merged, inhttp23.2.0-wip, not yet on pub.dev) addsSettings.connectionWindowSizeandSettings.maxFrameSize. Taking the wholeClientSettingslets users ofhttp23.2 set those without another dio API change, and works today with^2.1.0(streamWindowSize,concurrentStreamLimit,allowServerPushes). No dependency constraint changes.For reference, with
http23.2's settings (streamWindowSize6 MiB,connectionWindowSize16 MiB,maxFrameSize1 MiB), a 4 MiB body from a Gonet/httpserver through 100 ms of added RTT took ~340 ms against ~350 ms for HTTP/1.1, and 7.1 s with the defaults (numbers from dart-lang/http#1997).Verification: two tests in
test/http2_test.dartread the client's first SETTINGS frame off the wire through a local h2c server: withclientSettingsit carriesSETTINGS_INITIAL_WINDOW_SIZE= 1 MiB (fails when the settings are not passed toviaSocket), without it the setting is absent as before.dart analyze --fatal-infosanddart formatare clean for the package;dart testinplugins/http2_adapterpasses 104 tests.Unverified: three existing ALPN tests that drive
openssl s_serverfail on my Windows machine, identically on unmodifiedmain(fails with HandshakeException when only h2 is advertised...,routes to fallbackAdapter when h2 and http/1.1 are both advertised...,...through a proxy); I rely on CI for those. I did not runmelos run testacross the other packages, which this change does not touch.AI disclosure: implementation, tests and this description were drafted with Claude (Claude Code) and reviewed by me.
New Pull Request Checklist
mainbranch to avoid conflicts (via merge from master or rebase)maintoo; see Unverified)CHANGELOG.mdin the corresponding package