diff --git a/pkg/dds/CHANGELOG.md b/pkg/dds/CHANGELOG.md index df94fc114d1..2884166c84e 100644 --- a/pkg/dds/CHANGELOG.md +++ b/pkg/dds/CHANGELOG.md @@ -1,6 +1,11 @@ +# 1.2.1 + +- Fixed issue where `evaluate` and `evaluateInFrame` were not invoking client + provided implementations of `compileExpression`. + # 1.2.0 -- Fix issue where forwarding requests with no RPC parameters would return an +- Fixed issue where forwarding requests with no RPC parameters would return an RPC error. # 1.1.0 diff --git a/pkg/dds/lib/dds.dart b/pkg/dds/lib/dds.dart index 79d5768e4f4..90461e60dfc 100644 --- a/pkg/dds/lib/dds.dart +++ b/pkg/dds/lib/dds.dart @@ -29,6 +29,7 @@ part 'src/client.dart'; part 'src/client_manager.dart'; part 'src/constants.dart'; part 'src/dds_impl.dart'; +part 'src/expression_evaluator.dart'; part 'src/logging_repository.dart'; part 'src/isolate_manager.dart'; part 'src/named_lookup.dart'; diff --git a/pkg/dds/lib/src/client.dart b/pkg/dds/lib/src/client.dart index 7febf084e50..40991286cad 100644 --- a/pkg/dds/lib/src/client.dart +++ b/pkg/dds/lib/src/client.dart @@ -157,6 +157,19 @@ class _DartDevelopmentServiceClient { return supportedProtocols; }); + // `evaluate` and `evaluateInFrame` actually consist of multiple RPC + // invocations, including a call to `compileExpression` which can be + // overridden by clients which provide their own implementation (e.g., + // Flutter Tools). We handle all of this in [_ExpressionEvaluator]. + _clientPeer.registerMethod( + 'evaluate', + dds.expressionEvaluator.execute, + ); + _clientPeer.registerMethod( + 'evaluateInFrame', + dds.expressionEvaluator.execute, + ); + // When invoked within a fallback, the next fallback will start executing. // The final fallback forwards the request to the VM service directly. @alwaysThrows diff --git a/pkg/dds/lib/src/client_manager.dart b/pkg/dds/lib/src/client_manager.dart index 0ab590c3cfd..8527636096a 100644 --- a/pkg/dds/lib/src/client_manager.dart +++ b/pkg/dds/lib/src/client_manager.dart @@ -133,6 +133,16 @@ class _ClientManager { } } + _DartDevelopmentServiceClient findFirstClientThatHandlesService( + String service) { + for (final client in clients) { + if (client.services.containsKey(service)) { + return client; + } + } + return null; + } + // Handles namespace generation for service extensions. static const _kServicePrologue = 's'; final NamedLookup<_DartDevelopmentServiceClient> clients = NamedLookup( diff --git a/pkg/dds/lib/src/dds_impl.dart b/pkg/dds/lib/src/dds_impl.dart index 0ab172df159..221b2c9bbda 100644 --- a/pkg/dds/lib/src/dds_impl.dart +++ b/pkg/dds/lib/src/dds_impl.dart @@ -11,6 +11,7 @@ class _DartDevelopmentService implements DartDevelopmentService { this._authCodesEnabled, ) { _clientManager = _ClientManager(this); + _expressionEvaluator = _ExpressionEvaluator(this); _isolateManager = _IsolateManager(this); _loggingRepository = _LoggingRepository(); _streamManager = _StreamManager(this); @@ -197,6 +198,9 @@ class _DartDevelopmentService implements DartDevelopmentService { _ClientManager get clientManager => _clientManager; _ClientManager _clientManager; + _ExpressionEvaluator get expressionEvaluator => _expressionEvaluator; + _ExpressionEvaluator _expressionEvaluator; + _IsolateManager get isolateManager => _isolateManager; _IsolateManager _isolateManager; diff --git a/pkg/dds/lib/src/expression_evaluator.dart b/pkg/dds/lib/src/expression_evaluator.dart new file mode 100644 index 00000000000..e10af002710 --- /dev/null +++ b/pkg/dds/lib/src/expression_evaluator.dart @@ -0,0 +1,128 @@ +// Copyright (c) 2020, the Dart project authors. Please see the AUTHORS file +// for details. All rights reserved. Use of this source code is governed by a +// BSD-style license that can be found in the LICENSE file. + +part of dds; + +/// A helper class which handles `evaluate` and `evaluateInFrame` calls by +/// potentially forwarding compilation requests to an external compilation +/// service like Flutter Tools. +class _ExpressionEvaluator { + _ExpressionEvaluator(this.dds); + + Future> execute(json_rpc.Parameters parameters) async { + final isolateId = parameters['isolateId'].asString; + final expression = parameters['expression'].asString; + Map buildScopeResponse; + + try { + buildScopeResponse = await _buildScope(parameters); + } on json_rpc.RpcException catch (e) { + throw _RpcErrorCodes.buildRpcException( + _RpcErrorCodes.kExpressionCompilationError, + data: e.data, + ); + } + String kernelBase64; + try { + kernelBase64 = + await _compileExpression(isolateId, expression, buildScopeResponse); + } on json_rpc.RpcException catch (e) { + throw _RpcErrorCodes.buildRpcException( + _RpcErrorCodes.kExpressionCompilationError, + data: e.data, + ); + } + return await _evaluateCompiledExpression( + parameters, isolateId, kernelBase64); + } + + Future> _buildScope( + json_rpc.Parameters parameters) async { + final params = _setupParams(parameters); + params['isolateId'] = parameters['isolateId'].asString; + if (parameters['scope'].asMapOr(null) != null) { + params['scope'] = parameters['scope'].asMap; + } + return await dds._vmServiceClient.sendRequest( + '_buildExpressionEvaluationScope', + params, + ); + } + + Future _compileExpression(String isolateId, String expression, + Map buildScopeResponseResult) async { + _DartDevelopmentServiceClient externalClient = + dds.clientManager.findFirstClientThatHandlesService( + 'compileExpression', + ); + + final compileParams = { + 'isolateId': isolateId, + 'expression': expression, + 'definitions': buildScopeResponseResult['param_names'], + 'typeDefinitions': buildScopeResponseResult['type_params_names'], + 'libraryUri': buildScopeResponseResult['libraryUri'], + 'isStatic': buildScopeResponseResult['isStatic'], + }; + + final klass = buildScopeResponseResult['klass']; + if (klass != null) { + compileParams['klass'] = klass; + } + // TODO(bkonyi): handle service disappeared case? + try { + if (externalClient != null) { + return (await externalClient.sendRequest( + 'compileExpression', + compileParams, + ))['result']['kernelBytes']; + } else { + // Fallback to compiling using the kernel service. + return (await dds._vmServiceClient.sendRequest( + '_compileExpression', + compileParams, + ))['kernelBytes']; + } + } on json_rpc.RpcException catch (e) { + throw _RpcErrorCodes.buildRpcException( + _RpcErrorCodes.kExpressionCompilationError, + data: e.data, + ); + } + } + + Future> _evaluateCompiledExpression( + json_rpc.Parameters parameters, + String isolateId, + String kernelBase64, + ) async { + final params = _setupParams(parameters); + params['isolateId'] = isolateId; + params['kernelBytes'] = kernelBase64; + params['disableBreakpoints'] = + parameters['disableBreakpoints'].asBoolOr(false); + if (parameters['scope'].asMapOr(null) != null) { + params['scope'] = parameters['scope'].asMap; + } + return await dds._vmServiceClient.sendRequest( + '_evaluateCompiledExpression', + params, + ); + } + + Map _setupParams(json_rpc.Parameters parameters) { + if (parameters.method == 'evaluateInFrame') { + return { + 'frameIndex': parameters['frameIndex'].asInt, + }; + } else { + assert(parameters.method == 'evaluate'); + return { + 'targetId': parameters['targetId'].asString, + }; + } + } + + final _DartDevelopmentService dds; +} diff --git a/pkg/dds/lib/src/rpc_error_codes.dart b/pkg/dds/lib/src/rpc_error_codes.dart index 84b1f62dcd5..8826a5165fe 100644 --- a/pkg/dds/lib/src/rpc_error_codes.dart +++ b/pkg/dds/lib/src/rpc_error_codes.dart @@ -5,10 +5,11 @@ part of dds; abstract class _RpcErrorCodes { - static json_rpc.RpcException buildRpcException(int code) { + static json_rpc.RpcException buildRpcException(int code, {dynamic data}) { return json_rpc.RpcException( code, errorMessages[code], + data: data, ); } @@ -34,7 +35,7 @@ abstract class _RpcErrorCodes { // static const kIsolateMustHaveReloaded = 110; static const kServiceAlreadyRegistered = 111; static const kServiceDisappeared = 112; - // static const kExpressionCompilationError = 113; + static const kExpressionCompilationError = 113; // static const kInvalidTimelineRequest = 114; // Experimental (used in private rpcs). @@ -48,5 +49,6 @@ abstract class _RpcErrorCodes { kStreamNotSubscribed: 'Stream not subscribed', kServiceAlreadyRegistered: 'Service already registered', kServiceDisappeared: 'Service has disappeared', + kExpressionCompilationError: 'Expression compilation error', }; } diff --git a/pkg/dds/pubspec.yaml b/pkg/dds/pubspec.yaml index b1049486f97..75130315daa 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.2.0 +version: 1.2.1 homepage: https://github.com/dart-lang/sdk/tree/master/pkg/dds diff --git a/runtime/observatory/tests/service/string_escaping_test.dart b/runtime/observatory/tests/service/string_escaping_test.dart index 8a11dfbcb8c..31729de85c9 100644 --- a/runtime/observatory/tests/service/string_escaping_test.dart +++ b/runtime/observatory/tests/service/string_escaping_test.dart @@ -65,7 +65,6 @@ Future testStrings(Isolate isolate) async { expectTruncatedString(String varName, String varValueAsString) { Field field = lib.variables.singleWhere((v) => v.name == varName); Instance value = field.staticValue; - print(value.valueAsString); expect(varValueAsString, startsWith(value.valueAsString)); expect(value.valueAsStringIsTruncated, isTrue); } diff --git a/runtime/vm/json_stream.cc b/runtime/vm/json_stream.cc index 0fd8b33a8b0..041f250dcb0 100644 --- a/runtime/vm/json_stream.cc +++ b/runtime/vm/json_stream.cc @@ -174,7 +174,6 @@ void JSONStream::PrintError(intptr_t code, const char* details_format, ...) { va_start(args2, details_format); Utils::VSNPrint(buffer, (len + 1), details_format, args2); va_end(args2); - data.AddProperty("details", buffer); } } diff --git a/runtime/vm/json_stream.h b/runtime/vm/json_stream.h index 89907d4dba7..b563f5f65af 100644 --- a/runtime/vm/json_stream.h +++ b/runtime/vm/json_stream.h @@ -38,6 +38,7 @@ class Zone; // // - runtime/vm/service/vmservice.dart // - runtime/observatory/lib/src/service/object.dart +// - pkg/dds/lib/src/rpc_error_codes.dart // enum JSONRpcErrorCode { kParseError = -32700, diff --git a/sdk_nnbd/lib/_internal/vm/bin/vmservice_server.dart b/sdk_nnbd/lib/_internal/vm/bin/vmservice_server.dart index 4b30879237a..26f1bfa86aa 100644 --- a/sdk_nnbd/lib/_internal/vm/bin/vmservice_server.dart +++ b/sdk_nnbd/lib/_internal/vm/bin/vmservice_server.dart @@ -362,7 +362,7 @@ class Server { }); } else { // Forward the websocket connection request to DDS. - request.response.redirect(_service.ddsUri); + request.response.redirect(_service.ddsUri!); } return; } diff --git a/sdk_nnbd/lib/vmservice/running_isolates.dart b/sdk_nnbd/lib/vmservice/running_isolates.dart index a17b889b6de..20ab8a25012 100644 --- a/sdk_nnbd/lib/vmservice/running_isolates.dart +++ b/sdk_nnbd/lib/vmservice/running_isolates.dart @@ -75,6 +75,14 @@ class _Evaluator { _Evaluator(this._message, this._isolate, this._service); Future run() async { + if (_service.ddsUri != null) { + return Response.from(encodeRpcError( + _message, + kInternalError, + details: 'Fell through to VM Service expression evaluation when a DDS ' + 'instance was connected. Please file an issue on GitHub.', + )); + } final buildScopeResponse = await _buildScope(); final responseJson = buildScopeResponse.decodeJson(); diff --git a/sdk_nnbd/lib/vmservice/vmservice.dart b/sdk_nnbd/lib/vmservice/vmservice.dart index bbfb61b77ee..23f555b27e5 100644 --- a/sdk_nnbd/lib/vmservice/vmservice.dart +++ b/sdk_nnbd/lib/vmservice/vmservice.dart @@ -47,7 +47,7 @@ final serviceAuthToken = _makeAuthToken(); final isolateEmbedderData = {}; // These must be kept in sync with the declarations in vm/json_stream.h and -// pkg/dds/lib/src/stream_manager.dart. +// pkg/dds/lib/src/rpc_error_codes.dart. const kParseError = -32700; const kInvalidRequest = -32600; const kMethodNotFound = -32601; @@ -219,7 +219,7 @@ class VMService extends MessageRouter { final devfs = DevFS(); - Uri get ddsUri => _ddsUri!; + Uri? get ddsUri => _ddsUri; Uri? _ddsUri; Future _yieldControlToDDS(Message message) async {