From 408918d6f5e28250cb48f50bd2b444bcc2f7982f Mon Sep 17 00:00:00 2001 From: Kenzie Schmoll Date: Mon, 18 Mar 2024 21:50:09 +0000 Subject: [PATCH] [dds] Start DTD from DevTools server if it is not already started. Fixes https://github.com/dart-lang/sdk/issues/54937. Tested: pkg/dartdev test for `dart devtools` command, and new `dtd_test.dart` in pkg/dds. Change-Id: I530ba2fe4d5809082378b61c282ba7856974e21e Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/354460 Commit-Queue: Kenzie Davisson Reviewed-by: Ben Konyi Reviewed-by: Dan Chevalier --- pkg/dartdev/test/commands/devtools_test.dart | 63 ++++++++++---- pkg/dds/CHANGELOG.md | 1 + pkg/dds/bin/dds.dart | 2 + pkg/dds/lib/dds.dart | 6 ++ pkg/dds/lib/devtools_server.dart | 30 +++++-- pkg/dds/lib/src/dds_impl.dart | 29 ++++++- pkg/dds/lib/src/devtools/dtd.dart | 87 ++++++++++++++++++++ pkg/dds/lib/src/devtools/handler.dart | 17 +++- pkg/dds/test/dap/mocks.dart | 4 + pkg/dds/test/devtools_server/dtd_test.dart | 46 +++++++++++ pkg/dtd_impl/bin/dtd.dart | 11 ++- pkg/dtd_impl/lib/dart_tooling_daemon.dart | 24 ++++-- sdk/lib/_internal/vm/bin/vmservice_io.dart | 6 ++ 13 files changed, 288 insertions(+), 38 deletions(-) create mode 100644 pkg/dds/lib/src/devtools/dtd.dart create mode 100644 pkg/dds/test/devtools_server/dtd_test.dart diff --git a/pkg/dartdev/test/commands/devtools_test.dart b/pkg/dartdev/test/commands/devtools_test.dart index 53de4812259..c04c9414059 100644 --- a/pkg/dartdev/test/commands/devtools_test.dart +++ b/pkg/dartdev/test/commands/devtools_test.dart @@ -70,27 +70,60 @@ void devtools() { // start the devtools server process = await p.start(['devtools', '--no-launch-browser', '--machine']); process!.stderr.transform(utf8.decoder).listen(print); - final Stream inStream = process!.stdout + + String? devToolsHost; + int? devToolsPort; + final devToolsServedCompleter = Completer(); + final dtdServedCompleter = Completer(); + + late StreamSubscription sub; + sub = process!.stdout .transform(utf8.decoder) - .transform(const LineSplitter()); + .transform(const LineSplitter()) + .listen((line) async { + final json = jsonDecode(line); + final eventName = json['event'] as String?; + final params = (json['params'] as Map?)?.cast(); + switch (eventName) { + case 'server.dtdStarted': + // {"event":"server.dtdStarted","params":{ + // "uri":"ws://127.0.0.1:50882/nQf49D0YcbONeKVq" + // }} + expect(params!['uri'], isA()); + dtdServedCompleter.complete(); + case 'server.started': + // {"event":"server.started","method":"server.started","params":{ + // "host":"127.0.0.1","port":9100,"pid":93508,"protocolVersion":"1.1.0" + // }} + expect(params!['host'], isA()); + expect(params['port'], isA()); + devToolsHost = params['host'] as String; + devToolsPort = params['port'] as int; - final line = await inStream.first; - final json = jsonDecode(line); + // We can cancel the subscription because the 'server.started' event + // is expected after the 'server.dtdStarted' event. + await sub.cancel(); + devToolsServedCompleter.complete(); + default: + } + }); - // {"event":"server.started","method":"server.started","params":{ - // "host":"127.0.0.1","port":9100,"pid":93508,"protocolVersion":"1.1.0" - // }} - expect(json['event'], 'server.started'); - expect(json['params'], isNotNull); - - final host = json['params']['host']; - final port = json['params']['port']; - expect(host, isA()); - expect(port, isA()); + await Future.wait([ + dtdServedCompleter.future, + devToolsServedCompleter.future, + ]).timeout( + const Duration(seconds: 5), + onTimeout: () => throw Exception( + 'Expected DTD and DevTools to be served, but one or both were not.', + ), + ); // Connect to the port and confirm we can load a devtools resource. HttpClient client = HttpClient(); - final httpRequest = await client.get(host, port, 'index.html'); + expect(devToolsHost, isNotNull); + expect(devToolsPort, isNotNull); + final httpRequest = + await client.get(devToolsHost!, devToolsPort!, 'index.html'); final httpResponse = await httpRequest.close(); final contents = diff --git a/pkg/dds/CHANGELOG.md b/pkg/dds/CHANGELOG.md index 99d178f34cd..27179b42a36 100644 --- a/pkg/dds/CHANGELOG.md +++ b/pkg/dds/CHANGELOG.md @@ -1,6 +1,7 @@ # 3.3.1 - [DAP] Fixed an issue introduced in 3.3.0 where `Source.name` could contain a file paths when a `package:` or `dart:` URI should have been used. - Updated `package:devtools_shared` version to ^8.0.1. +- Start the Dart Tooling Daemon from the DevTools server when a connection is not passed to the server on start. # 3.3.0 - **Breaking change:** [DAP] Several signatures in DAP debug adapter classes have been updated to use `Uri`s where they previously used `String path`s. This is to support communicating with the DAP client using URIs instead of file paths. URIs may be used only when the client sets the custom `supportsDartUris` client capability during initialization. diff --git a/pkg/dds/bin/dds.dart b/pkg/dds/bin/dds.dart index acb009f9066..230cfe23dc4 100644 --- a/pkg/dds/bin/dds.dart +++ b/pkg/dds/bin/dds.dart @@ -148,6 +148,8 @@ ${argParser.usage} 'state': 'started', 'ddsUri': dds.uri.toString(), if (dds.devToolsUri != null) 'devToolsUri': dds.devToolsUri.toString(), + if (dds.hostedDartToolingDaemon?.uri != null) + 'dtdUri': dds.hostedDartToolingDaemon!.uri, })); } catch (e, st) { writeErrorResponse(e, st); diff --git a/pkg/dds/lib/dds.dart b/pkg/dds/lib/dds.dart index 86de77ca509..cbf2c1bd494 100644 --- a/pkg/dds/lib/dds.dart +++ b/pkg/dds/lib/dds.dart @@ -150,6 +150,12 @@ abstract class DartDevelopmentService { /// Returns `null` if DevTools is not running. Uri? get devToolsUri; + /// Metadata for the Dart Tooling Daemon instance that is hosted by DevTools. + /// + /// This will be null if DTD was not started by the DevTools server. For + /// example, it may have been started by an IDE. + ({String? uri, String? secret})? get hostedDartToolingDaemon; + /// Set to `true` if this instance of [DartDevelopmentService] is accepting /// requests. bool get isRunning; diff --git a/pkg/dds/lib/devtools_server.dart b/pkg/dds/lib/devtools_server.dart index 5044c317be3..c18ebebb74a 100644 --- a/pkg/dds/lib/devtools_server.dart +++ b/pkg/dds/lib/devtools_server.dart @@ -14,6 +14,7 @@ import 'package:shelf/shelf.dart' as shelf; import 'package:shelf/shelf_io.dart' as shelf; import 'src/devtools/client.dart'; +import 'src/devtools/dtd.dart'; import 'src/devtools/handler.dart'; import 'src/devtools/machine_mode_command_handler.dart'; import 'src/devtools/memory_profile.dart'; @@ -98,6 +99,12 @@ class DevToolsServer { help: 'Port to serve DevTools on; specify 0 to automatically use any ' 'available port.', ) + ..addOption( + argDtdUri, + valueHelp: 'uri', + help: 'A URI pointing to a dart tooling daemon that devtools should ' + 'interface with.', + ) ..addFlag( argLaunchBrowser, help: @@ -116,12 +123,6 @@ class DevToolsServer { help: 'Start devtools headlessly and write memory profiling samples to the ' 'indicated file.', - ) - ..addOption( - argDtdUri, - valueHelp: 'uri', - help: 'A uri pointing to a dart tooling daemon that devtools should ' - 'interface with.', ); argParser.addSeparator('App size options:'); @@ -273,13 +274,24 @@ class DevToolsServer { clientManager = ClientManager( requestNotificationPermissions: enableNotifications, ); + + String? dtdSecret; + if (dtdUri == null) { + final (:uri, :secret) = await startDtd( + machineMode: machineMode, + // TODO(https://github.com/dart-lang/sdk/issues/55034): pass the value + // of the Dart CLI flag `--print-dtd` here. + printDtdUri: false, + ); + dtdUri = uri; + dtdSecret = secret; + } + handler ??= await defaultHandler( buildDir: customDevToolsPath!, clientManager: clientManager, analytics: DevToolsUtils.initializeAnalytics(), - // TODO(kenz): pass the DTD secret here when DTD is started by DevTools - // server. - dtd: (uri: dtdUri, secret: null), + dtd: (uri: dtdUri, secret: dtdSecret), ); HttpServer? server; diff --git a/pkg/dds/lib/src/dds_impl.dart b/pkg/dds/lib/src/dds_impl.dart index 74c9799f8c1..d15d95c8f9f 100644 --- a/pkg/dds/lib/src/dds_impl.dart +++ b/pkg/dds/lib/src/dds_impl.dart @@ -9,6 +9,7 @@ import 'dart:io'; import 'dart:math'; import 'dart:typed_data'; +import 'package:devtools_shared/devtools_server.dart' show DTDConnectionInfo; import 'package:json_rpc_2/json_rpc_2.dart' as json_rpc; import 'package:meta/meta.dart'; import 'package:shelf/shelf.dart'; @@ -24,6 +25,7 @@ import 'client.dart'; import 'client_manager.dart'; import 'constants.dart'; import 'dap_handler.dart'; +import 'devtools/dtd.dart'; import 'devtools/handler.dart'; import 'expression_evaluator.dart'; import 'isolate_manager.dart'; @@ -171,6 +173,20 @@ class DartDevelopmentServiceImpl implements DartDevelopmentService { ); } pipeline = pipeline.addMiddleware(_authCodeMiddleware); + + if (_devToolsConfiguration?.enable ?? false) { + // If we are enabling DevTools in DDS, then we also need to start the Dart + // tooling daemon, since this is usually the responsibility of the + // DevTools server when a DTD uri is not already passed to the DevTools + // server on start. + _hostedDartToolingDaemon = await startDtd( + machineMode: false, + // TODO(https://github.com/dart-lang/sdk/issues/55034): pass the value + // of the Dart CLI flag `--print-dtd` here. + printDtdUri: false, + ); + } + final handler = pipeline.addHandler(_handlers().handler); // Start the DDS server. late String errorMessage; @@ -348,13 +364,17 @@ class DartDevelopmentServiceImpl implements DartDevelopmentService { // If DDS is serving DevTools, install the DevTools handlers and forward // any unhandled HTTP requests to the VM service. - if (_devToolsConfiguration != null && _devToolsConfiguration!.enable) { + if (_devToolsConfiguration?.enable ?? false) { final String buildDir = _devToolsConfiguration!.customBuildDirectoryPath.toFilePath(); return defaultHandler( dds: this, buildDir: buildDir, notFoundHandler: notFoundHandler, + dtd: ( + uri: _hostedDartToolingDaemon?.uri, + secret: _hostedDartToolingDaemon?.secret + ), ) as FutureOr Function(Request); } @@ -464,6 +484,8 @@ class DartDevelopmentServiceImpl implements DartDevelopmentService { return _devToolsUri; } + Uri? _devToolsUri; + @override void setExternalDevToolsUri(Uri uri) { if (_devToolsConfiguration?.enable ?? false) { @@ -472,7 +494,10 @@ class DartDevelopmentServiceImpl implements DartDevelopmentService { _devToolsUri = uri; } - Uri? _devToolsUri; + @override + DTDConnectionInfo? get hostedDartToolingDaemon => _hostedDartToolingDaemon; + + DTDConnectionInfo? _hostedDartToolingDaemon; final bool _ipv6; diff --git a/pkg/dds/lib/src/devtools/dtd.dart b/pkg/dds/lib/src/devtools/dtd.dart new file mode 100644 index 00000000000..f5c476984f8 --- /dev/null +++ b/pkg/dds/lib/src/devtools/dtd.dart @@ -0,0 +1,87 @@ +// Copyright (c) 2024, 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. + +import 'dart:async'; +import 'dart:convert'; +import 'dart:io'; +import 'dart:isolate'; + +import 'package:devtools_shared/devtools_server.dart' show DTDConnectionInfo; +import 'package:path/path.dart' as path; + +import 'utils.dart'; + +Future startDtd({ + required bool machineMode, + required bool printDtdUri, +}) async { + final sdkPath = File(Platform.resolvedExecutable).parent.parent.path; + String dtdSnapshot = path.absolute( + sdkPath, + 'bin', + 'snapshots', + 'dart_tooling_daemon.dart.snapshot', + ); + + final completer = Completer(); + void completeForError() => completer.complete((uri: null, secret: null)); + + final exitPort = ReceivePort() + ..listen((_) { + completeForError(); + }); + final errorPort = ReceivePort() + ..listen((_) { + completeForError(); + }); + final receivePort = ReceivePort() + ..listen((message) { + try { + // [message] is a JSON encoded String from package:dtd_impl. + final json = jsonDecode(message) as Map; + if (json + case { + 'tooling_daemon_details': { + 'uri': String uri, + 'trusted_client_secret': String secret, + } + }) { + if (printDtdUri || machineMode) { + DevToolsUtils.printOutput( + 'Serving the Dart Tooling Daemon at $uri', + { + 'event': 'server.dtdStarted', + 'params': {'uri': uri}, + }, + machineMode: machineMode, + ); + } + completer.complete((uri: uri, secret: secret)); + } + } catch (_) { + completeForError(); + } + }); + + try { + await Isolate.spawnUri( + Uri.file(dtdSnapshot), + ['--machine'], + receivePort.sendPort, + onExit: exitPort.sendPort, + onError: errorPort.sendPort, + ); + } catch (_, __) { + completeForError(); + } + + final result = await completer.future.timeout( + const Duration(seconds: 5), + onTimeout: () => (uri: null, secret: null), + ); + receivePort.close(); + errorPort.close(); + exitPort.close(); + return result; +} diff --git a/pkg/dds/lib/src/devtools/handler.dart b/pkg/dds/lib/src/devtools/handler.dart index f70548e2df7..24cf4fd6675 100644 --- a/pkg/dds/lib/src/devtools/handler.dart +++ b/pkg/dds/lib/src/devtools/handler.dart @@ -28,10 +28,17 @@ import 'utils.dart'; /// [buildDir] is the path to the pre-compiled DevTools instance to be served. /// /// [notFoundHandler] is a [Handler] to which requests that could not be handled -/// by the DevTools handler are forwarded (e.g., a proxy to the VM service). +/// by the DevTools handler are forwarded (e.g., a proxy to the VM +/// service). /// /// If [dds] is null, DevTools is not being served by a DDS instance and is /// served by a standalone server (see `package:dds/devtools_server.dart`). +/// +/// If [dtd] or [dtd.uri] is null, the Dart Tooling Daemon is not available for +/// this DevTools server connection. +/// +/// If [dtd.uri] is non-null, but [dtd.secret] is null, then DTD was started by a +/// client that is not the DevTools server (e.g. an IDE). FutureOr defaultHandler({ DartDevelopmentServiceImpl? dds, required String buildDir, @@ -183,7 +190,9 @@ Future _serveStaticFile( try { fileBytes = file.readAsBytesSync(); } catch (e) { - return Response.notFound('could not read file as bytes: ${file.path}'); + return Response.notFound( + 'could not read file as bytes: ${file.path}', + ); } } return Response.ok(fileBytes, headers: headers); @@ -193,7 +202,9 @@ Future _serveStaticFile( try { contents = file.readAsStringSync(); } catch (e) { - return Response.notFound('could not read file as String: ${file.path}'); + return Response.notFound( + 'could not read file as String: ${file.path}', + ); } if (baseHref != null) { diff --git a/pkg/dds/test/dap/mocks.dart b/pkg/dds/test/dap/mocks.dart index 9f22eea8b12..21e3d97d94c 100644 --- a/pkg/dds/test/dap/mocks.dart +++ b/pkg/dds/test/dap/mocks.dart @@ -10,6 +10,7 @@ import 'package:dds/dds.dart'; import 'package:dds/src/dap/adapters/dart_cli_adapter.dart'; import 'package:dds/src/dap/adapters/dart_test_adapter.dart'; import 'package:dds/src/dap/isolate_manager.dart'; +import 'package:devtools_shared/devtools_server.dart' show DTDConnectionInfo; import 'package:vm_service/vm_service.dart'; /// A [DartCliDebugAdapter] that captures information about the process that @@ -243,6 +244,9 @@ class MockDartDevelopmentService implements DartDevelopmentService { @override Uri? get devToolsUri => throw UnimplementedError(); + @override + DTDConnectionInfo? get hostedDartToolingDaemon => throw UnimplementedError(); + @override Future get done => throw UnimplementedError(); diff --git a/pkg/dds/test/devtools_server/dtd_test.dart b/pkg/dds/test/devtools_server/dtd_test.dart new file mode 100644 index 00000000000..e097f632b86 --- /dev/null +++ b/pkg/dds/test/devtools_server/dtd_test.dart @@ -0,0 +1,46 @@ +// Copyright 2024 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +import 'package:dds/devtools_server.dart'; +import 'package:test/test.dart'; + +import 'utils/server_driver.dart'; + +void main() { + group('Dart Tooling Daemon connection', () { + test('does not start DTD when a DTD uri is passed as an argument', + () async { + final server = await DevToolsServerDriver.create( + additionalArgs: ['--${DevToolsServer.argDtdUri}=some_uri'], + ); + try { + final dtdStartedEvent = await server.stdout + .firstWhere( + (map) => map!['event'] == 'server.dtdStarted', + orElse: () => null, + ) + .timeout( + const Duration(seconds: 3), + onTimeout: () => null, + ); + expect(dtdStartedEvent, isNull); + } finally { + server.kill(); + } + }); + + test('starts DTD when no DTD uri is passed as an argument', () async { + final server = await DevToolsServerDriver.create(); + try { + final dtdStartedEvent = await server.stdout.firstWhere( + (map) => map!['event'] == 'server.dtdStarted', + orElse: () => null, + ); + expect(dtdStartedEvent, isNotNull); + } finally { + server.kill(); + } + }); + }); +} diff --git a/pkg/dtd_impl/bin/dtd.dart b/pkg/dtd_impl/bin/dtd.dart index 4d7cc939282..206509606d9 100644 --- a/pkg/dtd_impl/bin/dtd.dart +++ b/pkg/dtd_impl/bin/dtd.dart @@ -2,11 +2,20 @@ // 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. +import 'dart:isolate'; + import 'package:dtd_impl/dart_tooling_daemon.dart'; -void main(List args) async { +/// Starts the Dart Tooling Daemon with a list of arguments and a nullable +/// Object [port], which will be cast as a [SendPort?] object. +/// +/// When [port] is non-null, the [DartToolingDaemon.startService] method will +/// send information about the DTD connection back over [port] instead of +/// printing it to stdout. +void main(List args, dynamic port) async { await DartToolingDaemon.startService( args, shouldLogRequests: true, + sendPort: port as SendPort?, ); // TODO(@danchevalier): turn off logging } diff --git a/pkg/dtd_impl/lib/dart_tooling_daemon.dart b/pkg/dtd_impl/lib/dart_tooling_daemon.dart index 27596741044..a9596c65f83 100644 --- a/pkg/dtd_impl/lib/dart_tooling_daemon.dart +++ b/pkg/dtd_impl/lib/dart_tooling_daemon.dart @@ -5,6 +5,7 @@ import 'dart:async'; import 'dart:convert'; import 'dart:io'; +import 'dart:isolate'; import 'dart:math'; import 'package:args/args.dart'; @@ -162,11 +163,15 @@ class DartToolingDaemon { /// Set [ipv6] to true to have the service use ipv6 instead of ipv4. /// /// Set [shouldLogRequests] to true to enable logging. + /// + /// When [sendPort] is non-null, information about the DTD connection will be + /// sent over [port] instead of being printed to stdout. static Future startService( List args, { bool ipv6 = false, bool shouldLogRequests = false, int port = 0, + SendPort? sendPort, }) async { final argParser = DartToolingDaemonOptions.createArgParser(); final parsedArgs = argParser.parse(args); @@ -186,14 +191,17 @@ class DartToolingDaemon { ); await dtd._startService(port: port); if (machineMode) { - print( - jsonEncode({ - 'tooling_daemon_details': { - 'uri': dtd.uri.toString(), - ...(!unrestrictedMode ? {'trusted_client_secret': secret} : {}), - }, - }), - ); + final encoded = jsonEncode({ + 'tooling_daemon_details': { + 'uri': dtd.uri.toString(), + ...(!unrestrictedMode ? {'trusted_client_secret': secret} : {}), + }, + }); + if (sendPort == null) { + print(encoded); + } else { + sendPort.send(encoded); + } } else { print( 'The Dart Tooling Daemon is listening on ' diff --git a/sdk/lib/_internal/vm/bin/vmservice_io.dart b/sdk/lib/_internal/vm/bin/vmservice_io.dart index 5f02fedc641..05cd7b3e58b 100644 --- a/sdk/lib/_internal/vm/bin/vmservice_io.dart +++ b/sdk/lib/_internal/vm/bin/vmservice_io.dart @@ -145,6 +145,12 @@ class _DebuggingSession { final state = result['state']; if (state == 'started') { if (result.containsKey('devToolsUri')) { + // TODO(https://github.com/dart-lang/sdk/issues/55034): only print + // this if the Dart CLI flag `--print-dtd` is present. + if (result.containsKey('dtdUri') && false) { + final dtdUri = result['dtdUri']; + print('The Dart Tooling Daemon is listening on $dtdUri'); + } // NOTE: update pkg/dartdev/lib/src/commands/run.dart if this message // is changed to ensure consistency. const devToolsMessagePrefix =