Dart-Code / Dart-Code

Dart and Flutter support for VS Code
https://dartcode.org/
MIT License
1.49k stars 304 forks source link

SDK DAPs fail to connect to web apps on latest code because subscribing to the ToolEvent stream fails #4414

Closed DanTup closed 1 year ago

DanTup commented 1 year ago

See https://github.com/Dart-Code/Dart-Code/issues/4413#issuecomment-1450339766.

DanTup commented 1 year ago

Copied from #4413:

In DAP, we use the DDS version being >= 1.4 to decide if custom streams are supported:

https://github.com/dart-lang/sdk/blob/7ca5ad46ce02ff00908f1b9d6737844d5aeeca41/pkg/dds/lib/src/dap/adapters/dart.dart#L2575-L2577

However in this web case, this check passes but DWDS does not support this:

https://github.com/dart-lang/webdev/blob/1e7f9b7e55c4ab55db15275e8834a1e50e2d2832/dwds/lib/src/services/chrome_proxy_service.dart#L707-L712

@bkonyi @annagrin what's the best thing for me to do here? It seems like using the version number isn't reliable. Is there a better way to know when this is supported, or should I just handle the error?

(related: is there a plan to extend this custom stream support to DWDS?)

DanTup commented 1 year ago

@CoderDake tracked this down. DDS tries to listen to the stream and catches the error to know when something is not a built-in stream:

https://github.com/dart-lang/sdk/blob/3643e771c487d64c2a9398d3ca62c443138e5e91/pkg/dds/lib/src/stream_manager.dart#L226-L235

It only catches kInvalidParams which is what the VM Service throws. However DWDS is throwing kMethodNotFound:

https://github.com/dart-lang/webdev/blob/1e7f9b7e55c4ab55db15275e8834a1e50e2d2832/dwds/lib/src/services/chrome_proxy_service.dart#L707-L712

This could be fixed at either end. I've tested both with DDS also handling kMethodNotFound and also with DWDS instead throwing kInvalidParams. Probably the latter is the best change (because it makes DWDS consistent with the VM). Either one will need a release of the Pub package and rolling into Flutter.

elliette commented 1 year ago

Thanks! Dan has opened a fix on DWDS' side. @DanTup - will we need to do a hotfix for DWDS, or are you okay with waiting to re-enable the SDK DAPs? (https://github.com/Dart-Code/Dart-Code)

DanTup commented 1 year ago

@elliette no hotfixes needed. The code causing problems here (the ToolEvent stream) wasn't merged until after the current stable, and because of the missing ToolEvent stream (and some other issues), I don't plan to enable SDK DAPs by default for current stable.

However, I would like to re-enable the SDK DAPs on Flutter master (the more use they get before the next stable branch, the more likely we'll find any other issues). So having a DWDS release that can be rolled into Flutter once this lands would definitely help (but it's not urgent). I'll re-enable them based on the version number that includes the fixed DWDS.

DanTup commented 1 year ago

This is all resolved. DWDS was released and rolled into Flutter. Enabling the new SDK DAPs and running on web, I can use the embedded inspector and it navigates my editor as expected. The DAP logs show the new toolEvent events:

{
    "seq": 104,
    "type": "event",
    "body": {
        "kind": "navigate",
        "data": {
            "fileUri": "file:///Users/danny/Desktop/dart_samples/flutter_counter/lib/main.dart",
            "line": 6,
            "column": 16,
            "source": "flutter.inspector"
        }
    },
    "event": "dart.toolEvent"
}