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

Wrong handling of environment variables breaks execution in VS Code #951

Closed
cdauth opened this issue Jan 5, 2021 · 1 comment
Closed

Comments

@cdauth
Copy link

cdauth commented Jan 5, 2021

I have recently started to use VS Code for development and am trying to get my existing Maven setup running there. When run from the VS Code terminal, the build fails with the following message:

[INFO] --- frontend-maven-plugin:1.6:install-node-and-yarn (install-node-and-yarn) @ scroll-documents-ui ---
[INFO] Node v10.13.0 is already installed.
[INFO] Installing Yarn version v1.12.1
[INFO] Unpacking /home/cdauth/.m2/repository/com/github/eirslett/yarn/1.12.1/yarn-1.12.1.tar.gz into /home/cdauth/Documents/workspace/k15t/java/scroll-documents-reloaded/scroll-documents-ui/src/main/frontend/node/yarn
[INFO] Installed Yarn locally.
[INFO] 
[INFO] --- frontend-maven-plugin:1.6:yarn (yarn-install) @ scroll-documents-ui ---
[INFO] Running 'yarn ' in /home/cdauth/Documents/workspace/k15t/java/scroll-documents-reloaded/scroll-documents-ui/src/main/frontend
[ERROR] internal/modules/cjs/loader.js:582
[ERROR]     throw err;
[ERROR]     ^
[ERROR] 
[ERROR] Error: Cannot find module '"/home/cdauth/.config/Code'
[ERROR]     at Function.Module._resolveFilename (internal/modules/cjs/loader.js:580:15)
[ERROR]     at Function.Module._load (internal/modules/cjs/loader.js:506:25)
[ERROR]     at Module.require (internal/modules/cjs/loader.js:636:17)
[ERROR]     at Module._preloadModules (internal/modules/cjs/loader.js:811:12)
[ERROR]     at preloadModules (internal/bootstrap/node.js:594:7)
[ERROR]     at startup (internal/bootstrap/node.js:275:9)
[ERROR]     at bootstrapNodeJSCore (internal/bootstrap/node.js:739:3)
[INFO] ------------------------------------------------------------------------
[INFO] BUILD FAILURE
[INFO] ------------------------------------------------------------------------
[INFO] Total time:  1.513 s
[INFO] Finished at: 2021-01-05T18:41:45+01:00
[INFO] ------------------------------------------------------------------------
[ERROR] Failed to execute goal com.github.eirslett:frontend-maven-plugin:1.6:yarn (yarn-install) on project scroll-documents-ui: Failed to run task: 'yarn ' failed. org.apache.commons.exec.ExecuteException: Process exited with an error: 1 (Exit value: 1) -> [Help 1]
[ERROR] 
[ERROR] To see the full stack trace of the errors, re-run Maven with the -e switch.
[ERROR] Re-run Maven using the -X switch to enable full debug logging.
[ERROR] 
[ERROR] For more information about the errors and possible solutions, please read the following articles:
[ERROR] [Help 1] http://cwiki.apache.org/confluence/display/MAVEN/MojoFailureException

I have done some extensive debugging and found the root of the problem.

The way that debugging works in VS Code is that you start your application through the terminal, as you normally would outside of the IDE. VS Code detects when you start a supported framework and automatically attaches a debugger to it. The way that Node.js debugging is supported seems to be by setting the NODE_OPTIONS environment variable in the terminal. In my case it is set to the value --require "/home/cdauth/.config/Code - OSS/User/workspaceStorage/7dc69b8b7a8030f521865ff9f4ba0066/ms-vscode.js-debug/bootloader.js". This seems to cause Node.js to require that file when starting, and I suppose the file contains some code that makes itself known to the IDE so that the debugger can connect.

From what I can see in ProcessExecutor:74, frontend-maven-plugin uses Apache Commons Exec to start processes such as yarn. Apache Commons Exec uses Runtime.exec() to start processes, as can be seen in Java13CommandLauncher:60 (keep in mind that Java13 stands for Java 1.3 here, not Java 13). [Runtime.exec()](https://docs.oracle.com/javase/7/docs/api/java/lang/Runtime.html#exec(java.lang.String[],%20java.lang.String[]) takes environment variables as an array of strings of the form VAR=value. Apache Commons Exec converts the existing environment variables to that format, so in my case it adds the value NODE_OPTIONS=--require "/home/cdauth/.config/Code - OSS/User/workspaceStorage/7dc69b8b7a8030f521865ff9f4ba0066/ms-vscode.js-debug/bootloader.js" to the array. I couldn't find any details about what format Runtime.exec() expects for the environment variables, but it seems that the quotes are getting lost. The spawned yarn process has in its environment NODE_OPTIONS=--require /home/cdauth/.config/Code - OSS/User/workspaceStorage/7dc69b8b7a8030f521865ff9f4ba0066/ms-vscode.js-debug/bootloader.js, so without the quotes, and this is where things are going wrong.

I could find many issue reports about Runtime.exec() handling quotes incorrectly, although none of them talk about environment variables specifically (most of them are about the command itself). I would argue that this is a bug in OpenJDK, or at least in its documentation, and I will try to report it there.

There is an issue report for Apache Commons Exec that is somewhat related to this, but it is more than 10 years old despite being marked as "Critical". It seems that Apache Commons Exec is unmaintained?

Even though I don't consider this problem a bug in frontend-maven-plugin, I'm reporting this here for other to find who run across the same issue.

A lot of people online seem to suggest to use ProcessBuilder instead of Runtime.exec(). It seems to have been created as a better replacement. Since it seems unlikely to me that OpenJDK or Apache Commons will fix this issue on their side any time soon, maybe it would be the best solution if frontend-maven-plugin migrated from Apache Commons Exec to ProcessBuilder?

@cdauth
Copy link
Author

cdauth commented Jan 5, 2021

While trying to create a simple test case to reproduce this issue, I discovered that the problem is actually not caused by OpenJDK, but by the way Node.js parses NODE_OPTIONS.

The issue was fixed in Node.js 12.0.0. Upgrading my Node.js version solved the issue for me.

@cdauth cdauth closed this as completed Jan 5, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant