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

Can't run electron from vscode due to ATOM_SHELL_INTERNAL_RUN_AS_NODE=1 #5064

Closed
unional opened this issue Apr 7, 2016 · 17 comments
Closed
Assignees
Labels
bug Issue identified by VS Code Team member as probable bug extensions Issues concerning extensions
Milestone

Comments

@unional
Copy link

unional commented Apr 7, 2016

This seems to be an issue on vscode
vscode version: 0.10.11
OS: OSX and Windows

I'm using vscode and vscode-startanyshell to open cmder.

When I run browserify test.js | tape-run
It fails with:

module.js:340
    throw err;
    ^
Error: Cannot find module 'electron'

After some digging, it turns out to be ATOM_SHELL_INTERNAL_RUN_AS_NODE=1 causes the issue.

Once I remove that env variable, it started working again.

Reference:
remcoros/vscode-startanyshell#2
electron/electron#4979 (comment)

@Tyriar
Copy link
Member

Tyriar commented Apr 7, 2016

ATOM_SHELL_INTERNAL_RUN_AS_NODE=1 is set purely for the CLI so that electron does not need to initialize before basic args are dealt with. Electron is then launched without ATOM_SHELL_INTERNAL_RUN_AS_NODE=1 as a detached process to start vscode here.

What do you think is the issue with that env var?

@Tyriar Tyriar self-assigned this Apr 7, 2016
@unional
Copy link
Author

unional commented Apr 7, 2016

Obviously I don't know about the inner working of electron and vscode. 😄

What I did is empirically determine that this env var is the cause. I compare the env between a normal terminal and the one spawned from vscode and saw that variable is set.

Once I remove it things started working again.

In https://github.com/remcoros/vscode-startanyshell/blob/master/src/extension.ts#L46
It is simply starting the cmder (or other shells) using child_process.exec. The config is:

    "startanyshell.shells": [
        {
            "description": "Cmder",
            "command": "start \"%description%\" \"%HOMEDRIVE%\\Program Files (x86)\\cmder\\Cmder.exe\" /START %path%"
        }
    ]

Given that, I don't see why the env var should be set and cause the issue I experienced.

On the other hand, when I use Open new command prompt, I don't see that flag being set.

@unional
Copy link
Author

unional commented Apr 7, 2016

CC /@remcoros

@remcoros, I tried with open new command prompt and that env var is not set.

@Tyriar , I do what vscode is running, process.env[] does not have that env var set. i.e. the code you pointed out is working correctly.

Really have no idea why when child_process.exec() called by @remcoros the env var is still there / added back in.

@unional
Copy link
Author

unional commented Apr 7, 2016

Just dig in a bit and here is my finding:
If I run require('child_process').exec('start cmder.exe /start e:\\') **directly in vscode developer tool, it is working correctly (i.e. the var is not set).
If I run start any shell to run the same thing (through its command), it is not working correctly (i.e. the var is set).

vscode runs extensions in difference process(es). In those process(es) the process.env still have that var set.
I confirm with directly modifying the extension.js:

vscode_1.window.showErrorMessage(process.env['ATOM_SHELL_INTERNAL_RUN_AS_NODE'].toString());

And it prints out
image

@egamma
Copy link
Member

egamma commented Apr 7, 2016

// CC @weinand

@Tyriar
Copy link
Member

Tyriar commented Apr 7, 2016

Looks like it's related to extensions, not launch.

@Tyriar Tyriar assigned joaomoreno and unassigned Tyriar Apr 7, 2016
@Tyriar Tyriar added bug Issue identified by VS Code Team member as probable bug extensions Issues concerning extensions labels Apr 7, 2016
@weinand
Copy link
Contributor

weinand commented Apr 7, 2016

VS Code should always un-define (or set to 0) this environment variable on entry to make VS Code independent from it.

@joaomoreno
Copy link
Member

We need that environment variable set, since we don't really know what the runtime consequences of unsetting it will be.

It's unhappy, but I suggest to let the vscode-startanyshell extension author know about it and he should unset it when spawning a process.

Thoughts @unional @Tyriar @weinand ?

@remcoros
Copy link

remcoros commented Apr 8, 2016

This effects all extensions spawning an external process, not just shell extensions.

vscode-cli unsets this variable on startup (see https://github.com/Microsoft/vscode/blob/ad7e861dbb9127e919bc686da162b19cbb7e2391/src/vs/workbench/electron-main/cli.ts#L59). Can this be also done for extension (forked) processes?

@Tyriar
Copy link
Member

Tyriar commented Apr 8, 2016

@remcoros @joaomoreno that's what I was thinking; clone the env and remove the var for extension processes.

@unional
Copy link
Author

unional commented Apr 8, 2016

Agree, should give the extensions a clean env to work with.

@Tyriar
Copy link
Member

Tyriar commented Apr 8, 2016

This is loosely related to #4779, vscode sources its env by running env which doesn't include scoped variables.

@joaomoreno
Copy link
Member

@Tyriar When I say that we don't really know what the runtime consequences of unsetting it will be, I mean it in the extension host process. We do want that process to run as Node, and I'm not sure if that flag is only read on process startup or during runtime too.

@unional
Copy link
Author

unional commented Apr 11, 2016

I'm not sure if that flag is only read on process startup or during runtime too.

It depends on how much testing you have in place. 😜
Just chiming in and not trying to be harsh in any sense 🌹

@unional
Copy link
Author

unional commented Dec 11, 2016

Seems like it is fixed now. 🌷

@joaomoreno
Copy link
Member

joaomoreno commented Dec 12, 2016

We haven't really changed anything. The variable isn't set anymore in your world, @unional?

@unional
Copy link
Author

unional commented Dec 12, 2016

I think there are PR that change ATOM_SHELL_INTERNAL_RUN_AS_NODE to ELECTRON_RUN_AS_NODE and there are some rewrite of the code.
I do not have the same setup anymore so I can't fully validate it.

@vscodebot vscodebot bot locked and limited conversation to collaborators Nov 18, 2017
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 extensions Issues concerning extensions
Projects
None yet
Development

No branches or pull requests

6 participants