Repository navigation
setBreakpoints does not use updated location if the debug adapter moves a breakpoint #545
Description
I'm trying to optimise a debug adapter to not just delete all breakpoints and re-create them when a setBreakpoints request is received by instead just adding and removing the necessary breakpoints.
As part of this, I need to determine which breakpoints in a setBreakpoints request are new, unchanged, and removed. This requires matching up their locations with the known breakpoints.
While testing this, I noticed that VS Code continues to send the original location of a breakpoint, even if the debug adapter has moved it.
For example:
// setBreakpoints request to add breakpoints on line 3 (invalid) and line 17 (valid)
[12:09:39 PM] [DAP] [Info] ==> {"command":"setBreakpoints","arguments":{"source":{"name":"breakpoint_testing.dart","path":"d:\\Dev\\Test Projects\\dart_sample\\bin\\breakpoint_testing.dart"},"lines":[3,17],"breakpoints":[{"line":3},{"line":17}],"sourceModified":false},"type":"request","seq":3}
// setBreakpoints response marks both as pending
[12:09:39 PM] [DAP] [Info] <== {"seq":3,"type":"response","body":{"breakpoints":[{"id":100000,"message":"Breakpoint has not yet been resolved","reason":"pending","verified":false},{"id":100001,"message":"Breakpoint has not yet been resolved","reason":"pending","verified":false}]},"command":"setBreakpoints","request_seq":3,"success":true}
// invalid brekpoint gets a message
[12:09:39 PM] [DAP] [Info] <== {"seq":88,"type":"event","body":{"breakpoint":{"id":100000,"message":"No debuggable code where breakpoint was requested","reason":"failed","verified":false},"reason":"changed"},"event":"breakpoint"}
// valid breakpoint gets resolved and set to column 3
[12:09:39 PM] [DAP] [Info] <== {"seq":94,"type":"event","body":{"breakpoint":{"column":3,"id":100001,"line":17,"verified":true},"reason":"changed"},"event":"breakpoint"}
// When adding a new breakpoint, the original 17:3 breakpoint is sent back to the DA as just a line breakpoint even though the DA updated it
[12:09:42 PM] [DAP] [Info] ==> {"command":"setBreakpoints","arguments":{"lines":[3,13,17],"breakpoints":[{"line":3},{"line":13},{"line":17}],"sourceModified":false},"type":"request","seq":15}
It's possible this is intended, but it's not specified this way, and in order to match up breakpoints, DAs need to know whether to expect the "original" breakpoints to be sent again, or whether a client will send them based on the changes it provided (so it knows which line/col to compare to).
So if this is a bug, it should be fixed - but regardless of whether it's a bug or intended, I think the DAP spec should make it clear that "changed" breakpoint events will not change the data sent in setBreakpoints when other breakpoints are added/removed in the same file.
Activity
connor4312 commentedon Jun 9, 2025
This is intentional. For VS Code in general the point of truth for where a breakpoint should be is the client. The breakpoint might not be in that location for a number of reasons depending on the state of things and the runtime. For example, if a breakpoint is set on an empty like, a DA might choose to move it to the expression that it will actually pause on. Similarly if the client's code is outdated versus what's in the runtime, the breakpoint might move unpredictably, but once the runtime reloads it binds to the location the user intended.
VS Code will preserve the breakpoint object identically between when they're sent unless the user changes them, so you can check their property equivalence (e.g. js-debug).
I agree with should specify this better. The behavior we currently have in VS Code:
addevent, or by the clientchangedby the DA to notify the client of a change in its display position or display attributes. However, the breakpoint itself outside of the display attributes (from the point of view of a DA) is immutable.removeevent, or by the clientDanTup commentedon Jun 10, 2025
Connor Peet (@connor4312) are breakpoints added with
addsubsequently included insetBreakpointscalls, and breakpoints removed withremoveexcluded? (if so, not reflectingchangefeels odd to me... but not including/excluding them also feels slightly odd).It would be nice to improve the spec here before I start making changes to the Dart DA, to ensure we're definitely all on the same page. Thanks!
connor4312 commentedon Jun 10, 2025
Yes
Agreed. It's kind of an odd case, in retrospect I think having DA-driven
add/removemight not have been the best long term thing to do but we are where we are.DanTup commentedon Jun 10, 2025
Yeah, I agree, seems odd. But since this is how it is, I think it's important it's well-documented.
Btw, I'd already opened #542 in this repo, so feel free to close one of them and keep the other.
I've updated the Dart DA implementation based on the above.
Thanks!
fix: clarify representation of breakpoint after changed event
fix: clarify representation of breakpoint after changed event (#546)