From a157ddbf94ee8905197da14f90aac6e1c17713fd Mon Sep 17 00:00:00 2001 From: Ben Konyi Date: Wed, 18 Nov 2020 23:45:41 +0000 Subject: [PATCH] [ DDS ] Add errorCode to DartDevelopmentServiceException to better communicate reason for error Change-Id: I5041558ad6370f873dc23515fdad1ec8a2cf4ba8 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/172880 Reviewed-by: Jonah Williams Commit-Queue: Ben Konyi --- pkg/dds/CHANGELOG.md | 4 +++ pkg/dds/lib/dds.dart | 29 ++++++++++++++++++- pkg/dds/lib/src/dds_impl.dart | 9 ++---- pkg/dds/pubspec.yaml | 2 +- ..._connection_closed_before_full_header.dart | 1 + pkg/dds/test/smoke_test.dart | 2 ++ 6 files changed, 39 insertions(+), 8 deletions(-) diff --git a/pkg/dds/CHANGELOG.md b/pkg/dds/CHANGELOG.md index 54979eb3453..953ac0bd40a 100644 --- a/pkg/dds/CHANGELOG.md +++ b/pkg/dds/CHANGELOG.md @@ -1,3 +1,7 @@ +# 1.6.0 +- Added `errorCode` to `DartDevelopmentServiceException` to communicate the + underlying reason of the failure. + # 1.5.1 - Improve internal error handling for situations with less than graceful shutdowns. diff --git a/pkg/dds/lib/dds.dart b/pkg/dds/lib/dds.dart index 0f716c68959..29cc2881984 100644 --- a/pkg/dds/lib/dds.dart +++ b/pkg/dds/lib/dds.dart @@ -159,9 +159,36 @@ abstract class DartDevelopmentService { } class DartDevelopmentServiceException implements Exception { - DartDevelopmentServiceException._(this.message); + /// Set when `DartDeveloperService.startDartDevelopmentService` is called and + /// the target VM service already has a Dart Developer Service instance + /// connected. + static const int existingDdsInstanceError = 1; + + /// Set when the connection to the remote VM service terminates unexpectedly + /// during Dart Development Service startup. + static const int failedToStartError = 2; + + /// Set when a connection error has occurred after startup. + static const int connectionError = 3; + + factory DartDevelopmentServiceException._existingDdsInstanceError( + String message) { + return DartDevelopmentServiceException._(existingDdsInstanceError, message); + } + + factory DartDevelopmentServiceException._failedToStartError() { + return DartDevelopmentServiceException._( + failedToStartError, 'Failed to start Dart Development Service'); + } + + factory DartDevelopmentServiceException._connectionError(String message) { + return DartDevelopmentServiceException._(connectionError, message); + } + + DartDevelopmentServiceException._(this.errorCode, this.message); String toString() => 'DartDevelopmentServiceException: $message'; + final int errorCode; final String message; } diff --git a/pkg/dds/lib/src/dds_impl.dart b/pkg/dds/lib/src/dds_impl.dart index fbb6bbab77d..ad0e46fc58c 100644 --- a/pkg/dds/lib/src/dds_impl.dart +++ b/pkg/dds/lib/src/dds_impl.dart @@ -49,17 +49,14 @@ class _DartDevelopmentService implements DartDevelopmentService { shutdown(); if (!started && !completer.isCompleted) { completer.completeError( - DartDevelopmentServiceException._( - 'Failed to start Dart Development Service', - ), - ); + DartDevelopmentServiceException._failedToStartError()); } }, onError: (e, st) { shutdown(); if (!completer.isCompleted) { completer.completeError( - DartDevelopmentServiceException._(e.toString()), + DartDevelopmentServiceException._connectionError(e.toString()), st, ); } @@ -118,7 +115,7 @@ class _DartDevelopmentService implements DartDevelopmentService { } on json_rpc.RpcException catch (e) { await _server.close(force: true); // _yieldControlToDDS fails if DDS is not the only VM service client. - throw DartDevelopmentServiceException._( + throw DartDevelopmentServiceException._existingDdsInstanceError( e.data != null ? e.data['details'] : e.toString(), ); } diff --git a/pkg/dds/pubspec.yaml b/pkg/dds/pubspec.yaml index a04a63c5364..d4132a1cb0f 100644 --- a/pkg/dds/pubspec.yaml +++ b/pkg/dds/pubspec.yaml @@ -3,7 +3,7 @@ description: >- A library used to spawn the Dart Developer Service, used to communicate with a Dart VM Service instance. -version: 1.5.1 +version: 1.6.0 homepage: https://github.com/dart-lang/sdk/tree/master/pkg/dds diff --git a/pkg/dds/test/handles_connection_closed_before_full_header.dart b/pkg/dds/test/handles_connection_closed_before_full_header.dart index 65d643684fb..80f1744d7a5 100644 --- a/pkg/dds/test/handles_connection_closed_before_full_header.dart +++ b/pkg/dds/test/handles_connection_closed_before_full_header.dart @@ -30,6 +30,7 @@ void main() async { await DartDevelopmentService.startDartDevelopmentService(uri); fail('Unexpected successful connection.'); } on DartDevelopmentServiceException catch (e) { + expect(e.errorCode, DartDevelopmentServiceException.connectionError); expect(e.toString().contains('WebSocketChannelException'), true); } }); diff --git a/pkg/dds/test/smoke_test.dart b/pkg/dds/test/smoke_test.dart index 0a7347c274d..d3178936bc7 100644 --- a/pkg/dds/test/smoke_test.dart +++ b/pkg/dds/test/smoke_test.dart @@ -107,6 +107,8 @@ void main() { } on DartDevelopmentServiceException catch (e) { expect(e.message, 'Existing VM service clients prevent DDS from taking control.'); + expect(e.errorCode, + DartDevelopmentServiceException.existingDdsInstanceError); } }); });