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

Retry of failed downloads #2277

Merged
merged 6 commits into from
Feb 13, 2018

Conversation

HebaruSan
Copy link
Member

@HebaruSan HebaruSan commented Feb 6, 2018

Problem

If you try to install mods in GUI and one or more downloads errors out, it's not handled well.

CKAN version Outcome
1.22.6 and earlier DownloadErrorsKraken thrown and caught by the uncaught exceptions handler, effectively crashing the app
1.24.0-PRE-1 (post-#2233) Error raised, user can cancel and retry, but retrying doesn't attempt to download or install anything

Cause

Before #2233, GUI's downloads happened on this line:

installer.EnsureCache(resolvedMods.UncachedModules);

That function can throw DownloadErrorsKrakens, and there's nothing there to catch them.

After #2233, that exception is handled, but the user's change set was still being lost in a few ways:

  • The code to install was calling UpdateChangesDialog with a null parameter immediately after starting the install. This cleared the Main.changeSet variable and the grid for previewing changes.
  • The post-install code was calling UpdateModsList regardless of success or failure, which rebuilds the GUIMod list from the registry. Unfortunately since each GUIMod tracks whether it is selected to install/remove/upgrade, and clicking Apply Changes rebuilds the change set from the GUIMod list, this also wiped the user's change set.

CKAN/GUI/MainChangeset.cs

Lines 116 to 122 in bb02892

//Using the changeset passed in can cause issues with versions.
// An example is Mechjeb for FAR at 25/06/2015 with a 1.0.2 install.
// TODO Work out why this is.
var user_change_set = mainModList.ComputeUserChangeSet().ToList();
installWorker.RunWorkerAsync(
new KeyValuePair<List<ModChange>, RelationshipResolverOptions>(
user_change_set, install_ops));

Changes

Handling download exceptions

#2233 can be taken as a first step towards fixing this; it moved the download process into a try{} block that catches DownloadErrorsKraken, so the user isn't confronted with a scary Continue/Quit exception popup.

Preserve change set on failure

Now we take the next step: we don't reset the change set until the end of the install attempt. If the install attempt is successful, we clear the change set and rebuild the list of GUIMods, since we are done with those changes. If the attempt fails, we keep the change set and also keep the current list of GUIMods, since they are used to regenerate the change set at the start of the next install attempt (see the code block above).

Fixes #2274.

Retry button after failure

Building on these improvements, a Retry button now appears next to Cancel when an install fails, and clicking it restarts the previously attempted install process.

Consolidated download error popup

Download error reporting is now more complete and user friendly. Rather than raising up to three popups in a row for download errors and ignoring remaining errors, GUI now shows all download errors in a single popup and adds them to the progress tab's log box as well for closer examination after the popup is closed. These errors are also now formatted better in Cmdline and ConsoleUI.

image

image

Host and size shown before downloading

We also now show the download host and size before each point where the user needs to decide whether to continue in GUI:

image
image
image

Jump to auth token screen if a download is throttled

If a user encounters the dreaded GitHub 403 response due to download throttling, we will notify them specifically that the download was throttled, and advise them that adding an authentication token might help. Cmdline says to try the authtoken command, and ConsoleUI and GUI both provide a yes/no popup to jump to their respective auth token editing screens. If the user selects yes, the GitHub auth token wiki is opened in the system browser as well.

Fixes #1025.
Part of addressing #2210.

Don't report success after repo update fails

#1579 contains this gem:

image

A failure message followed by a success message. Now repo update failure will be reported as a failure and only a failure.

Fixes #1389.

Make Cancel button always work

Previously, the Cancel button on the progress screen in GUI only worked for module downloads, and not for repo updates. You could click it, but nothing happened. Now it always takes you back to the mod list.

@HebaruSan HebaruSan added Bug Something is not working as intended GUI Issues affecting the interactive GUI Pull request Network Issues affecting internet connections of CKAN labels Feb 6, 2018
@HebaruSan HebaruSan force-pushed the fix/retry-failed-downloads branch 2 times, most recently from 5c745cc to 923f069 Compare February 7, 2018 01:21
@HebaruSan HebaruSan changed the title Make retry work if downloads fail Retry of failed downloads Feb 7, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Something is not working as intended GUI Issues affecting the interactive GUI Network Issues affecting internet connections of CKAN
Projects
None yet
2 participants