Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
DanTup opened this issue Mar 1, 2023 · 5 comments · Fixed by dart-lang/webdev#2011
Labels
in debugger Relates to the debug adapter or process of launching a debug session in flutter Relates to running Flutter apps in web Relates to running Dart or Flutter web apps is bug
Milestone

Comments

@DanTup
Copy link
Member

DanTup commented Mar 1, 2023

See #4413 (comment).

@DanTup DanTup added is bug in web Relates to running Dart or Flutter web apps in flutter Relates to running Flutter apps in debugger Relates to the debug adapter or process of launching a debug session labels Mar 1, 2023
@DanTup DanTup added this to the v3.62.0 milestone Mar 1, 2023
@DanTup
Copy link
Member Author

DanTup commented Mar 1, 2023

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
Copy link
Member Author

DanTup commented Mar 2, 2023

@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
Copy link

elliette commented Mar 2, 2023

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
Copy link
Member Author

DanTup commented Mar 2, 2023

@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
Copy link
Member Author

DanTup commented Mar 23, 2023

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"
}

@DanTup DanTup closed this as completed Mar 23, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in debugger Relates to the debug adapter or process of launching a debug session in flutter Relates to running Flutter apps in web Relates to running Dart or Flutter web apps is bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants