fix: improve CLI download error handling [IDE-1690]#353
Conversation
- Add HTTP status code validation in FileDownloadResponseHandler (only accept 2xx 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.
✅ Snyk checks have passed. No issues have been found so far.
💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse. |
| } | ||
| }); | ||
| } catch (RuntimeException e) { | ||
| } catch (IllegalStateException | SWTException e) { |
There was a problem hiding this comment.
This was intentional - a catch-all. maybe better to disable pmd here.
| public File handleResponse(HttpResponse httpResponse) throws IOException { | ||
| int statusCode = httpResponse.getStatusLine().getStatusCode(); | ||
|
|
||
| if (statusCode < 200 || statusCode > 299) { |
There was a problem hiding this comment.
The second part is not correct: >399 would be an error, not >299
There was a problem hiding this comment.
The only way I could imagine us hitting this would be if we got redirected but didn't follow it (bad redirections would throw redirection exceptions). So if we see 3xx then we didn't download the CLI, so I would prefer to treat that as an error.
However, I don't think will ever happen, and rather than debate it I will just allow it and add a comment.
| if (tempFile != null && tempFile.exists() && !tempFile.delete()) { | ||
| tempFile.deleteOnExit(); | ||
| } | ||
| } catch (SecurityException e) { |
There was a problem hiding this comment.
Was also an intentional catch-all exception block. Wouldn't specify the exceptions here.
Also allow 3xx status codes.
Description
Checklist
Screenshots / GIFs