[Salsa] Fix InvalidOperationException when attempting to validate a breakpoint location in a script block

[jramsay]

Issue: an InvalidOperationException can occur in Salsa when setting a breakpoint in a script block. This exception is caught but it means the TSLS never gets the request to resolve the breakpoint and thus the breakpoint cannot be set.

The pCodeSpan that is passed to AbstractLanguageService`3.VsLanguageDebugInfo's ValidateBreakpointLocationWorker contains the iStartLine and iEndline for the full HTML document. The snapshot textbuffer is actually only the contained language document though (JS script block). This means the attempt to get the span from the snapshot can fail because the span endline can be larger then the snapshot line count.

Fix: add a check to ensure the span endline is less than the snapshot line count before attempting to get the snapshot.

[jasonmalinowski]

contains the iStartLine and iEndline for the full HTML document. The snapshot textbuffer is actually only the contained language document though

Does this mean we're missing a mapping between the two documents then? Isn't the right fix to map the start/end lines down to the buffer in question? This seems like it's just working around the issue due an earlier bug.

[jasonmalinowski]

Review: @dotnet/roslyn-ide

[jramsay]

Perhaps, the snapshot point is retrieved by using the iLine and iCol values - these appear to be correct. It is the attempt to adjust the mapping using the pCodeSpan that causes the issue. The pCodeSpan is passed to Roslyn by vsdebug - will see if I can track down where this is set.

[CyrusNajmabadi]

I'm also concerned about this. It definitely looks like this is just trying to avoid crashing here due to bad data being introduced into the system at some point.

Note that BP spans should work in Venus/script-block scenarios already. So i'm surpirsed there's an issue here.

[jramsay]

I think I see the difference between the old JSLS and Salsa for this scenario - will add the WTE folks to the internal thread to confirm the issue.

[jaredpar]

We apologize, but we are closing this PR due to code drift. We regret letting this PR go so long without attention, but at this point the code base has changed too much for us to revisit older PRs. Going forward, we will manage PRs better under an SLA currently under draft in issue #26266 – we encourage you to add comments to that draft as you see fit. Although that SLA is still in draft form, we nevertheless want to move forward with the identification of older PRs to give contributors as much early notice as possible to review and update them.

If you are interested in pursuing this PR, please reset it against the head of master and we will reopen it. Thank you!