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

Missing HTTP proxy settings for Coursier when fetching Metals #1497

Closed
mliarakos opened this issue May 10, 2024 · 6 comments · Fixed by #1498
Closed

Missing HTTP proxy settings for Coursier when fetching Metals #1497

mliarakos opened this issue May 10, 2024 · 6 comments · Fixed by #1498

Comments

@mliarakos
Copy link
Contributor

Describe the bug

Since v1.30.0, Coursier fails to fetch Metals when an HTTP proxy is needed.

To Reproduce

Steps to reproduce the behavior:

  1. Install v1.30.0 of the extension
  2. Configure the serverProperties setting with the HTTP proxy, e.g.:
    • -Dhttps.proxyHost=...
    • -Dhttps.proxyPort=...
  3. Trigger the Metals extension install process
  4. It fails to fetch Metals with an unknown host exception (due to the missing proxy configuration)

This happens both if Coursier is already present on the PATH and if fetched by the extension.

Expected behavior

During the install process Coursier uses the HTTP proxy configuration from the serverProperties setting to successfully fetch Metals.

Screenshots

None.

Installation:

  • Operating system: Linux
  • VSCode version: 1.87.2
  • VSCode extension version: 1.30.3
  • Metals version: 1.3.0

Additional context

In v1.29.0 and earlier the fetchMetals method includes the serverProperties settings as JVM arguments when invoking the default included Coursier. However, in v1.30.0 and newer no external settings are included in the Coursier call to fetch Metals.

I believe the same issue occurs when trying to specify an HTTP proxy using $JAVA_OPTS or the .jvmopts file. Those options are included in v1.29.0, but not in v1.30.0. This is just based on the code, I wasn't using it that way so I didn't test it.

This issue might also be present in v1.29.0 when using an external copy of Couriser, but I didn't test that either.

Search terms

coursier, proxy

@tgodzik
Copy link
Contributor

tgodzik commented May 10, 2024

Thanks for reporting, looks like another issue we need to fix, that should be fairly easy I think. Will take a look!

@LeUser111
Copy link

LeUser111 commented May 22, 2024

I've been encountering this issue with multiple colleagues since version 1.30.0 and with version 1.34.0 the issue is back.

Here are my user settings:

"metals.mavenScript": "/usr/bin/mvn",
"metals.sbtScript": "/opt/sbt/bin/sbt",
"metals.javaHome": "/opt/jdk",
"metals.scalafmtConfigPath": "/home/user/.scalafmt.conf",
"metals.scalafixConfigPath": "/home/user/.scalafix.conf",
"metals.ammoniteJvmProperties": [
    "-Dhttps.proxyHost=127.0.0.1",
    "-Dhttps.proxyPort=3128"
],
"metals.bloopJvmProperties": [
    "-Dhttps.proxyHost=127.0.0.1",
    "-Dhttps.proxyPort=3128"
],
"metals.serverProperties": [
    "-Xmx2G",
    "-Dhttps.proxyHost=127.0.0.1",
    "-Dhttps.proxyPort=3128",
    "-Djavax.net.ssl.trustStore=/home/user/.m2/grobCaTrustStore.jks",
    "-Djavax.net.ssl.trustStorePassword=changeit"
],
"metals.customRepositories": [
    "https://nexuss/nexus/repository/public/"
],
"metals.coursierMirror": "https://nexuss/nexus/repository/public/",

I have removed any additional environment variables (e.g. JAVA_OPTS) and specific settings (e.g. ~/.bloop/bloop.json and ~/.config/coursier/mirror.properties) except ~/.config/coursier/credentials.properties

I've just been running multiple tests removing ~/.cache/coursier/ each time. Things are working with version 1.29.0, 1.32.0 and 1.33.0 (see attached files).
Versions 1.30.0, 1.31.0 and 1.34.0 ignore the server settings and the installation fails (see attached files).

There are two anomalies with the working versions, as well: Bloop fails to start initially, requiring an explicit 'Import build'. That leads to more dependencies being downloaded. That works, as the build server starts afterwards and metals is fully working. However, the download of the bloop dependencies gets reported as ERROR.

One more oddity: I have to specify -Djavax.net.ssl.trustStore for coursier even though the CA certificate has been added to /opt/jdk and any other build tool (maven and sbt) uses the certificte from there...

metals_log_1.29.0.txt
metals_log_1.30.0.txt
metals_log_1.31.0.txt
metals_log_1.32.0.txt
metals_log_1_33_0.txt
metals_log_1.34_0.txt

Edit: Grammar.

@mliarakos
Copy link
Contributor Author

@LeUser111 The fix in #1498 was released in v1.32.0 and reverted in v1.34.0, which explains the changes you're seeing.

@tgodzik Why did it need to be reverted? I can submit a modified PR if needed.

@tgodzik
Copy link
Contributor

tgodzik commented May 23, 2024

The extension on windows stopped working at all so I had to revert some of the changes for now. I plan to get back to it as soon as I can

@tgodzik
Copy link
Contributor

tgodzik commented May 23, 2024

Looks like we can't add the properties on windows in coursier. Adding back the changes aside from that in #1510

I will double check if it works and then will release new version

@tgodzik
Copy link
Contributor

tgodzik commented May 24, 2024

This should be now fixed, any chance anyone can confirm? You would need to switch to a prerelease version.

I will also want to check if everything works correctly on windows before doing the release.

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

Successfully merging a pull request may close this issue.

3 participants