--enable-vm-service flag behaviors are inconsistent between debugger and cli

[pattobrien]

CC @DanTup (re conversation: https://discord.com/channels/608014603317936148/1282837915847757964)

Given the following Dart script:

import 'dart:developer';

Future<void> main(List<String> args) async {
  print('Starting server code...');

  final vmServiceInfo = await Service.getInfo();
  final wsUri = vmServiceInfo.serverWebSocketUri!;
  print('VM Service accessible at: $wsUri');
}

... and the following launch config and cli commands:

launch.json (debugger):

    {
      "name": "launch.json example",
      "request": "launch",
      "type": "dart",
      "program": "bin/main.dart",
      "toolArgs": [
        "--observe",
        "--enable-vm-service=6968",
        "--disable-service-auth-codes"
      ],
    },

cli command:

dart run --observe --enable-vm-service=6968 --disable-service-auth-codes bin/main.dart

The output / behavior is different.

debugger output (incorrect expected behavior)

Connecting to VM Service at ws://127.0.0.1:51752/32Cv_oigsEk=/ws
Connected to the VM Service.
Starting server code...
VM Service accessible at: ws://127.0.0.1:51752/32Cv_oigsEk=/ws 

cli output (correct expected behavior)

❯ dart run --observe --enable-vm-service=6968 --disable-service-auth-codes bin/main.dart
Starting server code...
VM Service accessible at: ws://127.0.0.1:51780/ws
The Dart VM service is listening on http://127.0.0.1:6968/
The Dart DevTools debugger and profiler is available at: http://127.0.0.1:6968/devtools/?uri=ws://127.0.0.1:6968/ws

[dart-github-bot]

Summary: The --enable-vm-service flag behaves inconsistently between the debugger and the command line. When used in the debugger, the VM service URI is incorrect, while the command line produces the expected output with the correct URI.

[DanTup]

@bkonyi my guess is that this is because the flag is passed to the VM by the debug adapter and the original VM Service does use that port, but then the debug adapter starts DDS which binds a new port, and we take the URI from that?

sdk/pkg/dds/lib/src/dap/adapters/dart.dart

Line 792 in a6a2c57

Future<DartDevelopmentServiceLauncher> startDds(Uri uri) {

What's the best way to handle this? Can we return the normal VM Service URI (I can't remember if DDS needs to proxy all the traffic or is just registering new services etc.)? If not, I wonder if we'd need to either parse the VM args in DAP (which doesn't seem ideal to duplicate here) and use the port for DDS instead of passing it to the VM?

[bkonyi]

@bkonyi my guess is that this is because the flag is passed to the VM by the debug adapter and the original VM Service does use that port, but then the debug adapter starts DDS which binds a new port, and we take the URI from that?

It seems like that's the case.

What's the best way to handle this? Can we return the normal VM Service URI (I can't remember if DDS needs to proxy all the traffic or is just registering new services etc.)? If not, I wonder if we'd need to either parse the VM args in DAP (which doesn't seem ideal to duplicate here) and use the port for DDS instead of passing it to the VM?

All traffic should go through DDS as any attempt to connect directly to the VM service while DDS is connected will either fail or get a redirect to DDS (depending on whether or not the websocket implementation allows for following redirects).

While there's definitely a way to handle this properly, I'm wondering why the Dart DAP adapter even needs to start DDS itself? I honestly can't remember why we decided to do this, but since we don't rely on dart run to launch DDS anymore (the VM service handles this now) maybe we can avoid the headache and just let the VM handle this?

[DanTup]

While there's definitely a way to handle this properly, I'm wondering why the Dart DAP adapter even needs to start DDS itself?

I think we wanted to avoid an additional process and thought we might one day have DDS + DAP closer together (to avoid additional layers). I don't know if that still makes any sense though, and Flutter always has its own DDS and is hard-coded to set false to the DAP-provided DDS.

If you think it makes no sense to do here, I will remove it (keeping constructor the flag as a no-op initially, to simplify rolling the update into Flutter) and do some testing.

[DanTup]

I've a change open to remove DDS from DAP here:

https://dart-review.googlesource.com/c/sdk/+/386500

With that change, the flags noted above work as expected:

(Although FWIW using --observe is redundant because the debug adapter already enables the debugger, but --observe also leads to "has no debugger attached and is paused" messages that probably aren't useful here).