vscode-java icon indicating copy to clipboard operation
vscode-java copied to clipboard

Use of SERVER_PORT for the JDT LS is unexpected and inconvenient in a Devcontainer environment

Open dgerhardt opened this issue 2 years ago • 2 comments

When the SERVER_PORT is set for the container environment (e.g. through a docker-compose.yml), the extension uses it to bind the JDT language server. Since SERVER_PORT is commonly used (e.g. Spring applications), this can cause conflicts (the application cannot start because the port is already bound). Furthermore, changing the port causes issues with the extension.

Environment
  • Operating System: Ubuntu 22.04
  • JDK version: 17
  • Visual Studio Code version: 1.68.0
  • Java extension version: 1.7.0
Steps To Reproduce
  1. Create a new devcontainer with the mcr.microsoft.com/vscode/devcontainers/java:17-bullseye image, "containerEnv": {"SERVER_PORT": "8123"} and the extension vscjava.vscode-java-pack.
  2. Open a terminal in the devcontainer and run ss -tulpen. The Extension Host is using port 8123 set via SERVER_PORT.
  3. The vscjava.vscode-java-pack extension will not work correctly.

Reproduction repository: https://github.com/dgerhardt/vscode-devcontainer-server-port-issue

Current Result

The extension is affected by the container's environment variables which are meant for the application.

Expected Result

The extension should not be affected by the container's environment variables for the application, use a more specific variable name (e.g. JDTLS_SERVER_PORT) or provide a way to override this behavior.

dgerhardt avatar Jun 13 '22 13:06 dgerhardt

Makes sense to me.

Maybe we can change the line here: https://github.com/redhat-developer/vscode-java/blob/9b6046eecc65fd47507f309a3ccc9add45c6d3be/src/standardLanguageClient.ts#L82

to

const port = process.env['JDTLS_SERVER_PORT'] || process.env['SERVER_PORT']; ?

I'm thinking any possibility of introducing breaking by doing this, seems safe so far.

jdneo avatar Jun 14 '22 02:06 jdneo

Would process.env['JDTLS_SERVER_PORT'] ?? process.env['SERVER_PORT'] work here? This way JDTLS_SERVER_PORT= (empty value) could be used to ignore SERVER_PORT and revert to the default behavior.

It might also be worth to consider deprecating SERVER_PORT in favor of a new JDTLS_SERVER_PORT variable and warn the user if SERVER_PORT from the environment is used. In general, I would expect extensions to be isolated from my application's environment. For me as an extension user it took a long time of debugging to even recognize that the redhat.java extension using the SERVER_PORT variable from the container environment was the cause of my issue. At first I thought it was a bug in VS Code's Remote Containers extension until a dev over there lead me here.

dgerhardt avatar Jun 14 '22 07:06 dgerhardt