Implement new methods from vm_service 7.4.0: `lookupPackageUris`, `lookupResolvedPackageUris`

[kenzieschmoll]

Overrides added in #1445. Method definitions here: https://github.com/dart-lang/sdk/blob/main/pkg/vm_service/lib/src/vm_service.dart#L927-L956
@elliette @annagrin

Original SDK issue for providing new API: dart-lang/sdk#46631

[annagrin]

@bkonyi @DanTup I have a few of questions about this issue and current implementation:

• how is the result of the new APIs used when debugging for web platform?
• we frequently run the vm service on a different machine from the UI (devtools, not sure if it is possible with the VSCode) - what is the function of absolute paths in that case?
• From the security standpoint - as we are possibly exposing local paths to an external machine, is there anything that prevents unauthorized connection to the VM?
• How are the resolved sdk uris used? I am not sure where to find paths for dart: libraries, what can help me make the translation
  dart:io -> org-dartlang-sdk:///sdk/lib/io/io.dart?

I chatted to @bkonyi and we decided to just return the package and org-dartlang-app uris as resolved uris for now, as they are used in setBreakpointWithScriptUri as well. @DanTup will that break anything for dart-code?

[DanTup]

how is the result of the new APIs used when debugging for web platform?

It should be used the same regardless of the platform. The goal is to convert the URIs back to a file path on the users local machine so that the editor can open that file (for example when you step into it). Previously we would download the source code from the VM for any library we don't have file paths for, but that lead to some confusing experiences (such as having multiple copies of "the same" library open in the editor, where some are downloaded from the VM and some are from the users local disk).

we frequently run the vm service on a different machine from the UI (devtools, not sure if it is possible with the VSCode) - what is the function of absolute paths in that case?

I'm not too familiar with how things work in DevTools in that case, but in VS Code we'd expect the absolute paths to always be the same as the editor host machine* so the editor can open up that file from disk. These would be the same paths we send when adding breakpoints (since they're the only paths the editor knows about).

* note: in some situations like VS Code remoting, the "host machine" is not actually the one where VS Code is running, it's the one where the VS Code Extension Host is, but that's probably not that important here.

From the security standpoint - as we are possibly exposing local paths to an external machine, is there anything that prevents unauthorized connection to the VM?

My understanding (at least externally) is that this is protected by the auth tokens required to connect to the VM Service (since there are more issues than just leaking paths if someone connects to a VM service - they can evaluate arbitrary Dart expressions that will execute on the machine running the VM).

I chatted to @bkonyi and we decided to just return the package and org-dartlang-app uris as resolved uris for now, as they are used in setBreakpointWithScriptUri as well. @DanTup will that break anything for dart-code?

I'm not sure I understand. Do you mean the API will return the same package: URIs back that it was given and not their file: URIs (or org-dartlang-app for things outside of lib)? This sounds like it will cause issues for the new DAP, as it assumes we will get back the file URIs (my understanding was that was the purpose of this API, so that editors don't need to understand these implementation details).

It occurs to me that I landed changes to use this API in the SDK DAP, but if Web doesn't have an implementation of this, the requests will probably fail right now - @bkonyi is this accurate - do I need to handle this API not being supported in DAP? (note: we don't support Dart Web through the SDK DAP yet, but presumably the Flutter DAP when launched for a web device would fail?).

[bkonyi]

I'm not sure I understand. Do you mean the API will return the same package: URIs back that it was given and not their file: URIs (or org-dartlang-app for things outside of lib)?

Yes, that's the case. I'm not sure what the right thing to do for DWDS is here, but ideally we'll return the absolute file paths. File system paths are already reported via the service in other contexts, I don't think there's any issue doing the same here.

[annagrin]

Yes, that's the case. I'm not sure what the right thing to do for DWDS is here, but ideally we'll return the absolute file paths. File system paths are already reported via the service in other contexts, I don't think there's any issue doing the same here.

@bkonyi I think this means we are coming back to absolute paths solution here, in fact I didn't create a new PR yet so I can just submit the one mentioned in this thread. Let me know if you are ok with that.

One problem I still have there is how to resolve dart: libraries, but I can address it later.

These would be the same paths we send when adding breakpoints

@DanTup DWDS does not actually care if the paths used for breakpoints are resolved or not - it knows how to convert them back and forth internally, so implementing the new API in either way (resolving or not resolving the paths) will not affect breakpoint behavior.

It occurs to me that I landed changes to use this API in the SDK DAP, but if Web doesn't have an implementation of this, the requests will probably fail right now - @bkonyi is this accurate - do I need to handle this API not being supported in DAP? (note: we don't support Dart Web through the SDK DAP yet, but presumably the Flutter DAP when launched for a web device would fail?).

@DanTup Right now calling the new API on web throws a "not supported" exception. I haven't seen anything broken yet though, but maybe I need to update my DAP. Will try and report back.

[bkonyi]

Yeah, I think we should be okay using the absolute paths, so let's move forward with that to be consistent :-)

[DanTup]

@DanTup Right now calling the new API on web throws a "not supported" exception. I haven't seen anything broken yet though, but maybe I need to update my DAP. Will try and report back.

It's only the SDK DAP that uses this API so far, and VS Code is not using that currently (so it's not critical if it's broken, but does need fixing because we need to move VS Code over soon). I've made a note to do some testing, and should probably update the base DAP classes to handle this failure gracefully.

That said, I assumed this was available everywhere (forgetting about Web). Without it, the Web experience would regress (since we previously parsed the package_config file to do this, and that code was removed), so if we can't make DWDS work the same we might need to restore that.

Yeah, I think we should be okay using the absolute paths

I'm not sure if I've misunderstood something, as I thought that was the purpose of this API (to take a dart:/package: URI and resolve it to a file:// URI - or in the case of the SDK, org-dartlang-sdk:, and vice versa). If the web wasn't to return absolute paths, what would it be returning when given dart:io/package: URIs?

[annagrin]

Thanks, will do!

As for DAP for flutter not working for web, I tried running and stepping through the hello_world example, could not see anything failing - @DanTup what operations are the new API used for? I have Dart Extension v3.29.0 installed at the moment.

[DanTup]

@annagrin it's only the SDK DAP that uses this API. VS Code is not using that currently so it's not critical if it's broken, but does need fixing because we need to move VS Code over soon. Right now there's not an easy way to use the compiled SDK DAPs from VS Code, but you can run them from source with some hidden VS Code settings:

"dart.experimentalDartDapPath": "/Users/danny/Dev/Google/dart-sdk/sdk/pkg/dartdev/bin/dartdev.dart"
"dart.experimentalFlutterDapPath": "/Users/danny/Dev/Google/flutter/packages/flutter_tools/bin/flutter_tools.dart"

This will cause VS Code to spawn those scripts instead of the built-in DAP. In the next VS Code release I will likely add specific settings for enabling the SDK DAPs (which will just run dart debug_adapter or flutter debug_adapter), although I won't start moving users over to it until a version of the SDK(s) where there are no gaps from the current DAP (I still have a few things to implement).

[annagrin]

@DanTup got it, thanks! I found a way to resolve SDK uris, adding them to my PR. Will update when everything works.