microsoft / vscode

Visual Studio Code
https://code.visualstudio.com
MIT License
161.94k stars 28.47k forks source link

Web extension `broadcomMFD.hlasm-language-support` no longer works #220171

Closed mihailik closed 1 month ago

mihailik commented 1 month ago

Does this issue occur when all extensions are disabled?: Yes/No

Steps to Reproduce:

  1. Open an existing web extension codebase, or generate a brand new one with yo code as described in the official documentation at Web Extensions | Visual Studio Code Extension API
  2. Run npm install if necessary.
  3. Run npm run run-in-browser to debug Web Extension
  4. Hobbled non-functional version of VSCode is loaded, with disconnected "mount" and the web extension not loaded (see screenshot below)
  5. Clicking in the file tree crashes with unserious error messages. DevTools console looks disastrous (see below).

Unable to resolve resource vscode-test-web://mount/

image

This does 𝗻𝗼𝘁 look like vscode-test-web issue

The session fails on new, and on old well-used versions of @vscode/test-web

Also tried on MacOS (M1), on Ubuntu (arm64 VM) and Windows 10.

Never works.

bpasero commented 1 month ago

There was a recent change to our NLS story that requires vscode-test-web to install a newer version. Can you check what version of "@vscode/test-web" you have installed and make sure its updated to latest?

@aeschli I wonder how we can communicate this to extension authors that they will have to update, besides putting release notes.

mihailik commented 1 month ago

As mentioned above, tried with these versions, including the latest:

v0.22 v0.32 v0.52 v0.55 v0.56 <-- latest

mihailik commented 1 month ago

And I'd love to help you guys with fixing, give me some clues what's changed with NLS.

I can see these, but they're quite terse on details.

NLS: decouple from loader (helps for ESM) #212542

Implement NLS without AMD loader #214588

nls follow up debt work #219265

I'll try to check out https://github.com/microsoft/vscode-test-web update references and see if it comes with any issues by itself. But hints and tips appreciated!

mihailik commented 1 month ago

Aha! Clamping the VSCode to "stable" not "insiders" fixes it for vscode-test-web.

That's good enough to fix the immediate issue so I'll make a PR for vscode-test-web repo patch.

Still give me clues, I'd love to make a proper fix with whatever NLS shenanigans needing to change.

image

bpasero commented 1 month ago

I can reproduce with a fresh extension, but I am not clear how my NLS changes impact this, given we pushed a fix for that 🤔

mihailik commented 1 month ago

PR to vscode-test-web repo to unblock the functionality by falling back onto stable -

Clamp VSCode download to stable release until NLS issue on Insiders is fixed #129

At least my own development is unblocked, but I suspect others might appreciate it.

bpasero commented 1 month ago

I think we would rather address the real issue.

mihailik commented 1 month ago

Sure, I just thought one does not preclude another.

VSCodeTriageBot commented 1 month ago

This bug has been fixed in the latest release of VS Code Insiders!

@mihailik, you can help us out by commenting /verified if things are now working as expected.

If things still don't seem right, please ensure you're on version b23e791eb5afbd95f05aa24da7693ce89344a079 of Insiders (today's or later - you can use Help: About in the command palette to check), and leave a comment letting us know what isn't working as expected.

Happy Coding!

slavek-kucera commented 1 month ago

I am still getting errors:

09ac518b-16fa-4167-807a-4e021e8a04ae:1 Refused to load the script 'blob:http://v--09gskr5q7h8fetd36ifu8kdh2v08q87egl8od71vgvu9r06ame0s.localhost:3000/447093dc-785b-4ae1-8f63-7f861336850a' because it violates the following Content Security Policy directive: "script-src 'self' 'unsafe-eval' 'sha256-V28GQnL3aYxbwgpV3yW1oJ+VKKe/PBSzWntNyH8zVXA=' https: http://localhost:*". Note that 'script-src-elem' was not explicitly set, so 'script-src' is used as a fallback.

(anonymous) @ 09ac518b-16fa-4167-807a-4e021e8a04ae:1
(anonymous) @ 09ac518b-16fa-4167-807a-4e021e8a04ae:1
09ac518b-16fa-4167-807a-4e021e8a04ae:1 Uncaught DOMException: Failed to execute 'importScripts' on 'WorkerGlobalScope': The script at 'blob:http://v--09gskr5q7h8fetd36ifu8kdh2v08q87egl8od71vgvu9r06ame0s.localhost:3000/447093dc-785b-4ae1-8f63-7f861336850a' failed to load.
    at blob:http://v--09gskr5q7h8fetd36ifu8kdh2v08q87egl8od71vgvu9r06ame0s.localhost:3000/09ac518b-16fa-4167-807a-4e021e8a04ae:1:410
    at blob:http://v--09gskr5q7h8fetd36ifu8kdh2v08q87egl8od71vgvu9r06ame0s.localhost:3000/09ac518b-16fa-4167-807a-4e021e8a04ae:1:415
(anonymous) @ 09ac518b-16fa-4167-807a-4e021e8a04ae:1
(anonymous) @ 09ac518b-16fa-4167-807a-4e021e8a04ae:1
Version: 1.92.0-insider
Commit: b23e791eb5afbd95f05aa24da7693ce89344a079
Date: 2024-07-09T05:03:50.549Z
Browser: Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/127.0.0.0 Safari/537.36

EDIT: Would it be possible to add blob: data: to the script-src and connect-src? Seems to fix all my problems.

bpasero commented 1 month ago

@slavek-kucera I cannot reproduce. Here is what I do:

Can you test that?

Are you on Windows with Chrome?

slavek-kucera commented 1 month ago

That runs fine - so blob/data URL will now be unavailable?

bpasero commented 1 month ago

Is this something that your extension is using?

slavek-kucera commented 1 month ago

At the moment yes, I am looking for a workaround at the moment.

bpasero commented 1 month ago

Is there a way for me to run your extension?

slavek-kucera commented 1 month ago

https://marketplace.visualstudio.com/items?itemName=broadcomMFD.hlasm-language-support

slavek-kucera commented 1 month ago

I think this is somehow related to my problem https://github.com/microsoft/vscode/blob/fc8ade92b04da02aecebd856d781a5ea938c99e9/src/vs/workbench/api/worker/extensionHostWorker.ts#L109

bpasero commented 1 month ago

I would need a step by step instruction how to reproduce with your extension, please be detailed and assume I haver never used that extension or ran it.

slavek-kucera commented 1 month ago

I steps to reproduce the problem:

I managed to find a workaround and in the process I discovered a few additional details. The extension uses emscripten toolchain to produce wasm and js glue code that allows the language server to run in the browser. Originally, the Worker hosting the language server was able to fetch and import the glue code and later on start additional Workers (to emulate threads) reusing this imported module. In the Insiders version, this does not seem to be possible anymore and the browser refuses to start the nested Workers, because the first Worker creation is handled by the vscode code I posted above, which means the worker is running a code from a Blob URL and apparently losses the ability to start workers with any other scripts.

bpasero commented 1 month ago

I can reproduce, though I do not fully understand why the Content-Security-Policy suddenly became so strict in the extension host iframe. Before my change, the Worker extension host was created directly as new Worker(url) in the iframe. After my change, the Worker is created with a Blob that has a importScript inside:

https://github.com/microsoft/vscode/blob/316c67d8a25f2f892227bc21df0b2b7c5a98aaa0/src/vs/workbench/services/extensions/worker/webWorkerExtensionHostIframe.html#L96-L104

The iframe has always set a CSP:

https://github.com/microsoft/vscode/blob/316c67d8a25f2f892227bc21df0b2b7c5a98aaa0/src/vs/workbench/services/extensions/worker/webWorkerExtensionHostIframe.html#L4-L8

I can confirm that the adding blob: and data: to script-src allows your extension to run again.

@mihailik where do you see that you also need connect-src to be updated?

slavek-kucera commented 1 month ago

I assume that the connect-src question was directed at me - it might not be needed, it could have been an artifact of my debugging.

mihailik commented 1 month ago

/verified

mihailik commented 1 month ago

Totally works, no more apparent issues.

Though seeing some console outputs related to nls, not sure if it needs to be worrying?

Let me know if I should collect any more info on this one.

IMG20240710105431

And huge thanks for the fast turnaround, and a fix. You rock!!!

bpasero commented 1 month ago

Thanks, I did not figure out that 2 different users are in this issue, so while the original issue seems resolved, there is still the issue with broadcomMFD.hlasm-language-support not working currently in insiders.

slavek-kucera commented 1 month ago

@bpasero I think this can be closed now.

bpasero commented 1 month ago

@slavek-kucera did the change in https://github.com/microsoft/vscode/pull/223153 help?

slavek-kucera commented 1 month ago

@bpasero Not sure, I've found a way to build the extension so that I don't encounter the problem anymore.