From 8f08c49b9f65851a274eb611252c6a2e2022ffa4 Mon Sep 17 00:00:00 2001 From: Carlo Lucera Date: Thu, 8 Aug 2024 15:31:45 +0200 Subject: [PATCH 1/5] fix: fix headers not completing when call is terminated --- lib/src/client/call.dart | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/lib/src/client/call.dart b/lib/src/client/call.dart index 8d5918f8..12a27470 100644 --- a/lib/src/client/call.dart +++ b/lib/src/client/call.dart @@ -483,6 +483,14 @@ class ClientCall implements Response { if (_responseSubscription != null) { futures.add(_responseSubscription!.cancel()); } + if (!_headers.isCompleted) { + _headers.completeError( + GrpcError.unimplemented("Request terminated before headers")); + } + if (!_trailers.isCompleted) { + _trailers.completeError( + GrpcError.unimplemented("Request terminated before trailers")); + } await Future.wait(futures); } From ab731214d87f8d71d35f9bcc0b492d08fd658ae0 Mon Sep 17 00:00:00 2001 From: Carlo Lucera Date: Tue, 20 Aug 2024 15:53:41 +0200 Subject: [PATCH 2/5] fix: Add tests and Changelog text --- CHANGELOG.md | 4 ++ lib/src/client/call.dart | 4 +- pubspec.yaml | 2 +- test/client_tests/call_test.dart | 67 ++++++++++++++++++++++++++++++++ 4 files changed, 74 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 398f2c81..6e1faa37 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,7 @@ +## 4.0.1 + +* Fix header and trailing not completing if the call is terminated. Fixes [#727](https://github.com/grpc/grpc-dart/issues/727) + ## 4.0.0 * Set compressed flag correctly for grpc-encoding = identity. Fixes [#669](https://github.com/grpc/grpc-dart/issues/669) (https://github.com/grpc/grpc-dart/pull/693) diff --git a/lib/src/client/call.dart b/lib/src/client/call.dart index 12a27470..429348f5 100644 --- a/lib/src/client/call.dart +++ b/lib/src/client/call.dart @@ -485,11 +485,11 @@ class ClientCall implements Response { } if (!_headers.isCompleted) { _headers.completeError( - GrpcError.unimplemented("Request terminated before headers")); + GrpcError.unimplemented('Request terminated before headers')); } if (!_trailers.isCompleted) { _trailers.completeError( - GrpcError.unimplemented("Request terminated before trailers")); + GrpcError.unimplemented('Request terminated before trailers')); } await Future.wait(futures); } diff --git a/pubspec.yaml b/pubspec.yaml index de16e0fa..3ada1c62 100644 --- a/pubspec.yaml +++ b/pubspec.yaml @@ -1,6 +1,6 @@ name: grpc description: Dart implementation of gRPC, a high performance, open-source universal RPC framework. -version: 4.0.0 +version: 4.0.1 repository: https://github.com/grpc/grpc-dart diff --git a/test/client_tests/call_test.dart b/test/client_tests/call_test.dart index c519f6e8..e18dcf2b 100644 --- a/test/client_tests/call_test.dart +++ b/test/client_tests/call_test.dart @@ -13,10 +13,26 @@ // See the License for the specific language governing permissions and // limitations under the License. +import 'package:grpc/grpc.dart'; import 'package:grpc/src/client/call.dart'; import 'package:test/test.dart'; +import '../src/client_utils.dart'; + void main() { + const dummyValue = 0; + const cancelDurationMillis = 300; + + late ClientHarness harness; + + setUp(() { + harness = ClientHarness()..setUp(); + }); + + tearDown(() { + harness.tearDown(); + }); + test('WebCallOptions mergeWith CallOptions returns WebCallOptions', () { final options = WebCallOptions(bypassCorsPreflight: true, withCredentials: true); @@ -28,4 +44,55 @@ void main() { expect(mergedOptions.bypassCorsPreflight, true); expect(mergedOptions.withCredentials, true); }); + + test( + 'Terminating a call correctly complete headers and trailers futures', + () async { + final clientCall = harness.client.unary(dummyValue); + clientCall.catchError((error) { + expect(error.codeName, equals('CANCELLED')); + return dummyValue; + }); + + Future.delayed( + Duration(milliseconds: cancelDurationMillis), + ).then((_) { + clientCall.cancel(); + }); + + await expectLater( + clientCall.headers, + throwsA( + isA() + .having( + (e) => e.codeName, + 'Test codename', + contains('UNIMPLEMENTED'), + ) + .having( + (e) => e.message, + 'Test message', + contains('headers'), + ), + ), + ); + await expectLater( + clientCall.trailers, + throwsA( + isA() + .having( + (e) => e.codeName, + 'Test codename', + contains('UNIMPLEMENTED'), + ) + .having( + (e) => e.message, + 'Test message', + contains('trailers'), + ), + ), + ); + }, + timeout: Timeout(Duration(milliseconds: cancelDurationMillis * 2)), + ); } From f954ba00ccf112797c3fd6519e2cb24c5942b7a9 Mon Sep 17 00:00:00 2001 From: Carlo Lucera Date: Tue, 20 Aug 2024 16:28:46 +0200 Subject: [PATCH 3/5] fix: Replacing empty instead of a failure --- lib/src/client/call.dart | 6 +-- test/client_tests/call_test.dart | 68 ++++++++++++++++---------------- 2 files changed, 37 insertions(+), 37 deletions(-) diff --git a/lib/src/client/call.dart b/lib/src/client/call.dart index 429348f5..7010313a 100644 --- a/lib/src/client/call.dart +++ b/lib/src/client/call.dart @@ -484,12 +484,10 @@ class ClientCall implements Response { futures.add(_responseSubscription!.cancel()); } if (!_headers.isCompleted) { - _headers.completeError( - GrpcError.unimplemented('Request terminated before headers')); + _headers.complete({}); } if (!_trailers.isCompleted) { - _trailers.completeError( - GrpcError.unimplemented('Request terminated before trailers')); + _trailers.complete({}); } await Future.wait(futures); } diff --git a/test/client_tests/call_test.dart b/test/client_tests/call_test.dart index e18dcf2b..9ce832b5 100644 --- a/test/client_tests/call_test.dart +++ b/test/client_tests/call_test.dart @@ -46,53 +46,55 @@ void main() { }); test( - 'Terminating a call correctly complete headers and trailers futures', + 'Cancelling a call correctly complete headers future', () async { final clientCall = harness.client.unary(dummyValue); - clientCall.catchError((error) { - expect(error.codeName, equals('CANCELLED')); - return dummyValue; - }); Future.delayed( Duration(milliseconds: cancelDurationMillis), - ).then((_) { - clientCall.cancel(); - }); + ).then((_) => clientCall.cancel()); + + expect(await clientCall.headers, isEmpty); await expectLater( - clientCall.headers, + clientCall, throwsA( - isA() - .having( - (e) => e.codeName, - 'Test codename', - contains('UNIMPLEMENTED'), - ) - .having( - (e) => e.message, - 'Test message', - contains('headers'), - ), + isA().having( + (e) => e.codeName, + 'Test codename', + contains('CANCELLED'), + ), ), ); + }, + ); + + test( + 'Cancelling a call correctly complete trailers futures', + () async { + final clientCall = harness.client.unary(dummyValue); + + Future.delayed( + Duration(milliseconds: cancelDurationMillis), + ).then((_) { + clientCall.cancel(); + }); + + expect( + await clientCall.trailers, + isEmpty, + ); + await expectLater( - clientCall.trailers, + clientCall, throwsA( - isA() - .having( - (e) => e.codeName, - 'Test codename', - contains('UNIMPLEMENTED'), - ) - .having( - (e) => e.message, - 'Test message', - contains('trailers'), - ), + isA().having( + (e) => e.codeName, + 'Test codename', + contains('CANCELLED'), + ), ), ); }, - timeout: Timeout(Duration(milliseconds: cancelDurationMillis * 2)), ); } From d47e2468b2a4725c82c766f3561393c5507737b1 Mon Sep 17 00:00:00 2001 From: Carlo Lucera Date: Thu, 22 Aug 2024 12:24:01 +0200 Subject: [PATCH 4/5] fix: fix keepalive_test.dart --- test/keepalive_test.dart | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/keepalive_test.dart b/test/keepalive_test.dart index 7ffa1156..59fa7914 100644 --- a/test/keepalive_test.dart +++ b/test/keepalive_test.dart @@ -49,7 +49,7 @@ void main() { services: [FakeEchoService()], keepAliveOptions: serverOptions, ); - await server.serve(address: 'localhost', port: 8081); + await server.serve(address: 'localhost', port: 8082); fakeChannel = FakeClientChannel( 'localhost', port: server.port!, From 520350074a60f47e3f372abbf9871415e2bd2f08 Mon Sep 17 00:00:00 2001 From: Carlo Lucera Date: Wed, 28 Aug 2024 07:36:20 +0200 Subject: [PATCH 5/5] fix: Use 0 for keepalive_test.dart --- test/keepalive_test.dart | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/keepalive_test.dart b/test/keepalive_test.dart index 59fa7914..4e710811 100644 --- a/test/keepalive_test.dart +++ b/test/keepalive_test.dart @@ -49,7 +49,7 @@ void main() { services: [FakeEchoService()], keepAliveOptions: serverOptions, ); - await server.serve(address: 'localhost', port: 8082); + await server.serve(address: 'localhost', port: 0); fakeChannel = FakeClientChannel( 'localhost', port: server.port!,