-
Notifications
You must be signed in to change notification settings - Fork 307
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
Breakpoints in Dart SDK code are not hit #3551
Comments
@bkonyi do you know if this is expected to work? I was certain it did (except for Flutter), but I can repro this and it doesn't work. When you Go-to-Definition you end up in a file like inside the SDK folder which results in us sending a breakpoint like this:
The breakpoint is never resolved/never hit when printing though. |
Comments in flutter/flutter#27189 are very informative and still relevant. I tried the following dirty hack in if (uri === "dart:core/print.dart") {
uri = "org-dartlang-sdk:///sdk/lib/core/print.dart";
} With this, I'm able to add a breakpoint in It seems there are two distinct issues :
|
I'm not certain this is the correct place to fix this versus the VM. When we step into
It seems sensible that
Handling this in the VM would fix it for all editors rather than just VS Code (I also don't think VS Code should need to understand the internals of what |
I fully agree with you @DanTup : when I wrote "URIs should be converted before calling VM Service addBreakpointWithScriptUri RPC", I described the issue not what should be done to fix it 😄 I too think that it would be better to fix this issue at the VM level, otherwhise all debugging clients (DartCode, IntelliJ Plugin, DevTools, ...) will have to provide their own workaround. I will try to dig deeper to see what can be done with other VM Service RPC ( |
Yeah, this should almost certainly be fixed in the VM. Dealing with breakpoint resolution within SDK code is a pain since the VM doesn't always reference the code on disk. At the very least, for the standalone VM, I'm 99% sure we always load SDK code from a pre-built snapshot so breakpoints need to be set using the |
@bkonyi Yes, breakpoint resolution is a pain 😅 @DanTup Found some code in flutter/intellij dealing with BreakpointsUri : https://github.com/flutter/flutter-intellij/blob/4bd532c5dd021e88a6ad664e8ff8f929475a7796/src/io/flutter/run/FlutterPositionMapper.java#L183 |
Ok, I will file some bugs in the SDK later with clear examples of the issue and how to repro.
Yeah, though it's also a pain in the IDE where (while not debugging) we only have file paths from the editor and no good way to map them (at least not without edge cases :-)). For one of the cases above though, there's no file path involved - the VM gives us
We used to do the same in VS Code, but it brings some complications of mapping between a single breakpoint in VS Code and multiple in the VM. Most of the bugs that required doing this (which were breakpoints only working if you sent either file:/// or package: depending on how the file was loaded in the VM) have been fixed by the VM so my preference is to try to continue down that path. Any logic that goes in editors needs duplicating in multiple places, which raises the barrier to a new editor adding debugging support (although, hopefully an SDK-provided DAP will improve that a little). |
After several days of digging (well worth it, as I learned many things: building a custom Dart SDK, building a custom Dart Code, playing with Dart VM C++ code, and more), I found a very simple hack 🥁 : When calling Found this from this VM Code : https://github.com/dart-lang/sdk/blob/main/runtime/vm/object.cc#L12842 That works nicely for Flutter app. But for Dart apps, it works partially (works only when the breakpoint is added after 'stepping in' and it creates a bug : can't find source in editor when selecting the breakpoint). It's just hack in DartCode, but maybe it could be included, considering that :
@DanTup, I will create a PR so you can test this easily, but I don't have a strong opinion on releasing such a hack (maybe behind a flag ?). I will also pursue investigation on trying to fix the VMService code. |
Thanks for the digging! My only concern with shipping this workaround here, is that if it's fixed in the VM in a way that stops There's work in progress to move the debug adapters to the SDK which would actually avoid that issue, since the debug adapter code is coupled to the SDK/VM version so it might be less weird there. I am curious why it works this way though. If it's just an accidental bug and nobody else is doing this, then perhaps fixing it isn't going to regress anything. I'll file an SDK bug about this shortly though with some details, and wait to hear @bkonyi's opinion before deciding what to do (although if we do ship a workaround, I think it might be better to do just in the SDK DAP, which I hope we're not too far off completing and migrating Dart-Code too). |
Hi @DanTup , I agree with your concern with shipping this. I think that people really interested by this will be able to use my PR to build a custom DartCode. |
@bleroux is this now resolved by dart-lang/sdk#47204 or are there still issues? |
Hi @DanTup, with this PR, adding a Breakpoint with a 'dart:' URI is fixed. In Dart Code context, it means that when the VM Service is live, breakpoints could be added after stepping into SDK Code (breakpoints added with the 'dart:' prefix). For flutter app, the breakpoint will be correctly persisted and it will be hit in current and future debug session. Issues remaining (visible with Dart Code, maybe related to VM or Dart Code) :
I will investigate 1 & 2, maybe you can have a look to 3 & 4. |
I think 3&4 are are slightly related to microsoft/vscode#131502. However fixing that probably means those breakpoints will disappear, which also feels akward. I'll note that on the issue and see whether maybe there's a better way to fix this. |
Actually, I think I might be able to fix 4 by returning new I don't think 3 is generally possible to fix (any source code that was fetched from the VM in a debug session we have no way fo fetching when the session is gone). However, if we could map the |
Hi @DanTup, after looking at the VM Code, I think that 2. can't be fixed at the VM level. My understandings are that, for Flutter apps, snapshots files are created ('.dill' & 'snapshot' files, more details here https://mrale.ph/dartvm/). Those files don't store orginal file paths for "dart:async": "async/async.dart"
"dart:collection": "collection/collection.dart"
"dart:convert": "convert/convert.dart"
"dart:core": "core/core.dart" If it can't be fixed at the VM Level, I see some options :
|
Yep. Although I don't entirely understand the mechanics of this (why It would be best if any fixes didn't involve workarounds inside VS Code, because they'd need to be reproduced in each editor (or at least DAP), and it feels like these are implementation details that could change over time, and things may break. It feels like between the analysis server and the VM, there should be some way for us to get a path from Go-to-Definition that just works at runtime (and if we don't have the correct source code at development time that will run at runtime, then perhaps Go-to-Definition shouldn't be going anywhere). I don't know enough about how the VM works to understand how complex this is though. |
My understandings for your questions :
I could have a look to the analysis server. It would be helpful, if you can help me locate where Dart Code calls the analysis server when 'Go-to-Definition' is selected. |
Something I'm not clear on from the above - is which version of the code is the one that will actually run at runtime on the device? What's in the
Go-to-Definition is powered by the analysis servers navigation classes (https://github.com/dart-lang/sdk/blob/master/pkg/analyzer_plugin/lib/utilities/navigation/navigation_dart.dart), although I think it's just using resolution info from general analysis, so the reason it's getting That said, even if the analysis server could easily be changed to not use sky_engine, it's not going to be able to return |
Hi @DanTup , I have yet to understand Dart and Flutter internals, but here are my understandings for your questions : "which version of the code is the one that will actually run at runtime on the device?" . I think a comparison to Java or Typescript could be made :
For instance, in Dart Code we can select the 'print' function and 'go-to-definition'. If we edit its source code and launch the app, the changes are not loaded. We have to rebuild snapshots to see the changes in action. Second question "What's in the sky_engine package, or what's in the Dart SDK's lib folder?" :
I had a first look at analysis server code, it contains code to load SDK libs and find 'dart:' mapping in embedder.yaml files. I will try to find if there is a way to get the mapping (between files and 'dart:' libs) that analysis server build internally. |
@DanTup |
Interesting! I tried calling that API with
Although that's a nice improvement, I don't think it's the ideal solution (even ignoring that it's Flutter-only) because it leaves some problems:
I still think the ideal solution would be that when debugging and you break inside a) that's feasible (does the VM have enough info? if not, is it realistic to give it?) I'll see if I can get some time with people that know this better than me for a discussion and see if we can come up with something. This issue has bugged me forever and it'd be good to have a plan (or a concrete reason why it can't work). |
If we want to go further, we can try to split this on several step. First one would be to fully understand how and where client code would be impacted. Dart-Code/src/debug/dart_debug_impl.ts Lines 1085 to 1091 in e037571
So that the editor will open source file on disk ? And breakpoint would be registered with a source path instead of a source reference ?
If yes, another step would be that |
Sorry for the delay!
FWIW - all of the code in https://github.com/dart-lang/sdk/tree/main/pkg/dds/tool/dap#debug-adapter-protocol That means any solution should be implementable there instead, although its implementation is fairly similar, and the equiv of the code above is here: If we can reliably convert the non-file URIs into file URIs here (and as you say, back in That said, I know @bkonyi was looking at adding VM Service to map URIs (intended for package-to-file/file-to-package). If that could be extended to handle |
I'm expecting dart-lang/sdk@08db718 to handle this, but it'll only work when we've moved over to the SDK DAPs so I'll keep this issue open to verify once that happens. I hope to add an experimental flag to opt in to this after the next SDK release, and then start moving over by default based on feedback from those using it. |
This issue has been fixed for some upcoming releases. I tested it today using the current beta version of the Dart SDK (2.18.0), the pre-release v3.47.20220808 version of the Dart extension, and with the When using all of these, the Dart SDK libraryes (such as Some of these pieces are not released in stable releases yet, and the SDK DAPs will be a gradual roll out over the coming releases (although you can opt-in explicitly with the Please file issues if you still have issues with this once you have confirmed you're on:
|
I noticed today that this fix does not entirely work when running Flutter apps - they return a different I've filed #4128 to track this, and dart-lang/sdk#49863 to find out if it's a bug, and if not exactly what the mapping rules are. |
I tried to put breakpoints in Dart SDK code but those breakpoints are ignored.
To reproduce this I setup an environment with :
Dart SDK version: 2.15.0-68.0.dev (dev) (Wed Sep 1 05:51:32 2021 -0700) on "linux_x64"
Dart: New Project
and selectingsimple-console
templateprint
source codeI used two ways to put this breakpoint :
1 - Using 'Go to definition' and add the breakpoint
2 - Launching a debug session and 'Step into' to reach Dart SDK source code and add the breakpoint
Two breakpoints were created with different paths, neither works :
DartCode debugging logs :
Dart-Code-Log-2021-08-05 13-41-24.txt
Corresponding session in video :
https://user-images.githubusercontent.com/840911/132003058-ce293e04-b9d7-4642-bc9e-2889701ea086.mp4
The text was updated successfully, but these errors were encountered: