diff --git a/DEPS b/DEPS index 7dd6aee11ae..e2fe6a2944c 100644 --- a/DEPS +++ b/DEPS @@ -102,7 +102,7 @@ vars = { "boringssl_rev": "2db0eb3f96a5756298dcd7f9319e56a98585bd10", "browser-compat-data_tag": "ac8cae697014da1ff7124fba33b0b4245cc6cd1b", # v1.0.22 "cpu_features_rev": "936b9ab5515dead115606559502e3864958f7f6e", - "devtools_rev": "25053ae4af8f162188388c6f3786e03349652e51", + "devtools_rev": "f363ca0a9d233cbf238465185ab09d90cbcc75dd", "icu_rev": "43953f57b037778a1b8005564afabe214834f7bd", "jinja2_rev": "2222b31554f03e62600cd7e383376a7c187967a1", "libcxx_rev": "44079a4cc04cdeffb9cfe8067bfb3c276fb2bab0", diff --git a/pkg/dds/CHANGELOG.md b/pkg/dds/CHANGELOG.md index dc4f33e6ef0..aebd72a29c7 100644 --- a/pkg/dds/CHANGELOG.md +++ b/pkg/dds/CHANGELOG.md @@ -1,5 +1,8 @@ +# 5.0.0 +- Updated the `devtools_shared` dependency to version `^11.0.0`. + # 4.2.7 -- Added a new constant `RpcErrorCodes.kConnectionDisposed = -32010`) for requests +- Added a new constant `RpcErrorCodes.kConnectionDisposed = -32010` for requests failing because the service connection was closed. This value is not currently used but is provided for clients to handle in preperation for a future release that will use it to avoid clients having to read error messages. diff --git a/pkg/dds/lib/dds.dart b/pkg/dds/lib/dds.dart index 788615286f8..88c2ddc2d37 100644 --- a/pkg/dds/lib/dds.dart +++ b/pkg/dds/lib/dds.dart @@ -9,6 +9,8 @@ library; import 'dart:async'; import 'dart:io'; +import 'package:devtools_shared/devtools_shared.dart' show DtdInfo; + import 'src/dds_impl.dart'; typedef UriConverter = String? Function(String uri); @@ -154,7 +156,7 @@ abstract class DartDevelopmentService { /// /// 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; + DtdInfo? get hostedDartToolingDaemon; /// Set to `true` if this instance of [DartDevelopmentService] is accepting /// requests. diff --git a/pkg/dds/lib/devtools_server.dart b/pkg/dds/lib/devtools_server.dart index e2b544634ad..6009ce4a707 100644 --- a/pkg/dds/lib/devtools_server.dart +++ b/pkg/dds/lib/devtools_server.dart @@ -41,6 +41,7 @@ class DevToolsServer { static const argDdsPort = 'dds-port'; static const argDebugMode = 'debug'; static const argDtdUri = 'dtd-uri'; + static const argDtdExposedUri = 'dtd-exposed-uri'; static const argPrintDtd = 'print-dtd'; static const argLaunchBrowser = 'launch-browser'; static const argMachine = 'machine'; @@ -107,6 +108,13 @@ class DevToolsServer { help: 'A URI pointing to a Dart Tooling Daemon that DevTools should ' 'interface with.', ) + ..addOption( + argDtdExposedUri, + valueHelp: 'uri', + help: 'An optional URI for the DartTooling Daemon (--dtd-uri) that has ' + 'been exposed to the front-end to support environments split across ' + 'machines such as a web-based editor.', + ) ..addFlag( argLaunchBrowser, help: @@ -257,7 +265,7 @@ class DevToolsServer { String? profileFilename, String? appSizeBase, String? appSizeTest, - String? dtdUri, + DtdInfo? dtdInfo, }) async { hostname ??= 'localhost'; @@ -285,20 +293,15 @@ class DevToolsServer { requestNotificationPermissions: enableNotifications, ); - String? dtdSecret; - if (dtdUri == null) { - final (:uri, :secret) = await startDtd( - machineMode: machineMode, - printDtdUri: printDtdUri, - ); - dtdUri = uri; - dtdSecret = secret; - } + dtdInfo ??= await startDtd( + machineMode: machineMode, + printDtdUri: printDtdUri, + ); handler ??= await defaultHandler( buildDir: customDevToolsPath!, clientManager: clientManager, - dtd: (uri: dtdUri, secret: dtdSecret), + dtd: dtdInfo, devtoolsExtensionsManager: ExtensionsManager(), ); @@ -493,19 +496,40 @@ class DevToolsServer { final bool verboseMode = args[argVerbose]; final String? hostname = args[argHost]; - String? dtdUri; + // A helper to print a message and usage information that can be used in + // a return statement. + Null printUsage(String message) { + print(message); + print(''); + _printUsage(buildArgParser(verbose: verbose)); + } + + Uri? dtdUri; if (args.wasParsed(argDtdUri)) { - dtdUri = args[argDtdUri]; + dtdUri = Uri.tryParse(args[argDtdUri]); + if (dtdUri == null || !dtdUri.hasScheme) { + return printUsage('--dtd-uri must be a valid URI'); + } + } + + Uri? dtdExposedUri; + if (args.wasParsed(argDtdExposedUri)) { + if (dtdUri == null) { + return printUsage( + '--dtd-exposed-uri can only be supplied with --dtd-uri'); + } + + dtdExposedUri = Uri.tryParse(args[argDtdExposedUri]); + if (dtdExposedUri == null || !dtdExposedUri.hasScheme) { + return printUsage('--dtd-exposed-uri must be a valid URI'); + } } final printDtdUri = args.wasParsed(argPrintDtd); if (help) { - print( + return printUsage( 'Dart DevTools version ${await DevToolsUtils.getVersion(customDevToolsPath ?? "")}'); - print(''); - _printUsage(buildArgParser(verbose: verbose)); - return null; } if (version) { @@ -566,7 +590,8 @@ class DevToolsServer { hostname: hostname, appSizeBase: appSizeBase, appSizeTest: appSizeTest, - dtdUri: dtdUri, + dtdInfo: + dtdUri != null ? DtdInfo(dtdUri, exposedUri: dtdExposedUri) : null, printDtdUri: printDtdUri, ); } diff --git a/pkg/dds/lib/src/dds_cli_entrypoint.dart b/pkg/dds/lib/src/dds_cli_entrypoint.dart index 665ac276cf5..74e2b0c87ae 100644 --- a/pkg/dds/lib/src/dds_cli_entrypoint.dart +++ b/pkg/dds/lib/src/dds_cli_entrypoint.dart @@ -137,7 +137,9 @@ ${argParser.usage} if (dds.devToolsUri != null) 'devToolsUri': dds.devToolsUri.toString(), if (dtdInfo != null) 'dtd': { - 'uri': dtdInfo.uri, + // For DDS-hosted DTD, there's only ever a local URI since there + // is no mechanism for exposing URIs. + 'uri': dtdInfo.localUri.toString(), }, })); } catch (e, st) { diff --git a/pkg/dds/lib/src/dds_impl.dart b/pkg/dds/lib/src/dds_impl.dart index 69dcfd44118..da3728f8157 100644 --- a/pkg/dds/lib/src/dds_impl.dart +++ b/pkg/dds/lib/src/dds_impl.dart @@ -10,7 +10,7 @@ import 'dart:math'; import 'dart:typed_data'; import 'package:devtools_shared/devtools_extensions_io.dart'; -import 'package:devtools_shared/devtools_shared.dart' show DTDConnectionInfo; +import 'package:devtools_shared/devtools_shared.dart' show DtdInfo; import 'package:json_rpc_2/json_rpc_2.dart' as json_rpc; import 'package:meta/meta.dart'; import 'package:shelf/shelf.dart'; @@ -379,10 +379,7 @@ class DartDevelopmentServiceImpl implements DartDevelopmentService { dds: this, buildDir: buildDir, notFoundHandler: notFoundHandler, - dtd: ( - uri: _hostedDartToolingDaemon?.uri, - secret: _hostedDartToolingDaemon?.secret - ), + dtd: _hostedDartToolingDaemon, devtoolsExtensionsManager: ExtensionsManager(), ) as FutureOr Function(Request); } @@ -512,9 +509,9 @@ class DartDevelopmentServiceImpl implements DartDevelopmentService { } @override - DTDConnectionInfo? get hostedDartToolingDaemon => _hostedDartToolingDaemon; + DtdInfo? get hostedDartToolingDaemon => _hostedDartToolingDaemon; - DTDConnectionInfo? _hostedDartToolingDaemon; + DtdInfo? _hostedDartToolingDaemon; final bool _ipv6; diff --git a/pkg/dds/lib/src/devtools/client.dart b/pkg/dds/lib/src/devtools/client.dart index 2a6a141e3ba..d458253a52f 100644 --- a/pkg/dds/lib/src/devtools/client.dart +++ b/pkg/dds/lib/src/devtools/client.dart @@ -6,7 +6,6 @@ import 'dart:async'; -import 'package:devtools_shared/devtools_server.dart'; import 'package:json_rpc_2/src/peer.dart' as json_rpc; import 'package:meta/meta.dart'; import 'package:sse/src/server/sse_handler.dart'; @@ -214,18 +213,6 @@ class DevToolsClient { _embedded = parameters['embedded'].asBool; }); - _devToolsPeer.registerMethod('getPreferenceValue', (parameters) { - final key = parameters['key'].asString; - final value = ServerApi.devToolsPreferences.properties[key]; - return value; - }); - - _devToolsPeer.registerMethod('setPreferenceValue', (parameters) { - final key = parameters['key'].asString; - final value = parameters['value'].value; - ServerApi.devToolsPreferences.properties[key] = value; - }); - _devToolsPeer.registerMethod('pingResponse', (parameters) { _nextPingResponse.complete(); _nextPingResponse = Completer(); diff --git a/pkg/dds/lib/src/devtools/dtd.dart b/pkg/dds/lib/src/devtools/dtd.dart index f70f4038d61..38d2c210faa 100644 --- a/pkg/dds/lib/src/devtools/dtd.dart +++ b/pkg/dds/lib/src/devtools/dtd.dart @@ -7,12 +7,12 @@ import 'dart:convert'; import 'dart:io'; import 'dart:isolate'; -import 'package:devtools_shared/devtools_shared.dart' show DTDConnectionInfo; +import 'package:devtools_shared/devtools_shared.dart' show DtdInfo; import 'package:path/path.dart' as path; import 'utils.dart'; -Future startDtd({ +Future startDtd({ required bool machineMode, required bool printDtdUri, }) async { @@ -24,8 +24,8 @@ Future startDtd({ 'dart_tooling_daemon.dart.snapshot', ); - final completer = Completer(); - void completeForError() => completer.complete((uri: null, secret: null)); + final completer = Completer(); + void completeForError() => completer.complete(null); final exitPort = ReceivePort() ..listen((_) { @@ -57,7 +57,7 @@ Future startDtd({ machineMode: machineMode, ); } - completer.complete((uri: uri, secret: secret)); + completer.complete(DtdInfo(Uri.parse(uri), secret: secret)); } } catch (_) { completeForError(); @@ -78,7 +78,7 @@ Future startDtd({ final result = await completer.future.timeout( const Duration(seconds: 5), - onTimeout: () => (uri: null, secret: null), + onTimeout: () => null, ); receivePort.close(); errorPort.close(); diff --git a/pkg/dds/lib/src/devtools/handler.dart b/pkg/dds/lib/src/devtools/handler.dart index 7f3cdc13cd9..ef95f811c4c 100644 --- a/pkg/dds/lib/src/devtools/handler.dart +++ b/pkg/dds/lib/src/devtools/handler.dart @@ -45,7 +45,7 @@ FutureOr defaultHandler({ required String buildDir, ClientManager? clientManager, Handler? notFoundHandler, - DTDConnectionInfo? dtd, + DtdInfo? dtd, required ExtensionsManager devtoolsExtensionsManager, }) { // When served through DDS, the app root is /devtools. diff --git a/pkg/dds/lib/src/devtools/machine_mode_command_handler.dart b/pkg/dds/lib/src/devtools/machine_mode_command_handler.dart index a2e26b524bb..1be7fd8ba77 100644 --- a/pkg/dds/lib/src/devtools/machine_mode_command_handler.dart +++ b/pkg/dds/lib/src/devtools/machine_mode_command_handler.dart @@ -272,7 +272,7 @@ class MachineModeCommandHandler { _devToolsUsage = null; } break; - case apiSetActiveSurvey: + case SurveyApi.setActiveSurvey: _devToolsUsage!.activeSurvey = value; DevToolsUtils.printOutput( 'DevTools Survey', @@ -286,7 +286,7 @@ class MachineModeCommandHandler { machineMode: machineMode, ); break; - case apiGetSurveyActionTaken: + case SurveyApi.getSurveyActionTaken: DevToolsUtils.printOutput( 'DevTools Survey', { @@ -299,7 +299,7 @@ class MachineModeCommandHandler { machineMode: machineMode, ); break; - case apiSetSurveyActionTaken: + case SurveyApi.setSurveyActionTaken: _devToolsUsage!.surveyActionTaken = jsonDecode(value); DevToolsUtils.printOutput( 'DevTools Survey', @@ -313,7 +313,7 @@ class MachineModeCommandHandler { machineMode: machineMode, ); break; - case apiGetSurveyShownCount: + case SurveyApi.getSurveyShownCount: DevToolsUtils.printOutput( 'DevTools Survey', { @@ -326,7 +326,7 @@ class MachineModeCommandHandler { machineMode: machineMode, ); break; - case apiIncrementSurveyShownCount: + case SurveyApi.incrementSurveyShownCount: _devToolsUsage!.incrementSurveyShownCount(); DevToolsUtils.printOutput( 'DevTools Survey', diff --git a/pkg/dds/pubspec.yaml b/pkg/dds/pubspec.yaml index 6e994f146f9..b05ec0b1e47 100644 --- a/pkg/dds/pubspec.yaml +++ b/pkg/dds/pubspec.yaml @@ -1,5 +1,5 @@ name: dds -version: 4.2.7 +version: 5.0.0 description: >- A library used to spawn the Dart Developer Service, used to communicate with a Dart VM Service instance. @@ -16,7 +16,7 @@ dependencies: dds_service_extensions: ^2.0.0 dap: ^1.3.0 extension_discovery: ^2.0.0 - devtools_shared: ^10.0.2 + devtools_shared: ^11.0.0 http_multi_server: ^3.0.0 json_rpc_2: ^3.0.0 meta: ^1.1.8 diff --git a/pkg/dds/test/devtools_server/dtd_test.dart b/pkg/dds/test/devtools_server/dtd_test.dart index e097f632b86..95db57467ae 100644 --- a/pkg/dds/test/devtools_server/dtd_test.dart +++ b/pkg/dds/test/devtools_server/dtd_test.dart @@ -8,20 +8,25 @@ import 'package:test/test.dart'; import 'utils/server_driver.dart'; void main() { + const dtdUriSwitch = '--${DevToolsServer.argDtdUri}'; + const dtdExposedUriSwitch = '--${DevToolsServer.argDtdExposedUri}'; + 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'], + additionalArgs: ['$dtdUriSwitch=ws://localhost:123/'], ); try { + // Ensure the event does not arrive within some reasonable amount of + // time. final dtdStartedEvent = await server.stdout .firstWhere( (map) => map!['event'] == 'server.dtdStarted', orElse: () => null, ) .timeout( - const Duration(seconds: 3), + Duration(seconds: 3), onTimeout: () => null, ); expect(dtdStartedEvent, isNull); @@ -42,5 +47,45 @@ void main() { server.kill(); } }); + + test('rejects invalid URIs for --dtd-uri', () async { + final server = await DevToolsServerDriver.create( + additionalArgs: ['$dtdUriSwitch=some_uri'], + ); + try { + final firstLine = await server.stdoutRaw.first; + expect(firstLine, '$dtdUriSwitch must be a valid URI'); + } finally { + server.kill(); + } + }); + + test('rejects invalid URIs for --dtd-exposed-uri', () async { + final server = await DevToolsServerDriver.create( + additionalArgs: [ + '$dtdUriSwitch=ws://localhost:123/', + '$dtdExposedUriSwitch=some_uri' + ], + ); + try { + final firstLine = await server.stdoutRaw.first; + expect(firstLine, '$dtdExposedUriSwitch must be a valid URI'); + } finally { + server.kill(); + } + }); + + test('rejects --dtd-exposed-uri without --dtd-uri', () async { + final server = await DevToolsServerDriver.create( + additionalArgs: ['$dtdExposedUriSwitch=some_uri'], + ); + try { + final firstLine = await server.stdoutRaw.first; + expect(firstLine, + '$dtdExposedUriSwitch can only be supplied with $dtdUriSwitch'); + } finally { + server.kill(); + } + }); }); } diff --git a/pkg/dds/test/devtools_server/utils/server_driver.dart b/pkg/dds/test/devtools_server/utils/server_driver.dart index be1e39e437a..a078e5187b9 100644 --- a/pkg/dds/test/devtools_server/utils/server_driver.dart +++ b/pkg/dds/test/devtools_server/utils/server_driver.dart @@ -18,38 +18,51 @@ class DevToolsServerDriver { this._stdin, Stream _stdout, Stream _stderr, - ) : stdout = _convertToMapStream(_stdout), - stderr = _stderr.map((line) { + ) : stderr = _stderr.map((line) { _trace('<== STDERR $line'); return line; - }); + }) { + // Many tests verify JSON output in stdout but some verify usage output + // to stdout for invalid args, so split the process stdout into two + // streams, one as JSON and one raw strings. + var stdoutRawController = StreamController(); + stdoutRaw = stdoutRawController.stream; + var stdoutJsonController = StreamController?>(); + stdout = stdoutJsonController.stream; + _stdout.listen((line) { + _trace('<== $line'); + + // Send to raw stdout stream. + stdoutRawController.add(line); + + // If the output is JSON, also send a copy to stdoutJson. + try { + var json = jsonDecode(line) as Map; + stdoutJsonController.add(json); + } catch (_) {} + }, onError: (e, s) { + stdoutRawController.addError(e, s); + stdoutJsonController.addError(e, s); + }, onDone: () { + stdoutRawController.close(); + stdoutJsonController.close(); + }); + } final Process _process; - final Stream?> stdout; + late final Stream?> stdout; + late final Stream stdoutRaw; final Stream stderr; final StringSink _stdin; + Future get exitCode => _process.exitCode; + void write(Map request) { final line = jsonEncode(request); _trace('==> $line'); _stdin.writeln(line); } - static Stream?> _convertToMapStream( - Stream stream, - ) { - return stream.map((line) { - _trace('<== $line'); - return line; - }).map((line) { - try { - return jsonDecode(line) as Map; - } catch (e) { - return null; - } - }).where((item) => item != null); - } - static void _trace(String message) { if (verbose) { print(message);