From e3f9f0aca752b8a171d157585f22978f2337506f Mon Sep 17 00:00:00 2001 From: Elliott Brooks <21270878+elliette@users.noreply.github.com> Date: Mon, 12 Jun 2023 16:15:52 -0700 Subject: [PATCH 01/10] Use new error codes from package:vm_service --- dwds/lib/src/debugging/debugger.dart | 6 +++--- dwds/lib/src/debugging/inspector.dart | 4 ++-- dwds/lib/src/dwds_vm_client.dart | 8 ++++---- .../src/services/chrome_proxy_service.dart | 19 ++++++++++--------- dwds/lib/src/utilities/domain.dart | 2 +- dwds/pubspec.yaml | 2 +- dwds/test/chrome_proxy_service_test.dart | 3 ++- 7 files changed, 23 insertions(+), 21 deletions(-) diff --git a/dwds/lib/src/debugging/debugger.dart b/dwds/lib/src/debugging/debugger.dart index d85bb49ad..12edab2d4 100644 --- a/dwds/lib/src/debugging/debugger.dart +++ b/dwds/lib/src/debugging/debugger.dart @@ -158,7 +158,7 @@ class Debugger extends Domain { if (stackComputer == null) { throw RPCError( 'getStack', - RPCError.kInternalError, + RPCErrorKind.kInternalError.code, 'Cannot compute stack when application is not paused', ); } @@ -323,7 +323,7 @@ class Debugger extends Domain { if (jsId == null) { throw RPCError( 'removeBreakpoint', - RPCError.kInternalError, + RPCErrorKind.kInternalError.code, 'invalid JS breakpoint id $jsId', ); } @@ -715,7 +715,7 @@ Future sendCommandAndValidateResult( if (result == null) { throw RPCError( method, - RPCError.kInternalError, + RPCErrorKind.kInternalError.code, '$resultField not found in result from sendCommand', params, ); diff --git a/dwds/lib/src/debugging/inspector.dart b/dwds/lib/src/debugging/inspector.dart index 7d0c75027..691202eec 100644 --- a/dwds/lib/src/debugging/inspector.dart +++ b/dwds/lib/src/debugging/inspector.dart @@ -448,7 +448,7 @@ class AppInspector implements AppInspectorInterface { if (result == null) { throw RPCError( 'getMemoryUsage', - RPCError.kInternalError, + RPCErrorKind.kInternalError.code, 'Null result from chrome Devtools.', ); } @@ -461,7 +461,7 @@ class AppInspector implements AppInspectorInterface { if (usage == null) { throw RPCError( 'getMemoryUsage', - RPCError.kInternalError, + RPCErrorKind.kInternalError.code, 'Failed to parse memory usage result.', ); } diff --git a/dwds/lib/src/dwds_vm_client.dart b/dwds/lib/src/dwds_vm_client.dart index ac23dc570..fe9a574bd 100644 --- a/dwds/lib/src/dwds_vm_client.dart +++ b/dwds/lib/src/dwds_vm_client.dart @@ -142,7 +142,7 @@ class DwdsVmClient { if (ddsUri == null) { return RPCError( request['method'] as String, - RPCError.kInvalidParams, + RPCErrorKind.kInvalidParams.code, "'Missing parameter: 'uri'", ).toMap(); } @@ -229,7 +229,7 @@ Future> _hotRestart( // triggered in the middle of a full reload. return { 'error': { - 'code': RPCError.kInternalError, + 'code': RPCErrorKind.kInternalError.code, 'message': e.message, } }; @@ -249,7 +249,7 @@ Future> _hotRestart( final message = exception.error?['message']; // This corresponds to `Execution context was destroyed` which can // occur during a hot restart that must fall back to a full reload. - if (code != RPCError.kServerError) { + if (code != RPCErrorKind.kServerError.code) { return { 'error': { 'code': code, @@ -262,7 +262,7 @@ Future> _hotRestart( // Exceptions thrown by the injected client during hot restart. return { 'error': { - 'code': RPCError.kInternalError, + 'code': RPCErrorKind.kInternalError.code, 'message': '$exception', } }; diff --git a/dwds/lib/src/services/chrome_proxy_service.dart b/dwds/lib/src/services/chrome_proxy_service.dart index d8ea7a776..2d57576b7 100644 --- a/dwds/lib/src/services/chrome_proxy_service.dart +++ b/dwds/lib/src/services/chrome_proxy_service.dart @@ -563,7 +563,7 @@ ${globalLoadStrategy.loadModuleSnippet}("dart_sdk").developer.invokeExtension( } throw RPCError( 'evaluateInFrame', - RPCError.kInvalidRequest, + RPCErrorKind.kInvalidRequest.code, 'Expression evaluation is not supported for this configuration.', ); }, @@ -602,7 +602,7 @@ ${globalLoadStrategy.loadModuleSnippet}("dart_sdk").developer.invokeExtension( } throw RPCError( 'evaluateInFrame', - RPCError.kInvalidRequest, + RPCErrorKind.kInvalidRequest.code, 'Expression evaluation is not supported for this configuration.', ); }, @@ -811,7 +811,7 @@ ${globalLoadStrategy.loadModuleSnippet}("dart_sdk").developer.invokeExtension( default: throw RPCError( 'streamListen', - RPCError.kInvalidParams, + RPCErrorKind.kInvalidParams.code, 'The stream `$streamId` is not supported on web devices', ); } @@ -861,7 +861,7 @@ ${globalLoadStrategy.loadModuleSnippet}("dart_sdk").developer.invokeExtension( return Future.error( RPCError( 'reloadSources', - RPCError.kMethodNotFound, + RPCErrorKind.kMethodNotFound.code, 'Hot reload not supported on web devices', ), ); @@ -905,10 +905,11 @@ ${globalLoadStrategy.loadModuleSnippet}("dart_sdk").developer.invokeExtension( final errorMessage = e.message; if (errorMessage != null && errorMessage.contains('Can only perform operation while paused')) { - // TODO(https://github.com/dart-lang/sdk/issues/52636): Use error code - // from package:vm_service. - const kIsolateMustBePausedCode = 106; - throw RPCError('resume', kIsolateMustBePausedCode, errorMessage); + throw RPCError( + 'resume', + RPCErrorKind.kIsolateMustBePaused.code, + errorMessage, + ); } rethrow; } @@ -1267,7 +1268,7 @@ ${globalLoadStrategy.loadModuleSnippet}("dart_sdk").developer.invokeExtension( static RPCError _rpcNotSupported(String method) { return RPCError( method, - RPCError.kMethodNotFound, + RPCErrorKind.kMethodNotFound.code, '$method: Not supported on web devices', ); } diff --git a/dwds/lib/src/utilities/domain.dart b/dwds/lib/src/utilities/domain.dart index 699bd586b..bfb19204e 100644 --- a/dwds/lib/src/utilities/domain.dart +++ b/dwds/lib/src/utilities/domain.dart @@ -136,5 +136,5 @@ abstract class Domain { } Never throwInvalidParam(String method, String message) { - throw RPCError(method, RPCError.kInvalidParams, message); + throw RPCError(method, RPCErrorKind.kInvalidParams.code, message); } diff --git a/dwds/pubspec.yaml b/dwds/pubspec.yaml index e0fd22582..48f5da13f 100644 --- a/dwds/pubspec.yaml +++ b/dwds/pubspec.yaml @@ -33,7 +33,7 @@ dependencies: stack_trace: ^1.10.0 sse: ^4.1.2 uuid: ^3.0.6 - vm_service: ">=10.1.2 <12.0.0" + vm_service: ^11.7.1 web_socket_channel: ^2.2.0 webkit_inspection_protocol: ^1.0.1 diff --git a/dwds/test/chrome_proxy_service_test.dart b/dwds/test/chrome_proxy_service_test.dart index 8a93ac2e2..988a5977e 100644 --- a/dwds/test/chrome_proxy_service_test.dart +++ b/dwds/test/chrome_proxy_service_test.dart @@ -2382,7 +2382,8 @@ void main() { () => service.streamListen('aCustomStreamId'), throwsA( predicate( - (e) => (e is RPCError) && e.code == RPCError.kInvalidParams, + (e) => + (e is RPCError) && e.code == RPCErrorKind.kInvalidParams.code, ), ), ); From b0f53220835d33ee0ad82cc485bf01f3d433544b Mon Sep 17 00:00:00 2001 From: Elliott Brooks <21270878+elliette@users.noreply.github.com> Date: Tue, 13 Jun 2023 13:07:44 -0700 Subject: [PATCH 02/10] Wrap vm service API methods in error handlers --- .../src/services/chrome_proxy_service.dart | 278 +++++++++++++++++- 1 file changed, 264 insertions(+), 14 deletions(-) diff --git a/dwds/lib/src/services/chrome_proxy_service.dart b/dwds/lib/src/services/chrome_proxy_service.dart index 2d57576b7..43481e136 100644 --- a/dwds/lib/src/services/chrome_proxy_service.dart +++ b/dwds/lib/src/services/chrome_proxy_service.dart @@ -382,6 +382,18 @@ class ChromeProxyService implements VmServiceInterface { String scriptId, int line, { int? column, + }) async { + return _wrapInErrorHandler( + 'addBreakpoint', + () => _addBreakpoint(isolateId, scriptId, line), + ); + } + + Future _addBreakpoint( + String isolateId, + String scriptId, + int line, { + int? column, }) async { await isInitialized; _checkIsolate('addBreakpoint', isolateId); @@ -399,6 +411,22 @@ class ChromeProxyService implements VmServiceInterface { String scriptUri, int line, { int? column, + }) => + _wrapInErrorHandler( + 'addBreakpointWithScriptUri', + () => _addBreakpointWithScriptUri( + isolateId, + scriptUri, + line, + column: column, + ), + ); + + Future _addBreakpointWithScriptUri( + String isolateId, + String scriptUri, + int line, { + int? column, }) async { await isInitialized; _checkIsolate('addBreakpointWithScriptUri', isolateId); @@ -431,6 +459,20 @@ class ChromeProxyService implements VmServiceInterface { String method, { String? isolateId, Map? args, + }) => + _wrapInErrorHandler( + 'callServiceExtension', + () => _callServiceExtension( + method, + isolateId: isolateId, + args: args, + ), + ); + + Future _callServiceExtension( + String method, { + String? isolateId, + Map? args, }) async { await isInitialized; isolateId ??= _inspector?.isolate.id; @@ -539,6 +581,24 @@ ${globalLoadStrategy.loadModuleSnippet}("dart_sdk").developer.invokeExtension( String expression, { Map? scope, bool? disableBreakpoints, + }) => + _wrapInErrorHandler( + 'evaluate', + () => _evaluate( + isolateId, + targetId, + expression, + scope: scope, + disableBreakpoints: disableBreakpoints, + ), + ); + + Future _evaluate( + String isolateId, + String targetId, + String expression, { + Map? scope, + bool? disableBreakpoints, }) { // TODO(798) - respect disableBreakpoints. return captureElapsedTime( @@ -578,6 +638,24 @@ ${globalLoadStrategy.loadModuleSnippet}("dart_sdk").developer.invokeExtension( String expression, { Map? scope, bool? disableBreakpoints, + }) => + _wrapInErrorHandler( + 'evaluateInFrame', + () => _evaluateInFrame( + isolateId, + frameIndex, + expression, + scope: scope, + disableBreakpoints: disableBreakpoints, + ), + ); + + Future _evaluateInFrame( + String isolateId, + int frameIndex, + String expression, { + Map? scope, + bool? disableBreakpoints, }) { // TODO(798) - respect disableBreakpoints. @@ -643,7 +721,12 @@ ${globalLoadStrategy.loadModuleSnippet}("dart_sdk").developer.invokeExtension( } @override - Future getIsolate(String isolateId) { + Future getIsolate(String isolateId) => _wrapInErrorHandler( + 'getIsolate', + () => _getIsolate(isolateId), + ); + + Future _getIsolate(String isolateId) { return captureElapsedTime( () async { await isInitialized; @@ -655,7 +738,10 @@ ${globalLoadStrategy.loadModuleSnippet}("dart_sdk").developer.invokeExtension( } @override - Future getMemoryUsage(String isolateId) async { + Future getMemoryUsage(String isolateId) => + _wrapInErrorHandler('getMemoryUsage', () => _getMemoryUsage(isolateId)); + + Future _getMemoryUsage(String isolateId) async { await isInitialized; _checkIsolate('getMemoryUsage', isolateId); return inspector.getMemoryUsage(); @@ -667,6 +753,22 @@ ${globalLoadStrategy.loadModuleSnippet}("dart_sdk").developer.invokeExtension( String objectId, { int? offset, int? count, + }) => + _wrapInErrorHandler( + 'getObject', + () => _getObject( + isolateId, + objectId, + offset: offset, + count: count, + ), + ); + + Future _getObject( + String isolateId, + String objectId, { + int? offset, + int? count, }) async { await isInitialized; _checkIsolate('getObject', isolateId); @@ -674,7 +776,12 @@ ${globalLoadStrategy.loadModuleSnippet}("dart_sdk").developer.invokeExtension( } @override - Future getScripts(String isolateId) { + Future getScripts(String isolateId) => _wrapInErrorHandler( + 'getScripts', + () => _getScripts(isolateId), + ); + + Future _getScripts(String isolateId) { return captureElapsedTime( () async { await isInitialized; @@ -695,6 +802,30 @@ ${globalLoadStrategy.loadModuleSnippet}("dart_sdk").developer.invokeExtension( bool? forceCompile, bool? reportLines, List? libraryFilters, + }) => + _wrapInErrorHandler( + 'getSourceReport', + () => _getSourceReport( + isolateId, + reports, + scriptId: scriptId, + tokenPos: tokenPos, + endTokenPos: endTokenPos, + forceCompile: forceCompile, + reportLines: reportLines, + libraryFilters: libraryFilters, + ), + ); + + Future _getSourceReport( + String isolateId, + List reports, { + String? scriptId, + int? tokenPos, + int? endTokenPos, + bool? forceCompile, + bool? reportLines, + List? libraryFilters, }) { return captureElapsedTime( () async { @@ -720,7 +851,12 @@ ${globalLoadStrategy.loadModuleSnippet}("dart_sdk").developer.invokeExtension( /// /// The returned stack will contain up to [limit] frames if provided. @override - Future getStack(String isolateId, {int? limit}) async { + Future getStack(String isolateId, {int? limit}) => _wrapInErrorHandler( + 'getStack', + () => _getStack(isolateId, limit: limit), + ); + + Future _getStack(String isolateId, {int? limit}) async { await isInitialized; await isStarted; _checkIsolate('getStack', isolateId); @@ -728,7 +864,9 @@ ${globalLoadStrategy.loadModuleSnippet}("dart_sdk").developer.invokeExtension( } @override - Future getVM() { + Future getVM() => _wrapInErrorHandler('getVM', _getVM); + + Future _getVM() { return captureElapsedTime( () async { await isInitialized; @@ -752,7 +890,10 @@ ${globalLoadStrategy.loadModuleSnippet}("dart_sdk").developer.invokeExtension( } @override - Future getVersion() async { + Future getVersion() => + _wrapInErrorHandler('getVersion', _getVersion); + + Future _getVersion() async { final version = semver.Version.parse(vmServiceVersion); return Version(major: version.major, minor: version.minor); } @@ -764,6 +905,24 @@ ${globalLoadStrategy.loadModuleSnippet}("dart_sdk").developer.invokeExtension( String selector, List argumentIds, { bool? disableBreakpoints, + }) => + _wrapInErrorHandler( + 'invoke', + () => _invoke( + isolateId, + targetId, + selector, + argumentIds, + disableBreakpoints: disableBreakpoints, + ), + ); + + Future _invoke( + String isolateId, + String targetId, + String selector, + List argumentIds, { + bool? disableBreakpoints, }) async { await isInitialized; _checkIsolate('invoke', isolateId); @@ -819,7 +978,10 @@ ${globalLoadStrategy.loadModuleSnippet}("dart_sdk").developer.invokeExtension( } @override - Future pause(String isolateId) async { + Future pause(String isolateId) => + _wrapInErrorHandler('pause', () => _pause(isolateId)); + + Future _pause(String isolateId) async { await isInitialized; _checkIsolate('pause', isolateId); return (await debuggerFuture).pause(); @@ -832,6 +994,16 @@ ${globalLoadStrategy.loadModuleSnippet}("dart_sdk").developer.invokeExtension( String isolateId, List uris, { bool? local, + }) => + _wrapInErrorHandler( + 'lookupResolvedPackageUris', + () => _lookupResolvedPackageUris(isolateId, uris, local: local), + ); + + Future _lookupResolvedPackageUris( + String isolateId, + List uris, { + bool? local, }) async { await isInitialized; _checkIsolate('lookupResolvedPackageUris', isolateId); @@ -839,7 +1011,16 @@ ${globalLoadStrategy.loadModuleSnippet}("dart_sdk").developer.invokeExtension( } @override - Future lookupPackageUris(String isolateId, List uris) async { + Future lookupPackageUris(String isolateId, List uris) => + _wrapInErrorHandler( + 'lookupPackageUris', + () => _lookupPackageUris(isolateId, uris), + ); + + Future _lookupPackageUris( + String isolateId, + List uris, + ) async { await isInitialized; _checkIsolate('lookupPackageUris', isolateId); return UriList(uris: uris.map(DartUri.toPackageUri).toList()); @@ -850,8 +1031,7 @@ ${globalLoadStrategy.loadModuleSnippet}("dart_sdk").developer.invokeExtension( return _rpcNotSupportedFuture('registerService'); } - @override - Future reloadSources( + Future _reloadSources( String isolateId, { bool? force, bool? pause, @@ -871,6 +1051,15 @@ ${globalLoadStrategy.loadModuleSnippet}("dart_sdk").developer.invokeExtension( Future removeBreakpoint( String isolateId, String breakpointId, + ) => + _wrapInErrorHandler( + 'removeBreakpoint', + () => _removeBreakpoint(isolateId, breakpointId), + ); + + Future _removeBreakpoint( + String isolateId, + String breakpointId, ) async { await isInitialized; _checkIsolate('removeBreakpoint', isolateId); @@ -884,6 +1073,20 @@ ${globalLoadStrategy.loadModuleSnippet}("dart_sdk").developer.invokeExtension( String isolateId, { String? step, int? frameIndex, + }) => + _wrapInErrorHandler( + 'resume', + () => _resume( + isolateId, + step: step, + frameIndex: frameIndex, + ), + ); + + Future _resume( + String isolateId, { + String? step, + int? frameIndex, }) async { try { if (inspector.appConnection.isStarted) { @@ -933,6 +1136,20 @@ ${globalLoadStrategy.loadModuleSnippet}("dart_sdk").developer.invokeExtension( String isolateId, { String? exceptionPauseMode, bool? shouldPauseOnExit, + }) => + _wrapInErrorHandler( + 'setIsolatePauseMode', + () => _setIsolatePauseMode( + isolateId, + exceptionPauseMode: exceptionPauseMode, + shouldPauseOnExit: shouldPauseOnExit, + ), + ); + + Future _setIsolatePauseMode( + String isolateId, { + String? exceptionPauseMode, + bool? shouldPauseOnExit, }) async { // TODO(elliette): Is there a way to respect the shouldPauseOnExit parameter // in Chrome? @@ -957,7 +1174,12 @@ ${globalLoadStrategy.loadModuleSnippet}("dart_sdk").developer.invokeExtension( } @override - Future setName(String isolateId, String name) async { + Future setName(String isolateId, String name) => _wrapInErrorHandler( + 'setName', + () => _setName(isolateId, name), + ); + + Future _setName(String isolateId, String name) async { await isInitialized; _checkIsolate('setName', isolateId); inspector.isolate.name = name; @@ -965,7 +1187,10 @@ ${globalLoadStrategy.loadModuleSnippet}("dart_sdk").developer.invokeExtension( } @override - Future setVMName(String name) async { + Future setVMName(String name) => + _wrapInErrorHandler('setVMName', () => _setVMName(name)); + + Future _setVMName(String name) async { _vm.name = name; _streamNotify( 'VM', @@ -992,7 +1217,10 @@ ${globalLoadStrategy.loadModuleSnippet}("dart_sdk").developer.invokeExtension( } @override - Future streamListen(String streamId) async { + Future streamListen(String streamId) => + _wrapInErrorHandler('streamListen', () => _streamListen(streamId)); + + Future _streamListen(String streamId) async { // TODO: This should return an error if the stream is already being listened // to. onEvent(streamId); @@ -1247,7 +1475,12 @@ ${globalLoadStrategy.loadModuleSnippet}("dart_sdk").developer.invokeExtension( } @override - Future getSupportedProtocols() async { + Future getSupportedProtocols() => _wrapInErrorHandler( + 'getSupportedProtocols', + _getSupportedProtocols, + ); + + Future _getSupportedProtocols() async { final version = semver.Version.parse(vmServiceVersion); return ProtocolList( protocols: [ @@ -1277,6 +1510,23 @@ ${globalLoadStrategy.loadModuleSnippet}("dart_sdk").developer.invokeExtension( return Future.error(_rpcNotSupported(method)); } + Future _wrapInErrorHandler( + String command, + Future Function() asyncCallback, + ) { + try { + return asyncCallback(); + } on RPCError catch (_) { + rethrow; + } catch (e) { + throw RPCError( + command, + RPCErrorKind.kInternalError.code, + 'Unexpected DWDS error for $command: $e', + ); + } + } + @override Future getProcessMemoryUsage() => _rpcNotSupportedFuture('getProcessMemoryUsage'); From 35cc357e5f27f7967ecf45a8b587a63bea81fdd9 Mon Sep 17 00:00:00 2001 From: Elliott Brooks <21270878+elliette@users.noreply.github.com> Date: Tue, 13 Jun 2023 13:12:10 -0700 Subject: [PATCH 03/10] Update CHANGELOG --- dwds/CHANGELOG.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/dwds/CHANGELOG.md b/dwds/CHANGELOG.md index e47b9554d..44728316a 100644 --- a/dwds/CHANGELOG.md +++ b/dwds/CHANGELOG.md @@ -6,7 +6,8 @@ - Display type objects concisely. - [#2103](https://github.com/dart-lang/webdev/pull/2103) - Support using scope in `ChromeProxyService.evaluateInFrame`. - [#2122](https://github.com/dart-lang/webdev/pull/2122) - Check for new events more often in batched stream. - [#2123](https://github.com/dart-lang/webdev/pull/2123) -- Map Chrome error on `resume` to the same error returned by the Dart VM. - [#2133](https://github.com/dart-lang/webdev/issues/2133) +- Map Chrome error on `resume` to the same error returned by the Dart VM. - [#2133](https://github.com/dart-lang/webdev/issues/2133) +- VM service API methods throw type `RPCError`, same as the Dart VM. - [#2144](https://github.com/dart-lang/webdev/pull/2144) ## 19.0.0 From 88dd43e9ac8337baaae6e6b4f078bfd284632f6d Mon Sep 17 00:00:00 2001 From: Elliott Brooks <21270878+elliette@users.noreply.github.com> Date: Tue, 13 Jun 2023 13:20:23 -0700 Subject: [PATCH 04/10] Fix analyzer errors --- dwds/lib/src/services/chrome_proxy_service.dart | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/dwds/lib/src/services/chrome_proxy_service.dart b/dwds/lib/src/services/chrome_proxy_service.dart index 43481e136..dd3926c70 100644 --- a/dwds/lib/src/services/chrome_proxy_service.dart +++ b/dwds/lib/src/services/chrome_proxy_service.dart @@ -1031,7 +1031,8 @@ ${globalLoadStrategy.loadModuleSnippet}("dart_sdk").developer.invokeExtension( return _rpcNotSupportedFuture('registerService'); } - Future _reloadSources( + @override + Future reloadSources( String isolateId, { bool? force, bool? pause, From 78229ea1eb5cc9026917e84c7bd868d1167d112b Mon Sep 17 00:00:00 2001 From: Elliott Brooks <21270878+elliette@users.noreply.github.com> Date: Wed, 14 Jun 2023 10:53:02 -0700 Subject: [PATCH 05/10] Update utility and add test --- .../src/services/chrome_proxy_service.dart | 68 ++++++++----------- dwds/lib/src/utilities/shared.dart | 17 +++++ dwds/test/utilities_test.dart | 62 +++++++++++++++++ 3 files changed, 106 insertions(+), 41 deletions(-) create mode 100644 dwds/test/utilities_test.dart diff --git a/dwds/lib/src/services/chrome_proxy_service.dart b/dwds/lib/src/services/chrome_proxy_service.dart index dd3926c70..53963f8d4 100644 --- a/dwds/lib/src/services/chrome_proxy_service.dart +++ b/dwds/lib/src/services/chrome_proxy_service.dart @@ -383,7 +383,7 @@ class ChromeProxyService implements VmServiceInterface { int line, { int? column, }) async { - return _wrapInErrorHandler( + return wrapInErrorHandlerAsync( 'addBreakpoint', () => _addBreakpoint(isolateId, scriptId, line), ); @@ -412,7 +412,7 @@ class ChromeProxyService implements VmServiceInterface { int line, { int? column, }) => - _wrapInErrorHandler( + wrapInErrorHandlerAsync( 'addBreakpointWithScriptUri', () => _addBreakpointWithScriptUri( isolateId, @@ -460,7 +460,7 @@ class ChromeProxyService implements VmServiceInterface { String? isolateId, Map? args, }) => - _wrapInErrorHandler( + wrapInErrorHandlerAsync( 'callServiceExtension', () => _callServiceExtension( method, @@ -582,7 +582,7 @@ ${globalLoadStrategy.loadModuleSnippet}("dart_sdk").developer.invokeExtension( Map? scope, bool? disableBreakpoints, }) => - _wrapInErrorHandler( + wrapInErrorHandlerAsync( 'evaluate', () => _evaluate( isolateId, @@ -639,7 +639,7 @@ ${globalLoadStrategy.loadModuleSnippet}("dart_sdk").developer.invokeExtension( Map? scope, bool? disableBreakpoints, }) => - _wrapInErrorHandler( + wrapInErrorHandlerAsync( 'evaluateInFrame', () => _evaluateInFrame( isolateId, @@ -721,7 +721,7 @@ ${globalLoadStrategy.loadModuleSnippet}("dart_sdk").developer.invokeExtension( } @override - Future getIsolate(String isolateId) => _wrapInErrorHandler( + Future getIsolate(String isolateId) => wrapInErrorHandlerAsync( 'getIsolate', () => _getIsolate(isolateId), ); @@ -739,7 +739,8 @@ ${globalLoadStrategy.loadModuleSnippet}("dart_sdk").developer.invokeExtension( @override Future getMemoryUsage(String isolateId) => - _wrapInErrorHandler('getMemoryUsage', () => _getMemoryUsage(isolateId)); + wrapInErrorHandlerAsync( + 'getMemoryUsage', () => _getMemoryUsage(isolateId)); Future _getMemoryUsage(String isolateId) async { await isInitialized; @@ -754,7 +755,7 @@ ${globalLoadStrategy.loadModuleSnippet}("dart_sdk").developer.invokeExtension( int? offset, int? count, }) => - _wrapInErrorHandler( + wrapInErrorHandlerAsync( 'getObject', () => _getObject( isolateId, @@ -776,7 +777,7 @@ ${globalLoadStrategy.loadModuleSnippet}("dart_sdk").developer.invokeExtension( } @override - Future getScripts(String isolateId) => _wrapInErrorHandler( + Future getScripts(String isolateId) => wrapInErrorHandlerAsync( 'getScripts', () => _getScripts(isolateId), ); @@ -803,7 +804,7 @@ ${globalLoadStrategy.loadModuleSnippet}("dart_sdk").developer.invokeExtension( bool? reportLines, List? libraryFilters, }) => - _wrapInErrorHandler( + wrapInErrorHandlerAsync( 'getSourceReport', () => _getSourceReport( isolateId, @@ -851,7 +852,8 @@ ${globalLoadStrategy.loadModuleSnippet}("dart_sdk").developer.invokeExtension( /// /// The returned stack will contain up to [limit] frames if provided. @override - Future getStack(String isolateId, {int? limit}) => _wrapInErrorHandler( + Future getStack(String isolateId, {int? limit}) => + wrapInErrorHandlerAsync( 'getStack', () => _getStack(isolateId, limit: limit), ); @@ -864,7 +866,7 @@ ${globalLoadStrategy.loadModuleSnippet}("dart_sdk").developer.invokeExtension( } @override - Future getVM() => _wrapInErrorHandler('getVM', _getVM); + Future getVM() => wrapInErrorHandlerAsync('getVM', _getVM); Future _getVM() { return captureElapsedTime( @@ -891,7 +893,7 @@ ${globalLoadStrategy.loadModuleSnippet}("dart_sdk").developer.invokeExtension( @override Future getVersion() => - _wrapInErrorHandler('getVersion', _getVersion); + wrapInErrorHandlerAsync('getVersion', _getVersion); Future _getVersion() async { final version = semver.Version.parse(vmServiceVersion); @@ -906,7 +908,7 @@ ${globalLoadStrategy.loadModuleSnippet}("dart_sdk").developer.invokeExtension( List argumentIds, { bool? disableBreakpoints, }) => - _wrapInErrorHandler( + wrapInErrorHandlerAsync( 'invoke', () => _invoke( isolateId, @@ -979,7 +981,7 @@ ${globalLoadStrategy.loadModuleSnippet}("dart_sdk").developer.invokeExtension( @override Future pause(String isolateId) => - _wrapInErrorHandler('pause', () => _pause(isolateId)); + wrapInErrorHandlerAsync('pause', () => _pause(isolateId)); Future _pause(String isolateId) async { await isInitialized; @@ -995,7 +997,7 @@ ${globalLoadStrategy.loadModuleSnippet}("dart_sdk").developer.invokeExtension( List uris, { bool? local, }) => - _wrapInErrorHandler( + wrapInErrorHandlerAsync( 'lookupResolvedPackageUris', () => _lookupResolvedPackageUris(isolateId, uris, local: local), ); @@ -1012,7 +1014,7 @@ ${globalLoadStrategy.loadModuleSnippet}("dart_sdk").developer.invokeExtension( @override Future lookupPackageUris(String isolateId, List uris) => - _wrapInErrorHandler( + wrapInErrorHandlerAsync( 'lookupPackageUris', () => _lookupPackageUris(isolateId, uris), ); @@ -1053,7 +1055,7 @@ ${globalLoadStrategy.loadModuleSnippet}("dart_sdk").developer.invokeExtension( String isolateId, String breakpointId, ) => - _wrapInErrorHandler( + wrapInErrorHandlerAsync( 'removeBreakpoint', () => _removeBreakpoint(isolateId, breakpointId), ); @@ -1075,7 +1077,7 @@ ${globalLoadStrategy.loadModuleSnippet}("dart_sdk").developer.invokeExtension( String? step, int? frameIndex, }) => - _wrapInErrorHandler( + wrapInErrorHandlerAsync( 'resume', () => _resume( isolateId, @@ -1138,7 +1140,7 @@ ${globalLoadStrategy.loadModuleSnippet}("dart_sdk").developer.invokeExtension( String? exceptionPauseMode, bool? shouldPauseOnExit, }) => - _wrapInErrorHandler( + wrapInErrorHandlerAsync( 'setIsolatePauseMode', () => _setIsolatePauseMode( isolateId, @@ -1175,7 +1177,8 @@ ${globalLoadStrategy.loadModuleSnippet}("dart_sdk").developer.invokeExtension( } @override - Future setName(String isolateId, String name) => _wrapInErrorHandler( + Future setName(String isolateId, String name) => + wrapInErrorHandlerAsync( 'setName', () => _setName(isolateId, name), ); @@ -1189,7 +1192,7 @@ ${globalLoadStrategy.loadModuleSnippet}("dart_sdk").developer.invokeExtension( @override Future setVMName(String name) => - _wrapInErrorHandler('setVMName', () => _setVMName(name)); + wrapInErrorHandlerAsync('setVMName', () => _setVMName(name)); Future _setVMName(String name) async { _vm.name = name; @@ -1219,7 +1222,7 @@ ${globalLoadStrategy.loadModuleSnippet}("dart_sdk").developer.invokeExtension( @override Future streamListen(String streamId) => - _wrapInErrorHandler('streamListen', () => _streamListen(streamId)); + wrapInErrorHandlerAsync('streamListen', () => _streamListen(streamId)); Future _streamListen(String streamId) async { // TODO: This should return an error if the stream is already being listened @@ -1476,7 +1479,7 @@ ${globalLoadStrategy.loadModuleSnippet}("dart_sdk").developer.invokeExtension( } @override - Future getSupportedProtocols() => _wrapInErrorHandler( + Future getSupportedProtocols() => wrapInErrorHandlerAsync( 'getSupportedProtocols', _getSupportedProtocols, ); @@ -1511,23 +1514,6 @@ ${globalLoadStrategy.loadModuleSnippet}("dart_sdk").developer.invokeExtension( return Future.error(_rpcNotSupported(method)); } - Future _wrapInErrorHandler( - String command, - Future Function() asyncCallback, - ) { - try { - return asyncCallback(); - } on RPCError catch (_) { - rethrow; - } catch (e) { - throw RPCError( - command, - RPCErrorKind.kInternalError.code, - 'Unexpected DWDS error for $command: $e', - ); - } - } - @override Future getProcessMemoryUsage() => _rpcNotSupportedFuture('getProcessMemoryUsage'); diff --git a/dwds/lib/src/utilities/shared.dart b/dwds/lib/src/utilities/shared.dart index df7e3425c..29efac116 100644 --- a/dwds/lib/src/utilities/shared.dart +++ b/dwds/lib/src/utilities/shared.dart @@ -25,3 +25,20 @@ void safeUnawaited( _logger.warning('Error in unawaited Future:', error, stackTrace); unawaited(future.catchError(onError)); } + +/// Throws an [RPCError] if the [asyncCallback] has an exception. +Future wrapInErrorHandlerAsync( + String command, + Future Function() asyncCallback, +) { + return asyncCallback().catchError((error) { + if (error is RPCError) { + throw error; + } + throw RPCError( + command, + RPCErrorKind.kInternalError.code, + 'Unexpected DWDS error for $command: $error', + ); + }); +} diff --git a/dwds/test/utilities_test.dart b/dwds/test/utilities_test.dart new file mode 100644 index 000000000..c9923e917 --- /dev/null +++ b/dwds/test/utilities_test.dart @@ -0,0 +1,62 @@ +// Copyright 2023 The Dart Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +@Timeout(Duration(minutes: 1)) +import 'package:dwds/src/utilities/shared.dart'; +import 'package:test/test.dart'; +import 'package:vm_service/vm_service.dart'; + +import 'fixtures/context.dart'; + +void main() { + group('wrapInErrorHandlerAsync', () { + test('returns future success value if callback succeeds', () async { + Future successCallback() async { + await Future.delayed(Duration(milliseconds: 500)); + return true; + } + + final result = + await wrapInErrorHandlerAsync('successCallback', successCallback); + expect(result, equals(true)); + }); + + test('throws RPCError if callback throws RPCError', () async { + Future rpcErrorCallback() async { + await Future.delayed(Duration(milliseconds: 500)); + throw RPCError( + 'rpcErrorCallback', + RPCErrorKind.kInvalidRequest.code, + 'An error message', + ); + } + + try { + await wrapInErrorHandlerAsync('rpcErrorCallback', rpcErrorCallback); + fail("RPCError not thrown."); + } catch (error) { + expect(error, isRPCErrorWithMessage('An error message')); + } + }); + + test('throws RPCError if callback throws other error type', () async { + Future exceptionCallback() async { + await Future.delayed(Duration(milliseconds: 500)); + throw Exception('An unexpected exception'); + } + + try { + await wrapInErrorHandlerAsync('exceptionCallback', exceptionCallback); + fail("RPCError not thrown."); + } catch (error) { + expect( + error, + isRPCErrorWithMessage( + 'Unexpected DWDS error for exceptionCallback: Exception: An unexpected exception', + ), + ); + } + }); + }); +} From 72808f277ed27dbf1699e856edfec7a21d3c1d3d Mon Sep 17 00:00:00 2001 From: Elliott Brooks <21270878+elliette@users.noreply.github.com> Date: Wed, 14 Jun 2023 11:22:46 -0700 Subject: [PATCH 06/10] Format --- dwds/lib/src/services/chrome_proxy_service.dart | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/dwds/lib/src/services/chrome_proxy_service.dart b/dwds/lib/src/services/chrome_proxy_service.dart index 53963f8d4..f02304954 100644 --- a/dwds/lib/src/services/chrome_proxy_service.dart +++ b/dwds/lib/src/services/chrome_proxy_service.dart @@ -740,7 +740,9 @@ ${globalLoadStrategy.loadModuleSnippet}("dart_sdk").developer.invokeExtension( @override Future getMemoryUsage(String isolateId) => wrapInErrorHandlerAsync( - 'getMemoryUsage', () => _getMemoryUsage(isolateId)); + 'getMemoryUsage', + () => _getMemoryUsage(isolateId), + ); Future _getMemoryUsage(String isolateId) async { await isInitialized; From 3fa9286c9ec8e6094990bcf01a8933ceee17f0ce Mon Sep 17 00:00:00 2001 From: Elliott Brooks <21270878+elliette@users.noreply.github.com> Date: Wed, 14 Jun 2023 13:48:18 -0700 Subject: [PATCH 07/10] Rethrow SentinelExceptions as well --- dwds/lib/src/utilities/shared.dart | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/dwds/lib/src/utilities/shared.dart b/dwds/lib/src/utilities/shared.dart index 29efac116..29f5a0dd7 100644 --- a/dwds/lib/src/utilities/shared.dart +++ b/dwds/lib/src/utilities/shared.dart @@ -27,12 +27,16 @@ void safeUnawaited( } /// Throws an [RPCError] if the [asyncCallback] has an exception. +/// +/// If the exception is of type [RPCError] or [SentinelException] +/// (the two supported exception types of package:vm_service) then +/// simply rethrow that exception. Future wrapInErrorHandlerAsync( String command, Future Function() asyncCallback, ) { return asyncCallback().catchError((error) { - if (error is RPCError) { + if (error is RPCError || error is SentinelException) { throw error; } throw RPCError( From 41da5cbe3d3a745080c82e86c7ab84c9e42f2f91 Mon Sep 17 00:00:00 2001 From: Elliott Brooks <21270878+elliette@users.noreply.github.com> Date: Wed, 14 Jun 2023 16:58:50 -0700 Subject: [PATCH 08/10] Respond to PR comments --- dwds/lib/src/utilities/shared.dart | 26 +++++++++++++------------- dwds/test/utilities_test.dart | 19 +++++++++++++++++++ 2 files changed, 32 insertions(+), 13 deletions(-) diff --git a/dwds/lib/src/utilities/shared.dart b/dwds/lib/src/utilities/shared.dart index 29f5a0dd7..5853d5b73 100644 --- a/dwds/lib/src/utilities/shared.dart +++ b/dwds/lib/src/utilities/shared.dart @@ -28,21 +28,21 @@ void safeUnawaited( /// Throws an [RPCError] if the [asyncCallback] has an exception. /// -/// If the exception is of type [RPCError] or [SentinelException] -/// (the two supported exception types of package:vm_service) then -/// simply rethrow that exception. +/// Only throws a new exception if the original exception type was not +/// [RPCError] or [SentinelException] (the two supported exception types of +/// package:vm_service). Future wrapInErrorHandlerAsync( String command, Future Function() asyncCallback, ) { - return asyncCallback().catchError((error) { - if (error is RPCError || error is SentinelException) { - throw error; - } - throw RPCError( - command, - RPCErrorKind.kInternalError.code, - 'Unexpected DWDS error for $command: $error', - ); - }); + return asyncCallback().catchError( + (error) { + throw RPCError( + command, + RPCErrorKind.kInternalError.code, + 'Unexpected DWDS error for $command: $error', + ); + }, + test: (e) => e is! RPCError && e is! SentinelException, + ); } diff --git a/dwds/test/utilities_test.dart b/dwds/test/utilities_test.dart index c9923e917..725110beb 100644 --- a/dwds/test/utilities_test.dart +++ b/dwds/test/utilities_test.dart @@ -40,6 +40,25 @@ void main() { } }); + test('throws SentinelException if callback throws SentinelException', + () async { + Future sentinelExceptionCallback() async { + await Future.delayed(Duration(milliseconds: 500)); + throw SentinelException.parse( + 'sentinelExceptionCallback', + {'message': 'a sentinel exception'}, + ); + } + + try { + await wrapInErrorHandlerAsync( + 'sentinelExceptionCallback', sentinelExceptionCallback); + fail("SentinelException not thrown."); + } catch (error) { + expect(error, isSentinelException); + } + }); + test('throws RPCError if callback throws other error type', () async { Future exceptionCallback() async { await Future.delayed(Duration(milliseconds: 500)); From 259330c926ddfe0b90295e2db1d3422c2709ae95 Mon Sep 17 00:00:00 2001 From: Elliott Brooks <21270878+elliette@users.noreply.github.com> Date: Wed, 14 Jun 2023 17:13:27 -0700 Subject: [PATCH 09/10] Respond to more PR comments --- dwds/lib/src/utilities/shared.dart | 10 ++++++---- dwds/test/utilities_test.dart | 22 +++++++++------------- 2 files changed, 15 insertions(+), 17 deletions(-) diff --git a/dwds/lib/src/utilities/shared.dart b/dwds/lib/src/utilities/shared.dart index 5853d5b73..16a79f0e2 100644 --- a/dwds/lib/src/utilities/shared.dart +++ b/dwds/lib/src/utilities/shared.dart @@ -37,10 +37,12 @@ Future wrapInErrorHandlerAsync( ) { return asyncCallback().catchError( (error) { - throw RPCError( - command, - RPCErrorKind.kInternalError.code, - 'Unexpected DWDS error for $command: $error', + return Future.error( + RPCError( + command, + RPCErrorKind.kInternalError.code, + 'Unexpected DWDS error for $command: $error', + ), ); }, test: (e) => e is! RPCError && e is! SentinelException, diff --git a/dwds/test/utilities_test.dart b/dwds/test/utilities_test.dart index 725110beb..593be8303 100644 --- a/dwds/test/utilities_test.dart +++ b/dwds/test/utilities_test.dart @@ -32,12 +32,10 @@ void main() { ); } - try { - await wrapInErrorHandlerAsync('rpcErrorCallback', rpcErrorCallback); - fail("RPCError not thrown."); - } catch (error) { - expect(error, isRPCErrorWithMessage('An error message')); - } + await expectLater( + wrapInErrorHandlerAsync('rpcErrorCallback', rpcErrorCallback), + throwsRPCErrorWithMessage('An error message'), + ); }); test('throws SentinelException if callback throws SentinelException', @@ -50,13 +48,11 @@ void main() { ); } - try { - await wrapInErrorHandlerAsync( - 'sentinelExceptionCallback', sentinelExceptionCallback); - fail("SentinelException not thrown."); - } catch (error) { - expect(error, isSentinelException); - } + await expectLater( + wrapInErrorHandlerAsync( + 'sentinelExceptionCallback', sentinelExceptionCallback), + throwsSentinelException, + ); }); test('throws RPCError if callback throws other error type', () async { From 7b06c34eb052c9f710ba4c9ed47638246e403663 Mon Sep 17 00:00:00 2001 From: Elliott Brooks <21270878+elliette@users.noreply.github.com> Date: Fri, 16 Jun 2023 09:35:30 -0700 Subject: [PATCH 10/10] Trailing comma' --- dwds/test/utilities_test.dart | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/dwds/test/utilities_test.dart b/dwds/test/utilities_test.dart index 593be8303..4da71765a 100644 --- a/dwds/test/utilities_test.dart +++ b/dwds/test/utilities_test.dart @@ -50,7 +50,9 @@ void main() { await expectLater( wrapInErrorHandlerAsync( - 'sentinelExceptionCallback', sentinelExceptionCallback), + 'sentinelExceptionCallback', + sentinelExceptionCallback, + ), throwsSentinelException, ); });