Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

After a disconnect of the browser, commands from a VSCode extension do not work #5025

Closed
ariel-bentu opened this issue Apr 26, 2019 · 12 comments · Fixed by #6159
Closed

After a disconnect of the browser, commands from a VSCode extension do not work #5025

ariel-bentu opened this issue Apr 26, 2019 · 12 comments · Fixed by #6159
Assignees
Labels
bug bugs found in the application critical critical bugs / problems plug-in system issues related to the plug-in system

Comments

@ariel-bentu
Copy link

Description

If the browser disconnects from the server, either briefly due to network error, or if the server restarted, VSCode extension commands stop working.
I also noticed that if the server restarted, the browser reconnects but the VSCode extension was not activated.

Maybe related to this:
#4590

in that case the whole extension may stop functioning and commands is just a special case.

Reproduction Steps

You may clone this vscode extension: https://github.com/ariel-bentu/theia-vscode-gaps.git
when you start the browser first time, you can see that it functions (right click on a node in the view that was added, click one of the commands and see that they work.

Now restart Theia server, and wait for the browser to reconnect - see that now these commands don't work.
Also, in the server log, you can see that the extension did not load (it prints:
console.log('Congratulations, your extension "bugs" is now active!');)

In addition, try disconnecting the network w/o restart and still these commands don't work after the reconnect. (for disconnecting technic, see #4590)

OS and Theia version:
MAC, latest version

Diagnostics:
See above

@akosyakov akosyakov added 🤔 needs more info issues that require more info from the author bug bugs found in the application critical critical bugs / problems plug-in system issues related to the plug-in system labels Apr 29, 2019
@akosyakov
Copy link
Member

@benoitf I can have a look if you don't plan yet for someone else?

@akosyakov akosyakov self-assigned this May 2, 2019
@amiramw
Copy link
Member

amiramw commented May 14, 2019

@benoitf @akosyakov this issue is critical for us. Are you actively working on it or could we try to contribute? (with your help 😃 )

@akosyakov
Copy link
Member

@amiramw try to contribute

@akosyakov akosyakov removed their assignment May 14, 2019
@benoitf
Copy link
Contributor

benoitf commented May 14, 2019

I was on PTO and at Red Hat Summi. @akosyakov you may have a look
about plans, up to @evidolob

@amiramw
Copy link
Member

amiramw commented May 17, 2019

I also tried with language server extension.

When refreshing the browser connection is established fine.

When restarting the server I see this error:

Uncaught Error: no adapter found

Error: no adapter found
    at LanguagesExtImpl.withAdapter (C:\Users\i066470\repos\theia\packages\plugin-ext\lib\plugin\languages.js:109:35)
    at LanguagesExtImpl.$provideCompletionItems (C:\Users\i066470\repos\theia\packages\plugin-ext\lib\plugin\languages.js:139:21)
    at RPCProtocolImpl.doInvokeHandler (C:\Users\i066470\repos\theia\packages\plugin-ext\lib\api\rpc-protocol.js:207:23)
    at RPCProtocolImpl.invokeHandler (C:\Users\i066470\repos\theia\packages\plugin-ext\lib\api\rpc-protocol.js:192:41)
    at RPCProtocolImpl.receiveRequest (C:\Users\i066470\repos\theia\packages\plugin-ext\lib\api\rpc-protocol.js:154:45)
    at RPCProtocolImpl.receiveOneMessage (C:\Users\i066470\repos\theia\packages\plugin-ext\lib\api\rpc-protocol.js:123:22)
    at C:\Users\i066470\repos\theia\packages\plugin-ext\lib\api\rpc-protocol.js:55:89
    at C:\Users\i066470\repos\theia\packages\plugin-ext\lib\api\rpc-protocol.js:233:17
    at C:\Users\i066470\repos\theia\packages\core\lib\common\event.js:161:33
    at CallbackList.invoke (C:\Users\i066470\repos\theia\packages\core\lib\common\event.js:176:39)
    at http://localhost:3000/vs/editor/editor.main.js:12536:31

@benoitf @akosyakov does it give any hint where i should look for the problem? Also is there a nice way to debug the plugin host process code in plugin-ext extension code?

@benoitf
Copy link
Contributor

benoitf commented May 17, 2019

@amiramw you can debug using

--hosted-plugin-inspect-brk=<port-number>

when you start Theia

it will break on first line when you'll access with your browser the ide

@amiramw
Copy link
Member

amiramw commented May 21, 2019

@benoitf is there a way to unload plugin in the browser side? I am trying to trigger reload of all plugins on reconnect and there are duplicate configurations.

@amiramw
Copy link
Member

amiramw commented May 22, 2019

@akosyakov @benoitf
It seems that there are two possible approaches to solve this:

  • Browser listens to reconnects and reload the plugins. This will also work if the theia server is restarted. I tried it with no success. I think it is problematic as the plugins are already loaded in the browser. Is there a way to unload them?
  • The hosted plugins child process can be kept for a while (1 minute?) and on reconnects the new client is linked with the same process. This will work on bad connectivity but not on restart of theia server. I did a working POC but there are questions:
    1. Is there a way to identify that the HostedPluginClient instance is coming from the same old client? Not from different browsers and not from after browser refresh.
    2. Is it ok to keep static fields on HostedPluginProcess for the child process caching? This class is reinstantiated on each connection from the client. Is there a better way to keep such cache?

Any thoughts?

@amiramw
Copy link
Member

amiramw commented May 26, 2019

I created #5257 for keeping the host process alive for a while. This should solve the disconnections issue.

@akosyakov
Copy link
Member

akosyakov commented May 27, 2019

Browser listens to reconnects and reload the plugins. This will also work if the theia server is restarted. I tried it with no success. I think it is problematic as the plugins are already loaded in the browser. Is there a way to unload them?

It's a proper solution. We do it in the similar way for other services already (i.e. language servers, file watching, git watching and so on). On JSON-RPC connection you can be notified when connection is lost and recovered. If a connection is lost then a client plugin state should be cleaned up. If a connection is restored a client should deploy similarly as on first opening. See for file watching for example: https://github.com/theia-ide/theia/blob/78e1cb474e72ed7e641ba7df54ea62b2098684d6/packages/filesystem/src/common/filesystem-watcher-protocol.ts#L80-L84

Not sure that you need to implement the proper unloading of individual plugins. It would beneficial for dynamic plugin uninstallation, but can complicate things for now.

@akosyakov akosyakov self-assigned this Sep 6, 2019
@amiramw
Copy link
Member

amiramw commented Sep 10, 2019

@akosyakov do you plan to implement the proper solution here?
If not, what do you think about reflecting in the UI that plugins don't work anymore and refresh is required?

@akosyakov
Copy link
Member

@amiramw I've started to work on it yesterday.

@akosyakov akosyakov removed the 🤔 needs more info issues that require more info from the author label Sep 11, 2019
akosyakov added a commit that referenced this issue Sep 11, 2019
also fix #5829: ensure that each plugin is loaded only once

Signed-off-by: Anton Kosyakov <[email protected]>
akosyakov added a commit that referenced this issue Sep 11, 2019
also fix #5829: ensure that each plugin is loaded only once

Signed-off-by: Anton Kosyakov <[email protected]>
akosyakov added a commit that referenced this issue Sep 11, 2019
also fix #5829: ensure that each plugin is loaded only once

Signed-off-by: Anton Kosyakov <[email protected]>
akosyakov added a commit that referenced this issue Sep 11, 2019
also fix #5829: ensure that each plugin is loaded only once

Signed-off-by: Anton Kosyakov <[email protected]>
akosyakov added a commit that referenced this issue Sep 12, 2019
also fix #5829: ensure that each plugin is loaded only once

Signed-off-by: Anton Kosyakov <[email protected]>
akosyakov added a commit that referenced this issue Sep 12, 2019
also fix #5829: ensure that each plugin is loaded only once

Signed-off-by: Anton Kosyakov <[email protected]>
akosyakov added a commit that referenced this issue Sep 12, 2019
also fix #5829: ensure that each plugin is loaded only once

Signed-off-by: Anton Kosyakov <[email protected]>
akosyakov added a commit that referenced this issue Sep 12, 2019
also fix #5829: ensure that each plugin is loaded only once

Signed-off-by: Anton Kosyakov <[email protected]>
akosyakov added a commit that referenced this issue Sep 12, 2019
also fix #5829: ensure that each plugin is loaded only once

Signed-off-by: Anton Kosyakov <[email protected]>
akosyakov added a commit that referenced this issue Sep 16, 2019
also fix #5829: ensure that each plugin is loaded only once

Signed-off-by: Anton Kosyakov <[email protected]>
akosyakov added a commit that referenced this issue Sep 16, 2019
also fix #5829: ensure that each plugin is loaded only once

Signed-off-by: Anton Kosyakov <[email protected]>
akosyakov added a commit that referenced this issue Sep 16, 2019
also fix #5829: ensure that each plugin is loaded only once

Signed-off-by: Anton Kosyakov <[email protected]>
akosyakov added a commit that referenced this issue Sep 16, 2019
also fix #5829: ensure that each plugin is loaded only once

Signed-off-by: Anton Kosyakov <[email protected]>
akosyakov added a commit that referenced this issue Sep 16, 2019
also fix #5829: ensure that each plugin is loaded only once

Signed-off-by: Anton Kosyakov <[email protected]>
akosyakov added a commit that referenced this issue Sep 16, 2019
also fix #5829: ensure that each plugin is loaded only once

Signed-off-by: Anton Kosyakov <[email protected]>
akosyakov added a commit that referenced this issue Sep 16, 2019
also fix #5829: ensure that each plugin is loaded only once

Signed-off-by: Anton Kosyakov <[email protected]>
akosyakov added a commit that referenced this issue Sep 16, 2019
also fix #5829: ensure that each plugin is loaded only once

Signed-off-by: Anton Kosyakov <[email protected]>
akosyakov added a commit that referenced this issue Sep 17, 2019
also fix #5829: ensure that each plugin is loaded only once

Signed-off-by: Anton Kosyakov <[email protected]>
akosyakov added a commit that referenced this issue Sep 17, 2019
also fix #5829: ensure that each plugin is loaded only once

Signed-off-by: Anton Kosyakov <[email protected]>
akosyakov added a commit that referenced this issue Sep 17, 2019
also fix #5829: ensure that each plugin is loaded only once

Signed-off-by: Anton Kosyakov <[email protected]>
akosyakov added a commit that referenced this issue Sep 17, 2019
also fix #5829: ensure that each plugin is loaded only once

Signed-off-by: Anton Kosyakov <[email protected]>
akosyakov added a commit that referenced this issue Sep 18, 2019
also fix #5829: ensure that each plugin is loaded only once

Signed-off-by: Anton Kosyakov <[email protected]>
akosyakov added a commit that referenced this issue Sep 18, 2019
also fix #5829: ensure that each plugin is loaded only once

Signed-off-by: Anton Kosyakov <[email protected]>
akosyakov added a commit that referenced this issue Sep 18, 2019
also fix #5829: ensure that each plugin is loaded only once

Signed-off-by: Anton Kosyakov <[email protected]>
akosyakov added a commit that referenced this issue Sep 18, 2019
also fix #5829: ensure that each plugin is loaded only once

Signed-off-by: Anton Kosyakov <[email protected]>
akosyakov added a commit that referenced this issue Sep 18, 2019
also fix #5829: ensure that each plugin is loaded only once

Signed-off-by: Anton Kosyakov <[email protected]>
akosyakov added a commit that referenced this issue Sep 18, 2019
also fix #5829: ensure that each plugin is loaded only once

Signed-off-by: Anton Kosyakov <[email protected]>
akosyakov added a commit that referenced this issue Sep 18, 2019
also fix #5829: ensure that each plugin is loaded only once

Signed-off-by: Anton Kosyakov <[email protected]>
akosyakov added a commit that referenced this issue Sep 19, 2019
also fix #5829: ensure that each plugin is loaded only once

Signed-off-by: Anton Kosyakov <[email protected]>
akosyakov added a commit that referenced this issue Sep 19, 2019
also fix #5829: ensure that each plugin is loaded only once

Signed-off-by: Anton Kosyakov <[email protected]>
akosyakov added a commit that referenced this issue Sep 19, 2019
Also:
- fix #5829: ensure that each plugin is loaded only once
- fix #6186: start only one plugin host process for the same host instead of a new for each load as before
- revert #5257: Reconnect same host plugin process and client" 797db75

Signed-off-by: Anton Kosiakov <[email protected]>
akosyakov added a commit that referenced this issue Sep 19, 2019
Also:
- fix #5829: ensure that each plugin is loaded only once
- fix #6186: start only one plugin host process for the same host instead of a new for each load as before
- revert #5257: Reconnect same host plugin process and client" 797db75

Signed-off-by: Anton Kosiakov <[email protected]>
akosyakov added a commit that referenced this issue Sep 19, 2019
Also:
- fix #5829: ensure that each plugin is loaded only once
- fix #6186: start only one plugin host process for the same host instead of a new for each load as before
- revert #5257: Reconnect same host plugin process and client" 797db75

Signed-off-by: Anton Kosiakov <[email protected]>
akosyakov added a commit that referenced this issue Sep 20, 2019
Also:
- fix #5829: ensure that each plugin is loaded only once
- fix #6186: start only one plugin host process for the same host instead of a new for each load as before
- revert #5257: Reconnect same host plugin process and client" 797db75

Signed-off-by: Anton Kosiakov <[email protected]>
akosyakov added a commit that referenced this issue Sep 20, 2019
Also:
- fix #5829: ensure that each plugin is loaded only once
- fix #6186: start only one plugin host process for the same host instead of a new for each load as before
- revert #5257: Reconnect same host plugin process and client" 797db75

Signed-off-by: Anton Kosiakov <[email protected]>
akosyakov added a commit that referenced this issue Sep 20, 2019
Also:
- fix #5829: ensure that each plugin is loaded only once
- fix #6186: start only one plugin host process for the same host instead of a new for each load as before
- revert #5257: Reconnect same host plugin process and client" 797db75

Signed-off-by: Anton Kosiakov <[email protected]>
akosyakov added a commit that referenced this issue Sep 22, 2019
Also:
- fix #5829: ensure that each plugin is loaded only once
- fix #6186: start only one plugin host process for the same host instead of a new for each load as before
- revert #5257: Reconnect same host plugin process and client" 797db75

Signed-off-by: Anton Kosiakov <[email protected]>
akosyakov added a commit to akosyakov/theia that referenced this issue Feb 24, 2020
Also:
- fix eclipse-theia#5829: ensure that each plugin is loaded only once
- fix eclipse-theia#6186: start only one plugin host process for the same host instead of a new for each load as before
- revert eclipse-theia#5257: Reconnect same host plugin process and client" 797db75

Signed-off-by: Anton Kosiakov <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug bugs found in the application critical critical bugs / problems plug-in system issues related to the plug-in system
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants