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

Don't fail if the user is offline #1898

Merged
merged 10 commits into from
Aug 15, 2024

Conversation

nagilson
Copy link
Member

@nagilson nagilson commented Jul 30, 2024

Resolves #1866
#1858 (does not add api, we decide to do this automatically)

The VersionResolver class makes a web-request before downloading .NET to find the newest version of .NET to keep users up to date. The problem is, this resolution of which version of .NET to download occurs before when we check the pre-existing installations to see if we need to do an installation at all, because we need to resolve a version string such as '8.0' to '8.0.2'.

If the user had downloaded the .NET before hand then went on an airplane, which many have, it would not check if that install exists because it'd first see if there is a newer version to download then fail.

This adds a check where if we are offline we check the preexisting installs first to prevent this failure from occurring.

Previous Behavior:

It would wait for the timeout silently and then fail.
See users reporting this in microsoft/vscode-dotnettools#1028 which this resolves.

New Behavior:

image

For the first 2 requests, I request '7.0' which was installed previously. It uses the old existing install and remarks to update when possible.

For the next request, I request '8.0' which was not installed previously. It has a new warning message instead of hanging and will wait..

When connecting back to the internet, it works again.

@nagilson nagilson marked this pull request as ready for review August 14, 2024 20:58
@nagilson nagilson requested review from smitpatel and a team August 15, 2024 18:03
Copy link

@smitpatel smitpatel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please split the function to handle command in different files. It is making harder to review changes in extension.ts file as it is hard to find which function is getting modified.

@nagilson
Copy link
Member Author

Thanks for your review, you caught some important issues. For the refactor, I've made a separate issue to track this so it can be done in another PR: #1917

@nagilson nagilson enabled auto-merge (squash) August 15, 2024 19:24
@nagilson nagilson merged commit 20261b5 into dotnet:main Aug 15, 2024
7 checks passed
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 this pull request may close these issues.

Offline detection/install short circuiting
2 participants