Wrap VM service API methods in an error handler to convert any exceptions into type `RPCError` (#2144)
Fixes https://github.com/dart-lang/webdev/issues/2140, work towards https://github.com/flutter/flutter/issues/126362
diff --git a/dwds/CHANGELOG.md b/dwds/CHANGELOG.md
index e47b955..4472831 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 2d57576..f023049 100644
--- a/dwds/lib/src/services/chrome_proxy_service.dart
+++ b/dwds/lib/src/services/chrome_proxy_service.dart
@@ -383,6 +383,18 @@
int line, {
int? column,
}) async {
+ return wrapInErrorHandlerAsync(
+ 'addBreakpoint',
+ () => _addBreakpoint(isolateId, scriptId, line),
+ );
+ }
+
+ Future<Breakpoint> _addBreakpoint(
+ String isolateId,
+ String scriptId,
+ int line, {
+ int? column,
+ }) async {
await isInitialized;
_checkIsolate('addBreakpoint', isolateId);
return (await debuggerFuture).addBreakpoint(scriptId, line, column: column);
@@ -399,6 +411,22 @@
String scriptUri,
int line, {
int? column,
+ }) =>
+ wrapInErrorHandlerAsync(
+ 'addBreakpointWithScriptUri',
+ () => _addBreakpointWithScriptUri(
+ isolateId,
+ scriptUri,
+ line,
+ column: column,
+ ),
+ );
+
+ Future<Breakpoint> _addBreakpointWithScriptUri(
+ String isolateId,
+ String scriptUri,
+ int line, {
+ int? column,
}) async {
await isInitialized;
_checkIsolate('addBreakpointWithScriptUri', isolateId);
@@ -431,6 +459,20 @@
String method, {
String? isolateId,
Map? args,
+ }) =>
+ wrapInErrorHandlerAsync(
+ 'callServiceExtension',
+ () => _callServiceExtension(
+ method,
+ isolateId: isolateId,
+ args: args,
+ ),
+ );
+
+ Future<Response> _callServiceExtension(
+ String method, {
+ String? isolateId,
+ Map? args,
}) async {
await isInitialized;
isolateId ??= _inspector?.isolate.id;
@@ -539,6 +581,24 @@
String expression, {
Map<String, String>? scope,
bool? disableBreakpoints,
+ }) =>
+ wrapInErrorHandlerAsync(
+ 'evaluate',
+ () => _evaluate(
+ isolateId,
+ targetId,
+ expression,
+ scope: scope,
+ disableBreakpoints: disableBreakpoints,
+ ),
+ );
+
+ Future<Response> _evaluate(
+ String isolateId,
+ String targetId,
+ String expression, {
+ Map<String, String>? scope,
+ bool? disableBreakpoints,
}) {
// TODO(798) - respect disableBreakpoints.
return captureElapsedTime(
@@ -578,6 +638,24 @@
String expression, {
Map<String, String>? scope,
bool? disableBreakpoints,
+ }) =>
+ wrapInErrorHandlerAsync(
+ 'evaluateInFrame',
+ () => _evaluateInFrame(
+ isolateId,
+ frameIndex,
+ expression,
+ scope: scope,
+ disableBreakpoints: disableBreakpoints,
+ ),
+ );
+
+ Future<Response> _evaluateInFrame(
+ String isolateId,
+ int frameIndex,
+ String expression, {
+ Map<String, String>? scope,
+ bool? disableBreakpoints,
}) {
// TODO(798) - respect disableBreakpoints.
@@ -643,7 +721,12 @@
}
@override
- Future<Isolate> getIsolate(String isolateId) {
+ Future<Isolate> getIsolate(String isolateId) => wrapInErrorHandlerAsync(
+ 'getIsolate',
+ () => _getIsolate(isolateId),
+ );
+
+ Future<Isolate> _getIsolate(String isolateId) {
return captureElapsedTime(
() async {
await isInitialized;
@@ -655,7 +738,13 @@
}
@override
- Future<MemoryUsage> getMemoryUsage(String isolateId) async {
+ Future<MemoryUsage> getMemoryUsage(String isolateId) =>
+ wrapInErrorHandlerAsync(
+ 'getMemoryUsage',
+ () => _getMemoryUsage(isolateId),
+ );
+
+ Future<MemoryUsage> _getMemoryUsage(String isolateId) async {
await isInitialized;
_checkIsolate('getMemoryUsage', isolateId);
return inspector.getMemoryUsage();
@@ -667,6 +756,22 @@
String objectId, {
int? offset,
int? count,
+ }) =>
+ wrapInErrorHandlerAsync(
+ 'getObject',
+ () => _getObject(
+ isolateId,
+ objectId,
+ offset: offset,
+ count: count,
+ ),
+ );
+
+ Future<Obj> _getObject(
+ String isolateId,
+ String objectId, {
+ int? offset,
+ int? count,
}) async {
await isInitialized;
_checkIsolate('getObject', isolateId);
@@ -674,7 +779,12 @@
}
@override
- Future<ScriptList> getScripts(String isolateId) {
+ Future<ScriptList> getScripts(String isolateId) => wrapInErrorHandlerAsync(
+ 'getScripts',
+ () => _getScripts(isolateId),
+ );
+
+ Future<ScriptList> _getScripts(String isolateId) {
return captureElapsedTime(
() async {
await isInitialized;
@@ -695,6 +805,30 @@
bool? forceCompile,
bool? reportLines,
List<String>? libraryFilters,
+ }) =>
+ wrapInErrorHandlerAsync(
+ 'getSourceReport',
+ () => _getSourceReport(
+ isolateId,
+ reports,
+ scriptId: scriptId,
+ tokenPos: tokenPos,
+ endTokenPos: endTokenPos,
+ forceCompile: forceCompile,
+ reportLines: reportLines,
+ libraryFilters: libraryFilters,
+ ),
+ );
+
+ Future<SourceReport> _getSourceReport(
+ String isolateId,
+ List<String> reports, {
+ String? scriptId,
+ int? tokenPos,
+ int? endTokenPos,
+ bool? forceCompile,
+ bool? reportLines,
+ List<String>? libraryFilters,
}) {
return captureElapsedTime(
() async {
@@ -720,7 +854,13 @@
///
/// The returned stack will contain up to [limit] frames if provided.
@override
- Future<Stack> getStack(String isolateId, {int? limit}) async {
+ Future<Stack> getStack(String isolateId, {int? limit}) =>
+ wrapInErrorHandlerAsync(
+ 'getStack',
+ () => _getStack(isolateId, limit: limit),
+ );
+
+ Future<Stack> _getStack(String isolateId, {int? limit}) async {
await isInitialized;
await isStarted;
_checkIsolate('getStack', isolateId);
@@ -728,7 +868,9 @@
}
@override
- Future<VM> getVM() {
+ Future<VM> getVM() => wrapInErrorHandlerAsync('getVM', _getVM);
+
+ Future<VM> _getVM() {
return captureElapsedTime(
() async {
await isInitialized;
@@ -752,7 +894,10 @@
}
@override
- Future<Version> getVersion() async {
+ Future<Version> getVersion() =>
+ wrapInErrorHandlerAsync('getVersion', _getVersion);
+
+ Future<Version> _getVersion() async {
final version = semver.Version.parse(vmServiceVersion);
return Version(major: version.major, minor: version.minor);
}
@@ -764,6 +909,24 @@
String selector,
List argumentIds, {
bool? disableBreakpoints,
+ }) =>
+ wrapInErrorHandlerAsync(
+ 'invoke',
+ () => _invoke(
+ isolateId,
+ targetId,
+ selector,
+ argumentIds,
+ disableBreakpoints: disableBreakpoints,
+ ),
+ );
+
+ Future<Response> _invoke(
+ String isolateId,
+ String targetId,
+ String selector,
+ List argumentIds, {
+ bool? disableBreakpoints,
}) async {
await isInitialized;
_checkIsolate('invoke', isolateId);
@@ -819,7 +982,10 @@
}
@override
- Future<Success> pause(String isolateId) async {
+ Future<Success> pause(String isolateId) =>
+ wrapInErrorHandlerAsync('pause', () => _pause(isolateId));
+
+ Future<Success> _pause(String isolateId) async {
await isInitialized;
_checkIsolate('pause', isolateId);
return (await debuggerFuture).pause();
@@ -832,6 +998,16 @@
String isolateId,
List<String> uris, {
bool? local,
+ }) =>
+ wrapInErrorHandlerAsync(
+ 'lookupResolvedPackageUris',
+ () => _lookupResolvedPackageUris(isolateId, uris, local: local),
+ );
+
+ Future<UriList> _lookupResolvedPackageUris(
+ String isolateId,
+ List<String> uris, {
+ bool? local,
}) async {
await isInitialized;
_checkIsolate('lookupResolvedPackageUris', isolateId);
@@ -839,7 +1015,16 @@
}
@override
- Future<UriList> lookupPackageUris(String isolateId, List<String> uris) async {
+ Future<UriList> lookupPackageUris(String isolateId, List<String> uris) =>
+ wrapInErrorHandlerAsync(
+ 'lookupPackageUris',
+ () => _lookupPackageUris(isolateId, uris),
+ );
+
+ Future<UriList> _lookupPackageUris(
+ String isolateId,
+ List<String> uris,
+ ) async {
await isInitialized;
_checkIsolate('lookupPackageUris', isolateId);
return UriList(uris: uris.map(DartUri.toPackageUri).toList());
@@ -871,6 +1056,15 @@
Future<Success> removeBreakpoint(
String isolateId,
String breakpointId,
+ ) =>
+ wrapInErrorHandlerAsync(
+ 'removeBreakpoint',
+ () => _removeBreakpoint(isolateId, breakpointId),
+ );
+
+ Future<Success> _removeBreakpoint(
+ String isolateId,
+ String breakpointId,
) async {
await isInitialized;
_checkIsolate('removeBreakpoint', isolateId);
@@ -884,6 +1078,20 @@
String isolateId, {
String? step,
int? frameIndex,
+ }) =>
+ wrapInErrorHandlerAsync(
+ 'resume',
+ () => _resume(
+ isolateId,
+ step: step,
+ frameIndex: frameIndex,
+ ),
+ );
+
+ Future<Success> _resume(
+ String isolateId, {
+ String? step,
+ int? frameIndex,
}) async {
try {
if (inspector.appConnection.isStarted) {
@@ -933,6 +1141,20 @@
String isolateId, {
String? exceptionPauseMode,
bool? shouldPauseOnExit,
+ }) =>
+ wrapInErrorHandlerAsync(
+ 'setIsolatePauseMode',
+ () => _setIsolatePauseMode(
+ isolateId,
+ exceptionPauseMode: exceptionPauseMode,
+ shouldPauseOnExit: shouldPauseOnExit,
+ ),
+ );
+
+ Future<Success> _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 @@
}
@override
- Future<Success> setName(String isolateId, String name) async {
+ Future<Success> setName(String isolateId, String name) =>
+ wrapInErrorHandlerAsync(
+ 'setName',
+ () => _setName(isolateId, name),
+ );
+
+ Future<Success> _setName(String isolateId, String name) async {
await isInitialized;
_checkIsolate('setName', isolateId);
inspector.isolate.name = name;
@@ -965,7 +1193,10 @@
}
@override
- Future<Success> setVMName(String name) async {
+ Future<Success> setVMName(String name) =>
+ wrapInErrorHandlerAsync('setVMName', () => _setVMName(name));
+
+ Future<Success> _setVMName(String name) async {
_vm.name = name;
_streamNotify(
'VM',
@@ -992,7 +1223,10 @@
}
@override
- Future<Success> streamListen(String streamId) async {
+ Future<Success> streamListen(String streamId) =>
+ wrapInErrorHandlerAsync('streamListen', () => _streamListen(streamId));
+
+ Future<Success> _streamListen(String streamId) async {
// TODO: This should return an error if the stream is already being listened
// to.
onEvent(streamId);
@@ -1247,7 +1481,12 @@
}
@override
- Future<ProtocolList> getSupportedProtocols() async {
+ Future<ProtocolList> getSupportedProtocols() => wrapInErrorHandlerAsync(
+ 'getSupportedProtocols',
+ _getSupportedProtocols,
+ );
+
+ Future<ProtocolList> _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 df7e342..16a79f0 100644
--- a/dwds/lib/src/utilities/shared.dart
+++ b/dwds/lib/src/utilities/shared.dart
@@ -25,3 +25,26 @@
_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<T> wrapInErrorHandlerAsync<T>(
+ String command,
+ Future<T> Function() asyncCallback,
+) {
+ return asyncCallback().catchError(
+ (error) {
+ return Future<T>.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 0000000..4da7176
--- /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<bool> 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<bool> 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<bool> 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<bool> 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',
+ ),
+ );
+ }
+ });
+ });
+}
