Timeout Installer.Exe #1859
Timeout Installer.Exe #1859
Conversation
This prevents us from having to run commands twice to get the status and stdout. Caution must be taken to make sure that the assumptions here in what is returned as stdout vs stderr are correct. Furthermore, when looking at the code, I saw something that looked just plain incorrect, so I will need to investigate that by adding the TODO before merge
…de-dotnet-runtime into nagilson-fix-eperm
This will allow us to not count times when users never respond to the installer as a failure and or remediate when the installer is taking too long.
…ral command execution
|
LGTM on my own review. |
|
Are you launching the install with UI? Have you considered maybe just running it with /passive. You'll get progress, but not other UI elements |
|
Oh interesting, I just tried it out and /passive will even auto elevate. That's wonderful. I will add that to this, thanks @joeloff! |
| const displayChannelName = '.NET Install Tool'; | ||
| const defaultTimeoutValue = 600; | ||
| const moreInfoUrl = 'https://github.com/dotnet/vscode-dotnet-runtime/blob/main/Documentation/troubleshooting-runtime.md'; |
There was a problem hiding this comment.
Should this always link to main or some static commit? If you go to that URL right now and then hit y, it'll change your address bar to have the commit ID in the URL. It turns into https://github.com/dotnet/vscode-dotnet-runtime/blob/e68687f05ea3f59dc08437bfaa6c4d07774cf953/Documentation/troubleshooting-runtime.md for me right now. You should also use an aka.ms link (let me know if you need help creating them).
There was a problem hiding this comment.
I think it should default to main so it always provides the latest documentation if we ever improve the document.
The aka.ms link idea is a good one :) I will create another issue to do that. I appreciate that you offered to help, I'll see what I can do first! #1867
There was a problem hiding this comment.
The reason to not link to main is that file path changes (or renames) would make the URL no longer valid. But, if you use an aka.ms link, then main is fine as the forwarding link can be updated without issue if the file is moved/renamed.
| @@ -222,14 +222,14 @@ ${axiosBasedError.cause? `Error Cause: ${axiosBasedError.cause!.message}` : ``} | |||
| Please ensure that you are online. | |||
|
|
|||
| If you're on a proxy and disable registry access, you must set the proxy in our extension settings. See https://github.com/dotnet/vscode-dotnet-runtime/blob/main/Documentation/troubleshooting-runtime.md.`); | |||
There was a problem hiding this comment.
Similar comment about the same link in the other file.
vscode-dotnet-runtime-library/src/Acquisition/GlobalInstallerResolver.ts
Outdated
Show resolved
Hide resolved
vscode-dotnet-runtime-library/src/Acquisition/GlobalInstallerResolver.ts
Outdated
Show resolved
Hide resolved
Currently we will wait forever for the .NET installer to complete. This is not correct behavior, especially when users sometimes ignore the installer prompt and leaving it running, it will block our thread from continuing.
This PR adds the timeout option to nodejs
spawn. This sends the SIGTERM response.https://nodejs.org/api/child_process.html#child_processspawncommand-args-options
This also renames a function to be semantically correct.. this change is in a lot of files, but it does not have an impact on anything. Additionally, some functions that can be used more broadly were moved out of the command executor class.
This PR also adds the editorconfig file to standardize repo style conventions. More work will likely need to be done to apply it to files in another PR.