fix: keep alive timeout finishes transport instead of connection shutdown

[steffenhaak]

Currently, when the keep alive manager runs into a timeout, the connection is shutdown. The channel, however, cannot recover and stays in a stuck state.

Instead, we should finish the transport, which eventually will change the connection to be in transient failure mode, from where it recovers, once a stable connection can be established again.

[linux-foundation-easycla]

The committers listed above are authorized under a signed CLA.

• ✅ login: steffenhaak (a94ed46, fb4f301)
• ✅ login: mosuem / name: Moritz (97a81b6)

[mosuem]

Thanks for the PR! Sounds good at first glance, but I will check the specs and take a look at this PR next week.

[github-actions]

PR Health

Breaking changes ✔️

Package	Change	Current Version	New Version	Needed Version	Looking good?
grpc	None	4.0.2-wip	4.0.2-wip	4.0.2-wip	✔️

Changelog Entry ✔️

Package	Changed Files

Changes to files need to be accounted for in their respective changelogs.

Coverage ✔️

File	Coverage
lib/src/client/http2_connection.dart	💚 79 %

This check for test coverage is informational (issues shown here will not fail the PR).

API leaks ⚠️

The following packages contain symbols visible in the public API, but not exported by the library. Export these symbols or remove them from your publicly visible API.

Package	Leaked API symbols
grpc	$1.Duration ServerHandler Any

This check can be disabled by tagging the PR with skip-leaking-check.

Package publish validation ✔️

Package	Version	Status
package:grpc	4.0.2-wip	WIP (no publish necessary)

Documentation at https://github.com/dart-lang/ecosystem/wiki/Publishing-automation.

[steffenhaak]

Quote from the specs:

If a keepalive ping is not blocked and is sent on the transport, then the keepalive watchdog timer is started which will close the transport if the ping is not acknowledged before it fires.

Closing the transport should be the appropriate thing to do. But take your time and think about it thoroughly.

I'm using a modified version for now, so there's absolutely no pressure on my side. Please let me know if I can assist you in any way with this.

[mosuem]

LGTM in general, I filed a PR on your fork to address the failing test. Let me know what you think. Probably has to be slightly rebased after #732 landing.

[mosuem]

@steffenhaak Also sorry for the delay!

[steffenhaak]

@mosuem Great, thank you! I will take a look at it, most likely by the end of this week, and do the rebasing...

[steffenhaak]

@mosuem Your changed test looks good to me. I rebased the PR on the current main branch. So I guess this PR is ready to be merged :)

[mosuem]

@mosuem Your changed test looks good to me. I rebased the PR on the current main branch. So I guess this PR is ready to be merged :)

It seems to need a changelog entry, but then yes!

[steffenhaak]

@mosuem done :)