Allow Overwrite of Stacktrace with SupportsDelayedStackTraceLoading

[JustinGrote]

I have a situation where calculating my stacktrace is expensive (takes 300ms-800ms in PowerShell for "reasons").

Editors such as vscode will not display where they are stopped until they receive the first stackTrace frame, then they will background the remainder stacktrace fetch if SupportsDelayedStackTraceLoading is specified. Because of this, stepping in the PowerShell debug adapter feels "slow" as the editor will not respond until I can give it at least one stack trace.

I can send it a "dummy" breakpoint frame immediately that is 90% there with id: 0 in order to get the editor UI to respond quickly, but if the editor requests the next stackTrace by startFrame at 1, if I "spuriously" supply it again via id:0 it still gets appended to the call stack list rather than replace what is in the call stack.

Before I can ask in vscode to change this behavior, it should be less ambiguous in the spec, so either a statement that says subsequent stacktrace requests with the same ID should overwrite the existing stacktraces if spuriously received outside the requested range, or allow an additional method before stacktrace (or additional property as part of stopped event) that allows specifying the breakpoint location so the editor can immediately place its pointer there while backgrounding for further callstack/stacktrace info.

Thanks!

[connor4312]

I'm not a big fan of adding overwriting semantics to the spec just to work around the issue. I think the right solution is to bug the VS Code folks (hey, that's me!) to fix this behavior. I can transfer this to the VS Code repo if you concur.

[JustinGrote]

@connor4312 thanks for the fast response (especially as busy as you are!)

My thought was around the idea that, if a client requests continuing stackframes starting at 1, and I supply a "spurious" response that includes id:0 that the client didn't ask for, should that be considered an invalid operation, or should that be considered intent that I want to overwrite a previous stackframe? Seems to me that we wouldn't include IDs at all if we didn't want the ability to overwrite

My worry is that it will lead to inconsistent behavior if not specified in the spec. For instance, you may fix this in VS Code, but the neovim LSP client may disagree and not let me overwrite because it's not explicitly called out as a legal operation in the spec, leading to inconsistent behavior for users. Does that make sense?

That being said I'm totally happy to just fix it in vscode for now if you agree this should be allowed behavior, so please transfer if that is the case, otherwise it should be closed as "wontfix" if this should not be allowed behavior, or left open if agree this should be part of the spec.

[JustinGrote]

@connor4312 after looking into this further, since there is a stack presentation type of "label" specifically for artificially created stacktraces, I can present the first artificial item <BreakPoint> quickly by creating it as a label, and then have the remaining stacktrace arrive later. This works as expected in vscode and is a sufficient workaround with good UX to the user, so I'll leave this item open but it can be deprioritized.

[connor4312]

should that be considered an invalid operation, or should that be considered intent that I want to overwrite a previous stackframe? Seems to me that we wouldn't include IDs at all if we didn't want the ability to overwrite My worry is that it will lead to inconsistent behavior if not specified in the spec.

Yea, I would consider this undefined behavior and clients are free to do whatever they end up with -- duplicating the stack frame, throwing an error, making a BSOD, etc

The fix in VS Code that I would apply is just to have the "paused" state be shown immediately and have a loading indicator in the Call Stack view while the stacktrace is loading. It sounds like this would solve your use case, and lead to effectively the same behavior as <Breakpoint> but without the extraneous stack frame.

[JustinGrote]

The one issue with that is that it doesn't know "where" to pause yet, hence why the first frame is necessary, in my traces it uses the first frame position to determine where to drop the breakpoint indicator, so an initial frame is necessary.

As demoed here, it's definitely "good enough" however.

[connor4312]

I think a client could figure that out and effectively do the same if a DA passed hitBreakpointIds in the stopped event.