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

Hurried Command not found error because the extensions are not loaded yet #34913

Closed
fabiospampinato opened this issue Sep 24, 2017 · 10 comments
Closed
Assignees
Labels
bug Issue identified by VS Code Team member as probable bug extension-host Extension host issues verified Verification succeeded
Milestone

Comments

@fabiospampinato
Copy link
Contributor

I often switch between different projects and execute a shortcut right away, for instance for spawning a terminal via Terminals.

The problem is that I often get some quite annoying Command "whatever" not found, just because the extensions providing those commands have not been loaded yet.

I believe Code should behave like this:

if ( command exists ) {
  run it
} else if ( extensions are still loading ) {
  await them and re-execute the function
} else {
  show the error message
}

Or always wait until the extensions are loaded, in order to avoid executing the wrong command, for instance if an extension overrides a default shortcut.

What do you think?

@vscodebot vscodebot bot added the terminal Integrated terminal issues label Sep 24, 2017
@Tyriar
Copy link
Member

Tyriar commented Sep 26, 2017

I also have this problem primarily with this extension https://marketplace.visualstudio.com/items?itemName=felipecaputo.git-project-manager

Where I would:

  1. ctrl+shift+n
  2. ctrl+alt+p to run the extension command

@Tyriar Tyriar assigned joaomoreno and unassigned Tyriar Sep 26, 2017
@Tyriar Tyriar added extension-host Extension host issues and removed terminal Integrated terminal issues labels Sep 26, 2017
@joaomoreno joaomoreno assigned alexdima and unassigned joaomoreno Sep 27, 2017
@alexdima alexdima added the bug Issue identified by VS Code Team member as probable bug label Sep 27, 2017
@jrieken jrieken added under-discussion Issue is under discussion for relevance, priority, approach and removed bug Issue identified by VS Code Team member as probable bug labels Nov 20, 2017
@jrieken
Copy link
Member

jrieken commented Nov 20, 2017

} else if ( extensions are still loading ) {
await them and re-execute the function

That we don't do and that's on purpose. Editor commands, esp. typing, is being dispatched the same way and we don't want to block typing by slow extension activations. Unlikely, that we are changing this in the near future

@fabiospampinato
Copy link
Contributor Author

@jrieken If the issue with my original proposal is performance what about this:

  • Extracting all contributed commands from each extension's package.json file
  • Merging all these commands in a list of "soon-to-be-available" commands
  • After an extension completes its activation process its commands are removed from said list

So that when a command is triggered:

  • If it's on the list, wait asynchronously for the associated extension to activate and then trigger the command
  • If it's not on the list, it's handled as usual

Performance-wise the parsing step should be pretty fast, and most importantly it can be cached, a re-parse is only really needed if an extension is updated/installed.

This still won't fix the problem for dynamically registered commands, but I don't see much that can be done about them.

@alexdima alexdima added the bug Issue identified by VS Code Team member as probable bug label Nov 21, 2017
@alexdima alexdima added this to the November 2017 milestone Nov 21, 2017
@alexdima
Copy link
Member

There was a gap (the time the workbench comes up until the time extension's package.json finished getting scanned and processed) when commands contributed by extensions could trigger this error.

We changed the heuristic during this time to be:

  • if the command is known in the registry (e.g. the type command dispatched by the editor), it will execute without waiting for the scanning of package.json...
  • if the command is not known in the registry, it will block on scanning of package.json...

@alexdima alexdima removed the under-discussion Issue is under discussion for relevance, priority, approach label Nov 21, 2017
@fabiospampinato
Copy link
Contributor Author

@alexandrudima I'm still getting this issue using the Insiders build (89b158e 2017-11-24T06:03:48.383Z), am I doing something wrong?

@alexdima
Copy link
Member

@fabiospampinato We need to ask the author of that extension to enrich the activation events of the extension from * to '*', 'onCommand:terminals.runTerminal'.

When pressing ctrl+alt+t, we resolve that keybinding to the command terminals.runTerminal.
We then dispatch the command and eventually block on the activation event onCommand:terminals.runTerminal. But there is no extension listening to that activation event, so we proceed to look for and fail to find it. We do not wait for all * extensions to finish activating before invoking commands.

@fabiospampinato
Copy link
Contributor Author

@alexandrudima Thanks for the explanation.

I don't think this is a good enough solution though. By using * the extension should be loaded at startup, i.e. as soon as possible. I don't see way there should ever be the need to also provide a more specific activation event like onCommand:terminals.runTerminal as well.

Why is VSC looking for commands inside activationEvents instead of contributes.commands?
There there are the commands. In this particular case I don't see how adding them to activationEvents provides any information that's not already available.

I'm actually the author of that extension, and 19 others, and it bothers me that I have to update them because of this issue. There's no way those thousands of other extensions are going to get updated.

@alexdima
Copy link
Member

I agree with you. The root problem is that there are two kinds of commands getting invoked via the command service: built-in core commands and contributed extensions commands.

There is a third, more rare case, where a built-in command is overwritten by a contributed command (e.g. the vim extension overwrites the built-in type command).

Now, the problem is that when dispatching a command, the command service has an id and doesn't really know if that command is a built-in one or a contributed one. It should block on the extension host process for contributed commands and it should not for built-in commands. e.g. undo, typing, saving a file, etc. any data-loss related commands should work even if the extension host process refuses to start up, is crashed, spins at 100% CPU, etc.

We need to bring in more knowledge about the two distinct kinds of commands to the command service.

@alexdima alexdima reopened this Nov 27, 2017
@fabiospampinato
Copy link
Contributor Author

I think an easy (maybe temporary) fix for this might be to add the following logic to the bit of code that extracts activation events from an extension: if one of the activation events is *, automatically add onCommand:extension.commandId for each command defined under contributes.commands.

@jrieken jrieken removed this from the November 2017 milestone Dec 5, 2017
@alexdima
Copy link
Member

alexdima commented Feb 7, 2018

To verify:

  • package.json:
{
    "publisher": "alex",
    "name": "34913",
    "version": "0.0.0",
    "engines": {
        "vscode": "^1.0.0"
    },
    "activationEvents": [
        "*"
    ],
    "main": "index.js"
}
  • index.js:
const vscode = require('vscode');

exports.activate = function () {
	vscode.commands.registerCommand(`hello`, () => {
		console.log(`hello!`);
	});
}

*keybindings.json:

    {
        "key": "cmd+1",
        "command": "hello"
    }

Open VSCode and press cmd+1 very quickly. The console should print hello! and you should not get a command not found warning.

@alexdima alexdima added this to the February 2018 milestone Feb 7, 2018
@jrieken jrieken added the verified Verification succeeded label Feb 28, 2018
@vscodebot vscodebot bot locked and limited conversation to collaborators Mar 24, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Issue identified by VS Code Team member as probable bug extension-host Extension host issues verified Verification succeeded
Projects
None yet
Development

No branches or pull requests

5 participants