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

Debugger treats local packages as "external packages", and doesn't debug by default #1497

Closed
kinex opened this issue Mar 1, 2019 · 11 comments
Labels
in debugger Relates to the debug adapter or process of launching a debug session is bug
Milestone

Comments

@kinex
Copy link

kinex commented Mar 1, 2019

When code execution stops to a breakpoint (or to an exception), the source file and line where the breakpoint is located is not activated automatically in the editor window. You have to click the topmost line in the "Call stack" window to see the correct source location in the editor window. Execution point in the editor window is lost again every time you continue debugging with commands like Step Over or Step Into. So you have to continuously click the correct line in the "Call stack" window to see the correct line in the editor window.

This issue makes the debugging experience terribly bad. There was not this issue earlier, but unfortunately I cannot remember the exact time when this problem occurred.

@DanTup
Copy link
Member

DanTup commented Mar 4, 2019

Can you give a specific example? What frame was shown on the call stack, and what was the top frame?

There were changes made recently in both VS code and the Dart plugin to improve the experience of breaking on exceptions (showing the frame had the users code, rather than framework code that might just say <source not available>), so my guess is that's related - I'd need a concrete example to be sure though.

@DanTup DanTup added is bug in debugger Relates to the debug adapter or process of launching a debug session awaiting info Requires more information from the customer to progress labels Mar 4, 2019
@kinex
Copy link
Author

kinex commented Mar 4, 2019

I found steps how to reproduce the issue with a sample project:

  1. flutter create myapp
  2. flutter create -t package mylib
  3. In myapp/pubspec.yaml add
dependencies:
  mylib:
    path: ..\mylib
  1. Modify myapp/main.dart:
import 'package:mylib/mylib.dart';

void main() {
  final calculator = Calculator();
  calculator.addOne(1);
  runApp(MyApp());
}
  1. Modify mylib/lib/mylib.dart
library mylib;

/// A Calculator.
class Calculator {
  /// Returns [value] plus 1.
  int addOne(int value) {
    print(value);
    return value + 1;
  }
}
  1. Set a breakpoint to the first line of Calculator.addOne
  2. Start debugging
  3. Execution stops to the breakpoint but the file/line is not activated in the editor. Try also Step Over, it is not working as expected (focus in the editor is lost on every step).

Breakpoints in myapp seems to work properly, so I guess the breakpoint (or an exception) must be in a package project to reproduce the issue.

@DanTup
Copy link
Member

DanTup commented Mar 4, 2019

Thanks for the detailed steps - I can repro.

The issue seems related to the "dart.debugExternalLibraries" setting. This lets you control whether packages show be debuggable. In this case your library looks like an external library, but you've put a breakpoint in it. The results in the VM breaking, but VS Code thinks that code should not be debugged.

As a workaround, you can set "dart.debugExternalLibraries": true,, though I'll see if there's an easy way for me to separate "local" packages from pub ones, so you can still avoid stepping into pub packages without losing the ability to step into your own.

@DanTup DanTup removed the awaiting info Requires more information from the customer to progress label Mar 4, 2019
DanTup added a commit that referenced this issue Mar 4, 2019
@DanTup DanTup changed the title Current line is not highlighted automatically while debugging Debugger treats local packages as "external packages", and doesn't debug by default Mar 4, 2019
@DanTup DanTup added this to the v2.25.0 milestone Mar 4, 2019
@kinex
Copy link
Author

kinex commented Mar 4, 2019

Thanks, workaround helped! I think a bigger and well structured app will always consist of an app project + several local packages. For example my app has currently an app project + 11 local packages. Therefore I think it is important that debugging this kind of app works properly. I don't fully understand the idea of the setting dart.debugExternalLibraries here. If it stops to a breakpoint or an exception even if dart.debugExternalLibraries is false, why not to activate the file/line as well.

@DanTup
Copy link
Member

DanTup commented Mar 4, 2019

Therefore I think it is important that debugging this kind of app works properly.

I completely agree. I don't think the debugger is used all that thoroughly so there are a lot of edge cases that have slipped through. I'm trying to add tests where they come up to ensure they work going forwards though (I've got a failing test for this and a fix that seems to work, but has broken some others, so just working through).

I don't fully understand the idea of the setting dart.debugExternalLibraries here

By default, we assume people want to treat libraries (that aren't theirs) as "black boxes". So when an exception occurs, we try to put you on the top-most stack frame of your code rather than on the line that actually threw. It's more obvious with SDK libraries, imagine you had this:

assert(foo != null);

When this assertion fails, you'd want to be taken to that line of code, but without dart.debugSdkLibraries and dart.debugExternalLibraries you'd actually end up inside the implementation of assert which is confusing (especially right now, when the SDK doesn't include sources so you just see <source not available>).

So the bug is that we don't distinguish between "local libraries" (those you probably want to break in the implementation of) versus "external libraries" (those that you probably want to treat as black box, and break on your calls to - on the assumption that the library is bug-free, and you're using it wrong). This isn't always what you want - which is why we have the settings, and #1291 is a request to make it faster to toggle between them, but seems most common. It's intended to work like Just My Code in .NET.

The fix I have will narrow what's considered an "external library" to files inside the Pub Cache folder for pub.dartlang.org, which will hopefully be more like what you'd want (this has the probably-good side effect that if you switch to a path-override for a dependency, it'll be treated as local, as you may be trying to debug/fix something).

@DanTup
Copy link
Member

DanTup commented Mar 4, 2019

Most of what I wrote above was geared around exceptions... I think breakpoints are different and your point is valid - if the user put a breakpoint somewhere, we should never treat it as code we shouldn't break at. I've opened #1502 to tweak that too. It won't be needed for your case here (since we'll detect your local package as local), but I think if you step into an "external" package and set a breakpoint, it's reasonable to expect that to break in the right place too.

@kinex
Copy link
Author

kinex commented Mar 4, 2019

Thanks for the details and quick responses!

@DanTup
Copy link
Member

DanTup commented Mar 5, 2019

@kinex I believe I've fixed both issues. If you'd like to try it out, please remove the setting you added, and then install this build from the VSIX file:

https://github.com/Dart-Code/Dart-Code/releases/tag/v2.25.0-alpha.2

(I've called it an Alpha because it's not feature complete, I believe the quality is good and the tests are all passing minus some flakes)

There's info on installing preview builds here, it's pretty simple and you'll automatically be moved back to stable releases when the next one goes out.

@kinex
Copy link
Author

kinex commented Mar 5, 2019

@DanTup I installed the alpha! No issues found so far after a quick test, I continue testing it.

@DanTup
Copy link
Member

DanTup commented Mar 5, 2019

@kinex Great, thanks for confirming. Do open new issues if you do come across anything (related or otherwise). Thanks!

@kinex
Copy link
Author

kinex commented Mar 5, 2019

@DanTup Yes, sure! And thank you, great work!

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 is bug
Projects
None yet
Development

No branches or pull requests

2 participants