microsoft/vscode

VSCode 1.9.0 breaks MSSQL extension - preview HTML pane never refreshes on

kevcunnane opened this issue · 8 comments

  • VSCode Version: 1.9.0
  • OS Version: All

Steps to Reproduce:

A change to editor initialization in VSCode 1.9.0 is breaking the MSSQL extension – any query execution after the 1st time you open a results pane in the WebView is failing to refresh. This means that query execution appears to fail which really is our key experience. Please note this is a breaking change in behavior from previous versions of VSCode.

We raised this a couple of days ago as #19654 which was closed with instructions to change our API call. After fixing our code we discovered the problem happens even if when calling the API correctly.

  • Previously, calling previewHtml would refresh the page if it already existed. This ensured our page was reloaded so the UI could show new query results.
  • Now, it only refreshes if it goes out of focus and comes back into focus. If it’s the main tab on ViewColumn 2, for example, it’ll never be refreshed.
  • This breaks our existing, released extension's behavior.

So this code snippet will open an editor with a WebView component the 1st time it runs, but subsequent calls will never refresh the page.

previewCommandPromise = vscode.commands.executeCommand('vscode.previewHtml', resultsUri, resultPaneColumn, paneTitle);

The impactful change causing this only went into the main branch 6 days ago in https://github.com/Microsoft/vscode/releases/tag/translation%2F20172701.01. We believe the change is due to going from side by side to editor groups control: 0763cc8.

What are the next steps to finding a solution to this? I can imagine we are not the only extension that might be hit by this change. We are looking for a workaround on our side but would appreciate if we can get help on a solution or understand if this could be hotfixed by VSCode.

@kevcunnane the best way to make progress is to provide precise reproducable steps that allow us to reproduce the issue. Then we can debug into the problem. We have also investigated into issue #19654 in the development team. but it has suffered from the same problem and did not provide reproduceable steps.

Thanks - here are the exact steps to reproduce when using our extension. We can work on getting a separate repro in a clean sample extension if needed. Please let me know if this isn't sufficient and we will work on a better repro.

Configuration to connect to SQL from the extension
You need to setup a connection profile to connect to SQL, and create a .sql file that lets you execute SQL queries. Steps are as follows:

  • Have a SQL Server instance which you can connect to.
    • If you do not have one, setup Docker as instructed here. Note you need Docker configured to run with 4GB of RAM, and should take note of the password you use when starting it.
  • Install the MSSQL extension and reload Visual Studio Code
  • Open a .sql file, including 2 statements:
select * from sys.databases
GO
select * from sys.tables
  • Hit F1 and choose the MS SQL: Connect action.
  • Enter connection details:
    • server name =localhost for docker
    • database name = empty (hit Return)
    • username = sa
    • password = YourStrong!Passw0rd (whatever you entered when starting docker)
    • Save Password = true
  • This should create a connection profile and you will see the connection in the bottom right status bar

image

Repro steps - execute 2 separate queries

  • Highlight the text select * from sys.databases and execute MS SQL: Execute Query (Ctrl+Shift+E) to run
    • You will notice the results editor tab opens, which is a WebView component. It shows a result set listing databases

image

  • Highlight the text select * from sys.tables and execute the MS SQL: Execute Query

Expected:

  • The results editor tab is refreshed. You see a new executing event and a different result set with tables is shown

image

Actual

  • The results editor tab is not refreshed. No change happens from the user perspective - the sys.databases results are still shown
  • Note that if you close the results editor tab (or change focus from it by making a different editor active in the same ViewColumn) before calling Execute Query the tab is refreshed.

@kevcunnane thanks, we can work with that.

We've traced the issue on our side and have a fix that we'll be able to apply as an extension update. We will work to get this fix out ASAP so that users are unblocked.

HotFix information
Ensure that TextDocumentContentProvider.provideTextDocumentContent always provides an updated string when being refreshed.

Root cause
We implement TextDocumentContentProvider to represent our results. This expects you to fire an onDidChange event whenever you want to refresh your content, which then requests the updated document contents by calling your provideTextDocumentContent method.

  • We always return the same string, which kicks off our mini HTML app
  • In VSCode 1.9.0, it does a string comparison and only refreshes the page if the content is different
  • A short term fix is therefore to change the content so that it detects a difference and refreshes the document

It's not clear exactly what changed on the VSCode side that caused the change in behavior. However we'll work to fix in our code via this short term hotfix and a better long term solution where our results document is kept in sync without refreshing. Thanks.

Removing the important label, since there is a fix in the SQL extension.

Thanks for the detailed steps and thanks for drilling so deep into this. The issue you are facing was an intentional change to fix #18664.

We added the TextDocumentContentProvider-API to allow extensions to show virtual documents in editors, like sources from dlls or files from the git index. The string a content provider returns is kept in an editor model which does tokenisation, line management, etc. Not all of that is cheap to compute and therefore we added logic to not recompute things when the underlying string of a model didn't change.

Now comes the previewHtml-case which was designed to render a document (on disk or virtual) as html. The design wasn't to allow UI extensibility but we acknowledge that it's being used like that. Now there is a conflicting goal between the original use-case (showing a document that's not on disk) and UI extensibility use-case.

Understanding how two different parties use the content provider it seems both are right. Still, the original design is the one which wants to show a document in an editor. That makes me not wanna revert the fix for #18664. By using the preview-command to build UI you are on the dark side of the force and tricks like fake-updates to a fake documents might be what you need. Us, the VS Code team, should come up with a better story for adding UI to the editor that doesn't require virtual doc, servers, preview commands etc.

Looking forward we should make sure we catch those things sooner. @kevcunnane I wonder if a few devs on your team can self host on the insiders version of VS Code. We update daily, changes and unhappy fixes should become on table a lot quicker, resulting in a lot less stress for everyone.

Last, if no one disagrees, I'd close this without code change

This makes sense to me - the API needs to work for many situations, it's just unfortunate we didn't detect it early enough. I'm on Insiders myself (and did catch pre-release, just way too close to the release date) but will encourage the whole team to use it. Also if we can get the proper fix into our code that better manages our state we'll be more insulated from future breaking changes.

Longer term, it would be nice to have a more supported way to integrate. We're always at a higher risk of breakage due to the nature of the previewHtml implementation. Thanks for the speedy investigation and responsiveness!

Ok. Closing. Happy Coding!