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 diff --git a/dwds/lib/src/services/chrome_proxy_service.dart b/dwds/lib/src/services/chrome_proxy_service.dart index 2d57576b7..f02304954 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 wrapInErrorHandlerAsync( + '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, + }) => + wrapInErrorHandlerAsync( + '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, + }) => + wrapInErrorHandlerAsync( + '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, + }) => + wrapInErrorHandlerAsync( + '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, + }) => + wrapInErrorHandlerAsync( + '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) => wrapInErrorHandlerAsync( + 'getIsolate', + () => _getIsolate(isolateId), + ); + + Future _getIsolate(String isolateId) { return captureElapsedTime( () async { await isInitialized; @@ -655,7 +738,13 @@ ${globalLoadStrategy.loadModuleSnippet}("dart_sdk").developer.invokeExtension( } @override - Future getMemoryUsage(String isolateId) async { + Future getMemoryUsage(String isolateId) => + wrapInErrorHandlerAsync( + 'getMemoryUsage', + () => _getMemoryUsage(isolateId), + ); + + Future _getMemoryUsage(String isolateId) async { await isInitialized; _checkIsolate('getMemoryUsage', isolateId); return inspector.getMemoryUsage(); @@ -667,6 +756,22 @@ ${globalLoadStrategy.loadModuleSnippet}("dart_sdk").developer.invokeExtension( String objectId, { int? offset, int? count, + }) => + wrapInErrorHandlerAsync( + '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 +779,12 @@ ${globalLoadStrategy.loadModuleSnippet}("dart_sdk").developer.invokeExtension( } @override - Future getScripts(String isolateId) { + Future getScripts(String isolateId) => wrapInErrorHandlerAsync( + 'getScripts', + () => _getScripts(isolateId), + ); + + Future _getScripts(String isolateId) { return captureElapsedTime( () async { await isInitialized; @@ -695,6 +805,30 @@ ${globalLoadStrategy.loadModuleSnippet}("dart_sdk").developer.invokeExtension( bool? forceCompile, bool? reportLines, List? libraryFilters, + }) => + wrapInErrorHandlerAsync( + '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 +854,13 @@ ${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}) => + wrapInErrorHandlerAsync( + 'getStack', + () => _getStack(isolateId, limit: limit), + ); + + Future _getStack(String isolateId, {int? limit}) async { await isInitialized; await isStarted; _checkIsolate('getStack', isolateId); @@ -728,7 +868,9 @@ ${globalLoadStrategy.loadModuleSnippet}("dart_sdk").developer.invokeExtension( } @override - Future getVM() { + Future getVM() => wrapInErrorHandlerAsync('getVM', _getVM); + + Future _getVM() { return captureElapsedTime( () async { await isInitialized; @@ -752,7 +894,10 @@ ${globalLoadStrategy.loadModuleSnippet}("dart_sdk").developer.invokeExtension( } @override - Future getVersion() async { + Future getVersion() => + wrapInErrorHandlerAsync('getVersion', _getVersion); + + Future _getVersion() async { final version = semver.Version.parse(vmServiceVersion); return Version(major: version.major, minor: version.minor); } @@ -764,6 +909,24 @@ ${globalLoadStrategy.loadModuleSnippet}("dart_sdk").developer.invokeExtension( String selector, List argumentIds, { bool? disableBreakpoints, + }) => + wrapInErrorHandlerAsync( + '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 +982,10 @@ ${globalLoadStrategy.loadModuleSnippet}("dart_sdk").developer.invokeExtension( } @override - Future pause(String isolateId) async { + Future pause(String isolateId) => + wrapInErrorHandlerAsync('pause', () => _pause(isolateId)); + + Future _pause(String isolateId) async { await isInitialized; _checkIsolate('pause', isolateId); return (await debuggerFuture).pause(); @@ -832,6 +998,16 @@ ${globalLoadStrategy.loadModuleSnippet}("dart_sdk").developer.invokeExtension( String isolateId, List uris, { bool? local, + }) => + wrapInErrorHandlerAsync( + 'lookupResolvedPackageUris', + () => _lookupResolvedPackageUris(isolateId, uris, local: local), + ); + + Future _lookupResolvedPackageUris( + String isolateId, + List uris, { + bool? local, }) async { await isInitialized; _checkIsolate('lookupResolvedPackageUris', isolateId); @@ -839,7 +1015,16 @@ ${globalLoadStrategy.loadModuleSnippet}("dart_sdk").developer.invokeExtension( } @override - Future lookupPackageUris(String isolateId, List uris) async { + Future lookupPackageUris(String isolateId, List uris) => + wrapInErrorHandlerAsync( + 'lookupPackageUris', + () => _lookupPackageUris(isolateId, uris), + ); + + Future _lookupPackageUris( + String isolateId, + List uris, + ) async { await isInitialized; _checkIsolate('lookupPackageUris', isolateId); return UriList(uris: uris.map(DartUri.toPackageUri).toList()); @@ -871,6 +1056,15 @@ ${globalLoadStrategy.loadModuleSnippet}("dart_sdk").developer.invokeExtension( Future removeBreakpoint( String isolateId, String breakpointId, + ) => + wrapInErrorHandlerAsync( + 'removeBreakpoint', + () => _removeBreakpoint(isolateId, breakpointId), + ); + + Future _removeBreakpoint( + String isolateId, + String breakpointId, ) async { await isInitialized; _checkIsolate('removeBreakpoint', isolateId); @@ -884,6 +1078,20 @@ ${globalLoadStrategy.loadModuleSnippet}("dart_sdk").developer.invokeExtension( String isolateId, { String? step, int? frameIndex, + }) => + wrapInErrorHandlerAsync( + 'resume', + () => _resume( + isolateId, + step: step, + frameIndex: frameIndex, + ), + ); + + Future _resume( + String isolateId, { + String? step, + int? frameIndex, }) async { try { if (inspector.appConnection.isStarted) { @@ -933,6 +1141,20 @@ ${globalLoadStrategy.loadModuleSnippet}("dart_sdk").developer.invokeExtension( String isolateId, { String? exceptionPauseMode, bool? shouldPauseOnExit, + }) => + wrapInErrorHandlerAsync( + '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 +1179,13 @@ ${globalLoadStrategy.loadModuleSnippet}("dart_sdk").developer.invokeExtension( } @override - Future setName(String isolateId, String name) async { + Future setName(String isolateId, String name) => + wrapInErrorHandlerAsync( + 'setName', + () => _setName(isolateId, name), + ); + + Future _setName(String isolateId, String name) async { await isInitialized; _checkIsolate('setName', isolateId); inspector.isolate.name = name; @@ -965,7 +1193,10 @@ ${globalLoadStrategy.loadModuleSnippet}("dart_sdk").developer.invokeExtension( } @override - Future setVMName(String name) async { + Future setVMName(String name) => + wrapInErrorHandlerAsync('setVMName', () => _setVMName(name)); + + Future _setVMName(String name) async { _vm.name = name; _streamNotify( 'VM', @@ -992,7 +1223,10 @@ ${globalLoadStrategy.loadModuleSnippet}("dart_sdk").developer.invokeExtension( } @override - Future streamListen(String streamId) async { + Future streamListen(String streamId) => + wrapInErrorHandlerAsync('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 +1481,12 @@ ${globalLoadStrategy.loadModuleSnippet}("dart_sdk").developer.invokeExtension( } @override - Future getSupportedProtocols() async { + Future getSupportedProtocols() => wrapInErrorHandlerAsync( + 'getSupportedProtocols', + _getSupportedProtocols, + ); + + Future _getSupportedProtocols() async { final version = semver.Version.parse(vmServiceVersion); return ProtocolList( protocols: [ diff --git a/dwds/lib/src/utilities/shared.dart b/dwds/lib/src/utilities/shared.dart index df7e3425c..16a79f0e2 100644 --- a/dwds/lib/src/utilities/shared.dart +++ b/dwds/lib/src/utilities/shared.dart @@ -25,3 +25,26 @@ void safeUnawaited( _logger.warning('Error in unawaited Future:', error, stackTrace); unawaited(future.catchError(onError)); } + +/// Throws an [RPCError] if the [asyncCallback] has an 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) { + 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 new file mode 100644 index 000000000..4da71765a --- /dev/null +++ b/dwds/test/utilities_test.dart @@ -0,0 +1,79 @@ +// 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', + ); + } + + await expectLater( + wrapInErrorHandlerAsync('rpcErrorCallback', rpcErrorCallback), + throwsRPCErrorWithMessage('An error message'), + ); + }); + + 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'}, + ); + } + + await expectLater( + wrapInErrorHandlerAsync( + 'sentinelExceptionCallback', + sentinelExceptionCallback, + ), + throwsSentinelException, + ); + }); + + 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', + ), + ); + } + }); + }); +}