fix: improve CLI download error handling [IDE-1690] (#353)

Add HTTP status code validation in FileDownloadResponseHandler (only accept 2xx & 3xx responses, log response body for debugging).
Include download URL and status code in error messages.
Gracefully handle download failures in SnykStartup, but allow LS startup with existing binary if available.
Validate CLI binary exists before starting language server, with helpful error message suggesting to check Error Log when "Manage Binaries Automatically" is enabled.

Author: rrama

plugin/src/main/java/io/snyk/eclipse/plugin/SnykStartup.java

@@ -1,7 +1,5 @@
 package io.snyk.eclipse.plugin;
 
-import static io.snyk.eclipse.plugin.utils.SnykLogger.logError;
-
 import java.io.File;
 import java.io.IOException;
 import java.net.URISyntaxException;
@@ -16,6 +14,7 @@
 import org.eclipse.core.runtime.Platform;
 import org.eclipse.core.runtime.Status;
 import org.eclipse.core.runtime.jobs.Job;
+import org.eclipse.swt.SWTException;
 import org.eclipse.ui.IStartup;
 import org.eclipse.ui.IWorkbench;
 import org.eclipse.ui.IWorkbenchPage;
@@ -29,6 +28,7 @@
 import io.snyk.eclipse.plugin.wizards.SnykWizard;
 import io.snyk.languageserver.LsRuntimeEnvironment;
 import io.snyk.languageserver.SnykLanguageServer;
+import io.snyk.languageserver.download.ChecksumVerificationException;
 import io.snyk.languageserver.download.HttpClientFactory;
 import io.snyk.languageserver.download.LsBinaries;
 import io.snyk.languageserver.download.LsDownloader;
@@ -50,6 +50,8 @@ public void earlyStartup() {
 			protected IStatus run(IProgressMonitor monitor) {
 				monitor.beginTask("Initializing...", 100);
 				downloadIfNeeded(monitor);
+
+				// Even if download failed, try to start LS - there might be an existing binary
 				monitor.subTask("Starting Language Server...");
 
 				try {
@@ -62,7 +64,7 @@ protected IStatus run(IProgressMonitor monitor) {
 							SnykWizard.createAndLaunch();
 						}
 					});
-				} catch (RuntimeException e) {
+				} catch (RuntimeException e) { // NOPMD - intentional catch-all of runtime exceptions for UI initialization
 					SnykLogger.logError(e);
 				}
 				monitor.done();
@@ -76,12 +78,31 @@ private void downloadIfNeeded(IProgressMonitor monitor) {
 						monitor.subTask("Downloading CLI");
 						logger.info("LS: Need to download");
 						downloading = true;
-						download(monitor);
+						IStatus status = download(monitor);
+						if (!status.isOK()) {
+							logDownloadFailure(status);
+						}
 					}
-				} catch (Exception exception) {
-					logError(exception);
+				} catch (Exception e) { // NOPMD - intentional catch-all for download failures
+					logDownloadFailure(e.getMessage());
+				} finally {
+					downloading = false;
+				}
+			}
+
+			private void logDownloadFailure(IStatus status) {
+				String errorMessage = status.getMessage();
+				if (status.getException() != null) {
+					errorMessage += ": " + status.getException().getMessage();
 				}
-				downloading = false;
+				logDownloadFailure(errorMessage);
+			}
+
+			private void logDownloadFailure(String errorMessage) {
+				// Log the error - user may just be offline but have an existing binary. We show a user-facing error later if binary is missing.
+				String message = "Failed to download Snyk CLI: " + errorMessage
+						+ ". Will try to start with existing binary if available.";
+				logger.error(message);
 			}
 		};
 		initJob.setPriority(Job.LONG);
@@ -141,7 +162,7 @@ public static IStatus download(IProgressMonitor monitor) {
 			lsFile.getParentFile().mkdirs();
 			lsDownloader.download(monitor);
 			lsFile.setExecutable(true);
-		} catch (RuntimeException | URISyntaxException e) {
+		} catch (RuntimeException | IOException | URISyntaxException | ChecksumVerificationException e) { // NOPMD - intentional catch-all of runtime exceptions for download errors
 			return Status.error("Download of Snyk Language Server failed", e);
 		}
 		return Status.OK_STATUS;

plugin/src/main/java/io/snyk/eclipse/plugin/utils/FileDownloadResponseHandler.java

@@ -1,39 +1,87 @@
 package io.snyk.eclipse.plugin.utils;
 
+import java.io.BufferedReader;
 import java.io.File;
 import java.io.IOException;
 import java.io.InputStream;
+import java.io.InputStreamReader;
+import java.nio.charset.StandardCharsets;
 import java.nio.file.Files;
+import java.util.stream.Collectors;
 
 import org.apache.http.HttpResponse;
 import org.apache.http.client.ResponseHandler;
+import org.eclipse.core.runtime.ILog;
 import org.eclipse.core.runtime.IProgressMonitor;
+import org.eclipse.core.runtime.Platform;
 import org.eclipse.core.runtime.SubMonitor;
 
 public class FileDownloadResponseHandler implements ResponseHandler<File> {
 
 	private final File destinationFile;
 	private final IProgressMonitor progressMonitor;
+	private final String downloadUrl;
+	private final ILog logger;
 
-	public FileDownloadResponseHandler(File file, IProgressMonitor monitor) {
+	public FileDownloadResponseHandler(File file, IProgressMonitor monitor, String downloadUrl) {
 		this.destinationFile = file;
 		this.progressMonitor = monitor;
+		this.downloadUrl = downloadUrl;
+		this.logger = Platform.getLog(getClass());
 	}
 
 	@Override
 	public File handleResponse(HttpResponse httpResponse) throws IOException {
+		int statusCode = httpResponse.getStatusLine().getStatusCode();
+
+		// Accept 2xx (success) and 3xx (redirect) status codes.
+		// Note: We don't expect to see 3xx here since HttpClient follows redirects automatically.
+		// Redirect issues (circular redirects, too many redirects) throw RedirectException.
+		// If we do see 3xx, we'll treat it as okay.
+		if (statusCode < 200 || statusCode >= 400) {
+			handleHttpError(httpResponse, statusCode);
+		}
+
 		long contentLengthStr = httpResponse.getEntity().getContentLength();
 		SubMonitor subMonitor = SubMonitor.convert(progressMonitor, 100);
 		try (InputStream inputStream = httpResponse.getEntity().getContent();
 				var outputStream = Files.newOutputStream(destinationFile.toPath())) {
 			int readCount;
 			byte[] buffer = new byte[1024];
 
-			while ((readCount = inputStream.read(buffer)) != -1) { // NOPMD by bdoetsch on 3/11/25, 2:29 PM
+			while ((readCount = inputStream.read(buffer)) != -1) { // NOPMD by bdoetsch on 3/11/25, 2:29 PM
 				outputStream.write(buffer, 0, readCount);
 				subMonitor.split(Math.round((float) readCount / (float) contentLengthStr));
 			}
 		}
 		return this.destinationFile;
 	}
+
+	private void handleHttpError(HttpResponse httpResponse, int statusCode) throws IOException {
+		String reasonPhrase = httpResponse.getStatusLine().getReasonPhrase();
+		String responseBody = readResponseBody(httpResponse);
+
+		// Log the response body for debugging (may contain useful error details)
+		logger.warn(String.format("CLI download failed. URL: %s, Status: %d %s, Response body: %s",
+				downloadUrl, statusCode, reasonPhrase, responseBody));
+
+		// Throw exception with URL and status (but not the body - may be large, may be binary or may be HTML we accidentally render)
+		throw new IOException(String.format(
+				"Download failed with HTTP %d %s for URL: %s",
+				statusCode, reasonPhrase, downloadUrl));
+	}
+
+	private String readResponseBody(HttpResponse httpResponse) {
+		try {
+			if (httpResponse.getEntity() == null) {
+				return "(no response body)";
+			}
+			try (BufferedReader reader = new BufferedReader(
+					new InputStreamReader(httpResponse.getEntity().getContent(), StandardCharsets.UTF_8))) {
+				return reader.lines().collect(Collectors.joining("\n"));
+			}
+		} catch (IOException | UnsupportedOperationException e) {
+			return "(could not read response body: " + e.getMessage() + ")";
+		}
+	}
 }

plugin/src/main/java/io/snyk/languageserver/SnykLanguageServer.java

@@ -1,5 +1,6 @@
 package io.snyk.languageserver;
 
+import java.io.File;
 import java.io.IOException;
 import java.net.URI;
 import java.util.List;
@@ -30,15 +31,39 @@ public SnykLanguageServer() {
 	public void start() throws IOException {
 		var prefs = Preferences.getInstance();
 		waitForInit();
-		List<String> commands = Lists.of(prefs.getCliPath(), "language-server", "-l", "info");
+
+		String cliPath = getCliPathOrThrow(prefs);
+
+		List<String> commands = Lists.of(cliPath, "language-server", "-l", "info");
 		String workingDir = SystemUtils.USER_DIR;
 		setCommands(commands);
 		setWorkingDirectory(workingDir);
 		try {
 			super.start();
 		} catch (IOException e) {
-			SnykLogger.logAndShow("Cannot start the Snyk CLI. Please check the CLI path.");
+			SnykLogger.logAndShow("Cannot start the Snyk CLI. Please check the CLI path: " + cliPath);
+			throw e;
+		}
+	}
+
+	/**
+	 * Returns CLI path if binary exists, logs and throws IOException with helpful message if not.
+	 */
+	static String getCliPathOrThrow(Preferences prefs) throws IOException {
+		String cliPath = prefs.getCliPath();
+		File cliBinary = new File(cliPath);
+		if (!cliBinary.exists()) {
+			var sb = new StringBuilder(200);
+			sb.append("Snyk CLI binary not found at: '").append(cliPath).append("'. \n");
+			if (prefs.isManagedBinaries()) {
+				sb.append("'Manage Binaries Automatically' is enabled - check the Error Log for download details. \n");
+			}
+			sb.append("You can also specify a custom CLI path in Snyk Preferences.");
+			String message = sb.toString();
+			SnykLogger.logAndShow(message);
+			throw new IOException(message);
 		}
+		return cliPath;
 	}
 
 	public static void waitForInit() {

plugin/src/main/java/io/snyk/languageserver/download/ChecksumVerificationException.java

@@ -1,6 +1,6 @@
 package io.snyk.languageserver.download;
 
-public class ChecksumVerificationException extends RuntimeException {
+public class ChecksumVerificationException extends Exception {
 	private static final long serialVersionUID = 7210924932018107815L;
 
 	public ChecksumVerificationException(String s) {

plugin/src/main/java/io/snyk/languageserver/download/LsDownloader.java

@@ -45,7 +45,7 @@ public LsDownloader(HttpClientFactory factory, LsRuntimeEnvironment environment,
 		}
 	}
 
-	public void download(IProgressMonitor monitor) {
+	public void download(IProgressMonitor monitor) throws IOException, ChecksumVerificationException {
 		File destinationFile = new File(Preferences.getInstance().getCliPath());
 		File tempFile = null;
 		try {
@@ -82,7 +82,8 @@ public void download(IProgressMonitor monitor) {
 
 			monitor.subTask("Starting download of CLI version " + version);
 			LsDownloadRequest binaryRequest = new LsDownloadRequest(version, runtimeEnvironment);
-			tempFile = httpClient.execute(binaryRequest, new FileDownloadResponseHandler(tempFile, monitor), context);
+			String downloadUrl = binaryRequest.getURI().toString();
+			tempFile = httpClient.execute(binaryRequest, new FileDownloadResponseHandler(tempFile, monitor, downloadUrl), context);
 			monitor.worked(80);
 			logger.info("LS: Downloaded file.");
 
@@ -113,17 +114,12 @@ public void download(IProgressMonitor monitor) {
 			monitor.worked(5);
 			if (!destinationFile.setExecutable(true))
 				throw new IOException("Could not set executable permission on " + destinationFile);
-		} catch (ChecksumVerificationException e) {
-			throw e;
-		} catch (Exception e) {
-			logger.error("IOException", e);
-			throw new RuntimeException(e);
 		} finally {
 			try {
-				if (tempFile != null && tempFile.exists())
-					if (!tempFile.delete())
-						tempFile.deleteOnExit();
-			} catch (Exception e) {
+				if (tempFile != null && tempFile.exists() && !tempFile.delete()) {
+					tempFile.deleteOnExit();
+				}
+			} catch (Exception e) { // NOPMD - intentional catch-all for cleanup
 				SnykLogger.logError(e);
 			}
 		}
@@ -144,7 +140,7 @@ String getVersion(String releaseChannel) {
 		return response;
 	}
 
-	String getSha(String version) {
+	String getSha(String version) throws ChecksumVerificationException {
 		LsShaRequest shaRequest = new LsShaRequest(version);
 		try (CloseableHttpResponse response = httpClient.execute(shaRequest, context)) {
 			if (response.getStatusLine().getStatusCode() >= 400) { // NOPMD, AvoidLiteralsInIfCondition
@@ -164,7 +160,7 @@ String getSha(String version) {
 		}
 	}
 
-	private void verifyChecksum(String expectedSha, byte[] checksumDownloadedFile) {
+	private void verifyChecksum(String expectedSha, byte[] checksumDownloadedFile) throws ChecksumVerificationException {
 		try {
 			byte[] sha = MessageDigest.getInstance("SHA-256").digest(checksumDownloadedFile);
 			String actualSha = bytesToHex(sha).toLowerCase(Locale.getDefault());

tests/src/test/java/io/snyk/eclipse/plugin/utils/FileDownloadResponseHandlerTest.java

@@ -1,6 +1,5 @@
 package io.snyk.eclipse.plugin.utils;
 
-import io.snyk.languageserver.LsBaseTest;
 import org.apache.http.HttpEntity;
 import org.apache.http.ProtocolVersion;
 import org.apache.http.entity.BasicHttpEntity;
@@ -15,27 +14,59 @@
 import java.nio.file.Files;
 
 import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertThrows;
+import static org.junit.jupiter.api.Assertions.assertTrue;
 import static org.mockito.Mockito.mock;
 
-class FileDownloadResponseHandlerTest extends LsBaseTest {
+class FileDownloadResponseHandlerTest {
+
+  private static final String TEST_URL = "https://downloads.snyk.io/cli/v1.0.0/snyk-win-arm64.exe";
 
   @Test
   void shouldSaveFile() throws IOException {
     String payload = "test test test";
-    BasicHttpResponse response = getBasicHttpResponse(payload);
+    BasicHttpResponse response = getBasicHttpResponse(200, "OK", payload);
 
     File tempFile = File.createTempFile("pre", "fix");
     tempFile.deleteOnExit();
-    var cut = new FileDownloadResponseHandler(tempFile, mock(IProgressMonitor.class));
+    var cut = new FileDownloadResponseHandler(tempFile, mock(IProgressMonitor.class), TEST_URL);
     cut.handleResponse(response);
 
     var actual = Files.readString(tempFile.toPath());
     assertEquals(payload, actual);
   }
 
-  private BasicHttpResponse getBasicHttpResponse(String payload) {
+  @Test
+  void shouldThrowOnHttp404WithUrlAndStatus() throws IOException {
+    String errorBody = "<html>Not Found</html>";
+    BasicHttpResponse response = getBasicHttpResponse(404, "Not Found", errorBody);
+
+    File tempFile = File.createTempFile("pre", "fix");
+    tempFile.deleteOnExit();
+    var cut = new FileDownloadResponseHandler(tempFile, mock(IProgressMonitor.class), TEST_URL);
+
+    IOException exception = assertThrows(IOException.class, () -> cut.handleResponse(response));
+    assertTrue(exception.getMessage().contains("404"), "Should contain status code");
+    assertTrue(exception.getMessage().contains(TEST_URL), "Should contain URL");
+  }
+
+  @Test
+  void shouldThrowOnHttp500WithUrlAndStatus() throws IOException {
+    String errorBody = "Internal Server Error";
+    BasicHttpResponse response = getBasicHttpResponse(500, "Internal Server Error", errorBody);
+
+    File tempFile = File.createTempFile("pre", "fix");
+    tempFile.deleteOnExit();
+    var cut = new FileDownloadResponseHandler(tempFile, mock(IProgressMonitor.class), TEST_URL);
+
+    IOException exception = assertThrows(IOException.class, () -> cut.handleResponse(response));
+    assertTrue(exception.getMessage().contains("500"), "Should contain status code");
+    assertTrue(exception.getMessage().contains(TEST_URL), "Should contain URL");
+  }
+
+  private BasicHttpResponse getBasicHttpResponse(int statusCode, String reasonPhrase, String payload) {
     var response =
-      new BasicHttpResponse(new BasicStatusLine(new ProtocolVersion("https", 1, 1), 200, "OK"));
+      new BasicHttpResponse(new BasicStatusLine(new ProtocolVersion("https", 1, 1), statusCode, reasonPhrase));
     HttpEntity entity = getHttpEntity(payload);
     response.setEntity(entity);
     return response;