Skip to content

Commit 071ebc5

Browse files
steffenhaakmosuem
andauthored
fix: keep alive timeout finishes transport instead of connection shutdown (grpc#722)
* fix: keep alive timeout finishes transport instead of shutting down channel * Update keepalive_test.dart * Update CHANGELOG.md --------- Co-authored-by: Moritz <[email protected]>
1 parent 8177633 commit 071ebc5

File tree

3 files changed

+20
-11
lines changed

3 files changed

+20
-11
lines changed

CHANGELOG.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,8 @@
22

33
* Internal optimization to client code.
44
* Small fixes, such as ports in testing and enabling `timeline_test.dart`.
5+
* When the keep alive manager runs into a timeout, it will finish the transport instead of closing
6+
the connection, as defined in the gRPC spec.
57

68
## 4.0.1
79

lib/src/client/http2_connection.dart

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -113,7 +113,7 @@ class Http2ClientConnection implements connection.ClientConnection {
113113
transport.ping();
114114
}
115115
},
116-
onPingTimeout: () => shutdown(),
116+
onPingTimeout: () => transport.finish(),
117117
);
118118
transport.onFrameReceived
119119
.listen((_) => keepAliveManager?.onFrameReceived());

test/keepalive_test.dart

Lines changed: 17 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@ void main() {
3232
late EchoServiceClient fakeClient;
3333
late FakeClientChannel fakeChannel;
3434
late EchoServiceClient unresponsiveClient;
35-
late ClientChannel unresponsiveChannel;
35+
late FakeClientChannel unresponsiveChannel;
3636

3737
final pingInterval = Duration(milliseconds: 10);
3838
final timeout = Duration(milliseconds: 30);
@@ -101,14 +101,18 @@ void main() {
101101
expect(fakeChannel.newConnectionCounter, 1);
102102
});
103103

104-
test('Server doesnt ack the ping, making the client shutdown the connection',
104+
test('Server doesnt ack the ping, making the client shutdown the transport',
105105
() async {
106+
//Send a first request, get a connection
106107
await unresponsiveClient.echo(EchoRequest());
108+
expect(unresponsiveChannel.newConnectionCounter, 1);
109+
110+
//Ping is not being acked on time
107111
await Future.delayed(timeout * 2);
108-
await expectLater(
109-
unresponsiveClient.echo(EchoRequest()),
110-
throwsA(isA<GrpcError>()),
111-
);
112+
113+
//A second request gets a new connection
114+
await unresponsiveClient.echo(EchoRequest());
115+
expect(unresponsiveChannel.newConnectionCounter, 2);
112116
});
113117
}
114118

@@ -146,7 +150,7 @@ class FakeHttp2ClientConnection extends Http2ClientConnection {
146150
}
147151

148152
/// A wrapper around a [FakeHttp2ClientConnection]
149-
class UnresponsiveClientChannel extends ClientChannel {
153+
class UnresponsiveClientChannel extends FakeClientChannel {
150154
UnresponsiveClientChannel(
151155
super.host, {
152156
super.port,
@@ -155,11 +159,14 @@ class UnresponsiveClientChannel extends ClientChannel {
155159
});
156160

157161
@override
158-
ClientConnection createConnection() =>
159-
UnresponsiveHttp2ClientConnection(host, port, options);
162+
ClientConnection createConnection() {
163+
fakeHttp2ClientConnection =
164+
UnresponsiveHttp2ClientConnection(host, port, options);
165+
return fakeHttp2ClientConnection!;
166+
}
160167
}
161168

162-
class UnresponsiveHttp2ClientConnection extends Http2ClientConnection {
169+
class UnresponsiveHttp2ClientConnection extends FakeHttp2ClientConnection {
163170
UnresponsiveHttp2ClientConnection(super.host, super.port, super.options);
164171

165172
@override

0 commit comments

Comments
 (0)