Inspector navigation doesn't work in noDebug mode with the SDK DAPs

[DanTup]

It might not be obvious to users (especially as it changed with the new adapters) that they should run using Debug mode to get some functionality from the inspector (navigating to files as they select widgets).

#4876 is for deciding (and documenting) the differences between Run/Debug (which should probably be based on specific features rather than arbitrarily what comes over the VM Service) but in the short-term we should do something to avoid confusion from users that don't realise navigation only works in Debug mode.

As a minimum, we should warn users opening the inspector that they should use Debug mode, but we should also evaluate other options (such as a hotfix that connects the VM Service for noDebug mode, or forcing noDebug mode off in the extension and intercepting breakpoints/pause-on-exception requests).

[DanTup]

Some options:

1. Handle ToolEvent inside the extension (similar to how DTD will probably work)
   [WIP] Handle ToolEvents in the editor for noDebug mode  #4882
   This might not work for google3 where the URIs need mapping (unless we also handle that mapping in the extension, but it makes the fix more complicated)
2. Warn users when the open inspector if they're in noDebug mode that they should run in Debug mode
   [WIP] Prompt to re-run in debug mode when using the inspector #4881
   Has a button to re-start the session for them
3. Force all runs through debug mode
   Force Flutter non-test sessions into debug mode to force SDK DAP to connect VM Service #4880
   This may make Run slightly slower because it'll trigger pause-on-start which it didn't before
   VS Code already doesn't send breakpoints for noDebug so by intercepting launchRequest on the way to the DAP, we get disabled breakpoints/exceptions for free
   debugger() will still break
4. Update the flutter DAP to always connect the debugger
   This would require an SDK hotfix that is potentially risky (and if it introduced any unintended consequences would be hardest to re-patch)
   This might be the right medium-turn (non-hotfix) solution for the next SDK release

[DanTup]

@jacob314 @jwren I've tested some possible fixes that don't require SDK releases/hot-fixes. I'm interested in your opinions. The details of each fix is above. I haven't implemented any SDK changes yet because I think the changes there will be too risky for a hotfix so will have to wait for the next SDK release.

At first I thought (1) would be a good fix (and would be implemented in the extension where we'll ultimately connect to DTD). However, after implementing it I realised it was missing URI mappings (required for google3) and although we could add that, it'll make the solution more complicated.

My feeling is that we should ship (3) which feels like the simplest/least risky change. We rewrite the noDebug flag on the way to the debug adapter, which results in putting it into debug mode. However because Code now automatically skips breakpoints/exception settings for noDebug mode we don't even need to intercept the breakpoint requests as I'd expected. The only slightly odd thing (but this applied to the legacy DAP in noDebug mode for Flutter) is that debugger() will still break.

While (2) works, it also may teach users that they need to use Debug mode for this functionality, and if we decide that's not the long-term behaviour we want, that's a little odd (I've opened #4876 about determining exactly what we want the difference between Run/Debug to be).

[jwren]

@DanTup Thanks for articulating the current situation, yes option (3) looks to be the most pragmatic.

[DanTup]

Great - I'll add some tests tomorrow with a simulated navigate ToolEvent and ensure we handle it in both debug + noDebug mode, and then I'll push this out in the pre-release version of the extension. I'd like to leave it there at least a few days / a week before doing a stable hotfix just to ensure there aren't any unexpected consequences I didn't find in my own testing.

I'll also think a bit more about #4876 so we can try plan a "proper" fix (for example one that won't cause noDebug sessions to use --start-pause which might slow down startup a little).

[DanTup]

I've merged the fix for this and published a pre-release version. If nothing comes up for the rest of the week, I'll push a stable patch out for this.

[jacob314]

Thank you! Making sure inspector navigation works outside of debug mode is a P0. We should think about how we can test it better end to end. For example, maybe IDEs could start ACKing when they accept a code navigation request so DevTools can at least display a warning message and track analytics. E.g. "something went wrong and your IDE wasn't able to navigate to the location... go here for tips on setting up your IDE..."
Fyi @kenzieschmoll

[DanTup]

The new tests I added here will post a navigation event:

Dart-Code/src/test/test_projects/flutter_hello_world/lib/navigate_to.dart

Lines 14 to 23 in da36c47

	postEvent(
	'navigate',
	{
	'uri': 'package:flutter_hello_world/navigate_to.dart',
	'line': 1,
	'column': 2,
	'source': 'flutter.inspector',
	},
	stream: 'ToolEvent',
	);

And the tests will verify that file is actually opened:

Dart-Code/src/test/flutter_debug/flutter_run.test.ts

Lines 270 to 274 in da36c47

	await waitForResult(
	() => vs.window.activeTextEditor?.document.uri.toString() === flutterHelloWorldNavigateToFile.toString(),
	"Did not navigate to expected file",
	60000,
	);

Wrapping up the postEvent into a function that is called both by the inspector and this test app would improve things further (because the current test relies on that code matching the event the inspector actually sends).

We can't respond directly to this event (because it's a stream of events and not a request, and there could in theory be multiple or zero connected IDEs) so I'm not sure how worthwhile it is changing how this works if we have tests at both sides (verifying Flutter sends the event, and verifying IDEs given an event perform the navigation).

[DanTup]

This issue is resolved in the stable v3.78.2 of the Dart extension.