From 413a29044fdc1c5932331c5fc5b1ffb173b029cc Mon Sep 17 00:00:00 2001 From: Paul Hebble Date: Tue, 6 Feb 2018 07:02:26 +0000 Subject: [PATCH 1/6] Make retry work if downloads fail --- GUI/Main.cs | 1 - GUI/MainChangeset.cs | 13 ++++++------- GUI/MainInstall.cs | 11 +++++++---- GUI/MainModList.cs | 10 +++++----- 4 files changed, 18 insertions(+), 17 deletions(-) diff --git a/GUI/Main.cs b/GUI/Main.cs index 1f00499a73..7b25644414 100644 --- a/GUI/Main.cs +++ b/GUI/Main.cs @@ -974,7 +974,6 @@ await mainModList.ComputeChangeSetFromModList( install_ops ) ); - UpdateChangesDialog(null, installWorker); ShowWaitDialog(); } } diff --git a/GUI/MainChangeset.cs b/GUI/MainChangeset.cs index 728ab9e535..72fcb1429b 100644 --- a/GUI/MainChangeset.cs +++ b/GUI/MainChangeset.cs @@ -32,7 +32,7 @@ public void UpdateChangesDialog(List changeset, BackgroundWorker inst IEnumerable leftOver = changeset.Where(change => change.ChangeType != GUIModChangeType.Remove && change.ChangeType != GUIModChangeType.Update); - + // Now make our list more human-friendly (dependencies for a mod are listed directly // after it.) CreateSortedModList(leftOver); @@ -74,7 +74,7 @@ public void UpdateChangesDialog(List changeset, BackgroundWorker inst /// It arranges the changeset in a human-friendly order /// The requested mod is listed first, it's dependencies right after it /// So we get for example "ModuleRCSFX" directly after "USI Exploration Pack" - /// + /// /// It is very likely that this is forward-compatible with new ChangeTypes's, /// like a a "reconfigure" changetype, but only the future will tell /// @@ -116,13 +116,12 @@ private void ConfirmChangesButton_Click(object sender, EventArgs e) //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, RelationshipResolverOptions>( - user_change_set, install_ops)); - changeSet = null; - - UpdateChangesDialog(null, installWorker); + mainModList.ComputeUserChangeSet().ToList(), + install_ops + ) + ); ShowWaitDialog(); } diff --git a/GUI/MainInstall.cs b/GUI/MainInstall.cs index 99e9a6a194..1df7d80ee8 100644 --- a/GUI/MainInstall.cs +++ b/GUI/MainInstall.cs @@ -301,12 +301,13 @@ private void PostInstallMods(object sender, RunWorkerCompletedEventArgs e) { KeyValuePair result = (KeyValuePair) e.Result; - UpdateModsList(false, result.Value); - tabController.SetTabLock(false); if (result.Key) { + // Rebuilds the list of GUIMods + UpdateModsList(false, result.Value); + if (modChangedCallback != null) { foreach (var mod in result.Value) @@ -320,13 +321,15 @@ private void PostInstallMods(object sender, RunWorkerCompletedEventArgs e) HideWaitDialog(true); tabController.HideTab("ChangesetTabPage"); ApplyToolButton.Enabled = false; + UpdateChangesDialog(null, installWorker); } else { - // there was an error - // rollback user's choices but stay on the log dialog + // There was an error + // Stay on the log dialog and re-apply the user's change set to allow retry AddStatusMessage("Error!"); SetDescription("An error occurred, check the log for information"); + UpdateChangesDialog(result.Value, installWorker); Util.Invoke(DialogProgressBar, () => DialogProgressBar.Style = ProgressBarStyle.Continuous); Util.Invoke(DialogProgressBar, () => DialogProgressBar.Value = 0); } diff --git a/GUI/MainModList.cs b/GUI/MainModList.cs index 453e19f84f..5c80aad8f1 100644 --- a/GUI/MainModList.cs +++ b/GUI/MainModList.cs @@ -653,13 +653,13 @@ public static Dictionary ComputeConflictsFromModList(IRegistryQu public HashSet ComputeUserChangeSet() { - var changes = Modules.Where(mod => mod.IsInstallable()).Select(mod => mod.GetRequestedChange()); - var changeset = new HashSet( - changes.Where(change => change.HasValue). + return new HashSet(Modules. + Where(mod => mod.IsInstallable()). + Select(mod => mod.GetRequestedChange()). + Where(change => change.HasValue). Select(change => change.Value). Select(change => new ModChange(change.Key, change.Value, null)) - ); - return changeset; + ); } } } From 93413b406ab0ec7e1fbdabb715d443c41ba0074a Mon Sep 17 00:00:00 2001 From: Paul Hebble Date: Wed, 7 Feb 2018 01:04:25 +0000 Subject: [PATCH 2/6] Clean up download error reporting --- Cmdline/Action/Install.cs | 5 ++ ConsoleUI/InstallScreen.cs | 2 + Core/Net/NetAsyncDownloader.cs | 68 ++++++++++++++++----------- Core/Net/NetAsyncModulesDownloader.cs | 66 ++++++++++---------------- Core/Types/Kraken.cs | 63 +++++++++++++++++++++++-- GUI/MainInstall.cs | 5 +- 6 files changed, 134 insertions(+), 75 deletions(-) diff --git a/Cmdline/Action/Install.cs b/Cmdline/Action/Install.cs index cb7b01030e..b7f1655f27 100644 --- a/Cmdline/Action/Install.cs +++ b/Cmdline/Action/Install.cs @@ -249,6 +249,11 @@ public int RunCommand(CKAN.KSP ksp, object raw_options) user.RaiseMessage("One or more files failed to download, stopped."); return Exit.ERROR; } + catch (ModuleDownloadErrorsKraken kraken) + { + user.RaiseMessage(kraken.ToString()); + return Exit.ERROR; + } catch (DirectoryNotFoundKraken kraken) { user.RaiseMessage("\r\n{0}", kraken.Message); diff --git a/ConsoleUI/InstallScreen.cs b/ConsoleUI/InstallScreen.cs index a8c11ab13d..c6dcc4510a 100644 --- a/ConsoleUI/InstallScreen.cs +++ b/ConsoleUI/InstallScreen.cs @@ -94,6 +94,8 @@ public override void Run(Action process = null) RaiseError("Game files reverted."); } catch (DownloadErrorsKraken ex) { RaiseError(ex.ToString()); + } catch (ModuleDownloadErrorsKraken ex) { + RaiseError(ex.ToString()); } catch (MissingCertificateKraken ex) { RaiseError(ex.ToString()); } catch (InconsistentKraken ex) { diff --git a/Core/Net/NetAsyncDownloader.cs b/Core/Net/NetAsyncDownloader.cs index ae6bc963d1..b5487265a4 100644 --- a/Core/Net/NetAsyncDownloader.cs +++ b/Core/Net/NetAsyncDownloader.cs @@ -294,20 +294,23 @@ public void DownloadAndWait(ICollection urls) } // Check to see if we've had any errors. If so, then release the kraken! - var exceptions = downloads - .Select(x => x.error) - .Where(ex => ex != null) - .ToList(); - - // Let's check if any of these are certificate errors. If so, - // we'll report that instead, as this is common (and user-fixable) - // under Linux. - if (exceptions.Any(ex => ex is WebException && - Regex.IsMatch(ex.Message, "authentication or decryption has failed"))) + List> exceptions = new List>(); + for (int i = 0; i < downloads.Count; ++i) { - throw new MissingCertificateKraken(); + if (downloads[i].error != null) + { + // Check if it's a certificate error. If so, report that instead, + // as this is common (and user-fixable) under Linux. + if (downloads[i].error is WebException + && certificatePattern.IsMatch(downloads[i].error.Message)) + { + throw new MissingCertificateKraken(); + } + // Otherwise just note the error and which download it came from, + // then throw them all at once later. + exceptions.Add(new KeyValuePair(i, downloads[i].error)); + } } - if (exceptions.Count > 0) { throw new DownloadErrorsKraken(exceptions); @@ -316,6 +319,11 @@ public void DownloadAndWait(ICollection urls) // Yay! Everything worked! } + private static readonly Regex certificatePattern = new Regex( + @"authentication or decryption has failed", + RegexOptions.Compiled + ); + /// /// /// This will also call onCompleted with all null arguments. @@ -409,32 +417,36 @@ private void FileDownloadComplete(int index, Exception error) { log.InfoFormat("Finished downloading {0}", downloads[index].url); } - completed_downloads++; // If there was an error, remember it, but we won't raise it until // all downloads are finished or cancelled. downloads[index].error = error; - if (completed_downloads == downloads.Count) + if (++completed_downloads == downloads.Count) { - log.Info("All files finished downloading"); - - // If we have a callback, then signal that we're done. + FinalizeDownloads(); + } + } - var fileUrls = new Uri[downloads.Count]; - var filePaths = new string[downloads.Count]; - var errors = new Exception[downloads.Count]; + private void FinalizeDownloads() + { + log.Info("All files finished downloading"); - for (int i = 0; i < downloads.Count; i++) - { - fileUrls[i] = downloads[i].url; - filePaths[i] = downloads[i].path; - errors[i] = downloads[i].error; - } + Uri[] fileUrls = new Uri[downloads.Count]; + string[] filePaths = new string[downloads.Count]; + Exception[] errors = new Exception[downloads.Count]; - log.Debug("Signalling completion via callback"); - triggerCompleted(fileUrls, filePaths, errors); + for (int i = 0; i < downloads.Count; ++i) + { + fileUrls[i] = downloads[i].url; + filePaths[i] = downloads[i].path; + errors[i] = downloads[i].error; } + + // If we have a callback, then signal that we're done. + log.Debug("Signalling completion via callback"); + triggerCompleted(fileUrls, filePaths, errors); } + } } diff --git a/Core/Net/NetAsyncModulesDownloader.cs b/Core/Net/NetAsyncModulesDownloader.cs index 38429e2032..eed0fdab3a 100644 --- a/Core/Net/NetAsyncModulesDownloader.cs +++ b/Core/Net/NetAsyncModulesDownloader.cs @@ -51,20 +51,28 @@ public void DownloadModules(NetModuleCache cache, IEnumerable module (_uris, paths, errors) => ModuleDownloadsComplete(cache, _uris, paths, errors); - // Start the downloads! - downloader.DownloadAndWait( - unique_downloads.Select(item => new Net.DownloadTarget( - item.Key, - // Use a temp file name - null, - item.Value.download_size, - // Send the MIME type to use for the Accept header - // The GitHub API requires this to include application/octet-stream - string.IsNullOrEmpty(item.Value.download_content_type) - ? defaultMimeType - : $"{item.Value.download_content_type};q=1.0,{defaultMimeType};q=0.9" - )).ToList() - ); + try + { + // Start the downloads! + downloader.DownloadAndWait( + unique_downloads.Select(item => new Net.DownloadTarget( + item.Key, + // Use a temp file name + null, + item.Value.download_size, + // Send the MIME type to use for the Accept header + // The GitHub API requires this to include application/octet-stream + string.IsNullOrEmpty(item.Value.download_content_type) + ? defaultMimeType + : $"{item.Value.download_content_type};q=1.0,{defaultMimeType};q=0.9" + )).ToList() + ); + } + catch (DownloadErrorsKraken kraken) + { + // Associate the errors with the affected modules + throw new ModuleDownloadErrorsKraken(this.modules, kraken); + } } /// @@ -74,33 +82,13 @@ public void DownloadModules(NetModuleCache cache, IEnumerable module /// private void ModuleDownloadsComplete(NetModuleCache cache, Uri[] urls, string[] filenames, Exception[] errors) { - if (urls != null) + if (filenames != null) { - // spawn up to 3 dialogs - int errorDialogsLeft = 3; - for (int i = 0; i < errors.Length; i++) { - if (errors[i] != null) - { - if (errorDialogsLeft > 0) - { - User.RaiseError("Failed to download \"{0}\" - error: {1}", urls[i], errors[i].Message); - errorDialogsLeft--; - } - } - else + if (errors[i] == null) { - // Even if some of our downloads failed, we want to cache the - // ones which succeeded. - - // This doesn't work :( - // for some reason the tmp files get deleted before we get here and we get a nasty exception - // not only that but then we try _to install_ the rest of the mods and then CKAN crashes - // and the user's registry gets corrupted forever - // commenting out until this is resolved - // ~ nlight - + // Cache the downloads that succeeded. try { cache.Store(modules[i], filenames[i], modules[i].StandardName()); @@ -111,14 +99,10 @@ private void ModuleDownloadsComplete(NetModuleCache cache, Uri[] urls, string[] } } } - } - if (filenames != null) - { // Finally, remove all our temp files. // We probably *could* have used Store's integrated move function above, but if we managed // to somehow get two URLs the same in our download set, that could cause right troubles! - foreach (string tmpfile in filenames) { log.DebugFormat("Cleaning up {0}", tmpfile); diff --git a/Core/Types/Kraken.cs b/Core/Types/Kraken.cs index 17f92499ff..41a695b981 100644 --- a/Core/Types/Kraken.cs +++ b/Core/Types/Kraken.cs @@ -1,4 +1,5 @@ using System; +using System.Text; using System.Collections.Generic; namespace CKAN @@ -218,18 +219,72 @@ public FileExistsKraken(string filename, string reason = null, Exception innerEx /// public class DownloadErrorsKraken : Kraken { - public List exceptions; + public readonly List> exceptions + = new List>(); - public DownloadErrorsKraken(IEnumerable errors, string reason = null, Exception innerException = null) - : base(reason, innerException) + public DownloadErrorsKraken(List> errors) : base() { - exceptions = new List(errors); + exceptions = new List>(errors); } public override string ToString() { return "Uh oh, the following things went wrong when downloading...\r\n\r\n" + String.Join("\r\n", exceptions); } + + } + + /// + /// A download errors exception that knows about modules, + /// to make the error message nicer. + /// + public class ModuleDownloadErrorsKraken : Kraken + { + /// + /// Initialize the exception. + /// + /// List of modules that we tried to download + /// Download errors from URL-level downloader + public ModuleDownloadErrorsKraken(IList modules, DownloadErrorsKraken kraken) + : base() + { + foreach (var kvp in kraken.exceptions) + { + exceptions.Add(new KeyValuePair( + modules[kvp.Key], kvp.Value + )); + } + } + + /// + /// Generate a user friendly description of this error. + /// + /// + /// One or more downloads were unsuccessful: + /// + /// Error downloading Astrogator v0.7.8: The remote server returned an error: (404) Not Found. + /// Etc. + /// + public override string ToString() + { + if (builder == null) + { + builder = new StringBuilder(); + builder.AppendLine("One or more downloads were unsuccessful:"); + builder.AppendLine(""); + foreach (KeyValuePair kvp in exceptions) + { + builder.AppendLine( + $"Error downloading {kvp.Key.ToString()}: {kvp.Value.Message}" + ); + } + } + return builder.ToString(); + } + + private readonly List> exceptions + = new List>(); + private StringBuilder builder = null; } /// diff --git a/GUI/MainInstall.cs b/GUI/MainInstall.cs index 1df7d80ee8..82ca6f8f7f 100644 --- a/GUI/MainInstall.cs +++ b/GUI/MainInstall.cs @@ -194,9 +194,10 @@ private void InstallMods(object sender, DoWorkEventArgs e) // this probably need GUI.user.RaiseMessage(kraken.ToString()); return; } - catch (DownloadErrorsKraken) + catch (ModuleDownloadErrorsKraken kraken) { - // User notified in InstallList + GUI.user.RaiseMessage(kraken.ToString()); + GUI.user.RaiseError(kraken.ToString()); return; } catch (DirectoryNotFoundKraken kraken) From e753c336a0200a5764ff4024efaf97616adb22d4 Mon Sep 17 00:00:00 2001 From: Paul Hebble Date: Wed, 7 Feb 2018 06:54:43 +0000 Subject: [PATCH 3/6] Add a Retry button after failed downloads --- GUI/Main.Designer.cs | 41 +++++++++++++++++++++++++++++------------ GUI/MainChangeset.cs | 1 + GUI/MainDialogs.cs | 8 ++++++-- GUI/MainInstall.cs | 2 ++ GUI/MainWait.cs | 5 +++++ 5 files changed, 43 insertions(+), 14 deletions(-) diff --git a/GUI/Main.Designer.cs b/GUI/Main.Designer.cs index e2684ca55b..38d6eb921a 100644 --- a/GUI/Main.Designer.cs +++ b/GUI/Main.Designer.cs @@ -98,6 +98,7 @@ private void InitializeComponent() this.Reason = ((System.Windows.Forms.ColumnHeader)(new System.Windows.Forms.ColumnHeader())); this.WaitTabPage = new System.Windows.Forms.TabPage(); this.CancelCurrentActionButton = new System.Windows.Forms.Button(); + this.RetryCurrentActionButton = new System.Windows.Forms.Button(); this.LogTextBox = new System.Windows.Forms.TextBox(); this.DialogProgressBar = new System.Windows.Forms.ProgressBar(); this.MessageTextBox = new System.Windows.Forms.TextBox(); @@ -769,26 +770,27 @@ private void InitializeComponent() this.ChangesListView.TabIndex = 4; this.ChangesListView.UseCompatibleStateImageBehavior = false; this.ChangesListView.View = System.Windows.Forms.View.Details; - // + // // Mod - // + // this.Mod.Text = "Mod"; this.Mod.Width = 332; - // + // // ChangeType - // + // this.ChangeType.Text = "Change"; this.ChangeType.Width = 111; - // + // // Reason - // + // this.Reason.Text = "Reason for action"; this.Reason.Width = 606; - // + // // WaitTabPage - // + // this.WaitTabPage.BackColor = System.Drawing.SystemColors.Control; this.WaitTabPage.Controls.Add(this.CancelCurrentActionButton); + this.WaitTabPage.Controls.Add(this.RetryCurrentActionButton); this.WaitTabPage.Controls.Add(this.LogTextBox); this.WaitTabPage.Controls.Add(this.DialogProgressBar); this.WaitTabPage.Controls.Add(this.MessageTextBox); @@ -812,11 +814,25 @@ private void InitializeComponent() this.CancelCurrentActionButton.Text = "Cancel"; this.CancelCurrentActionButton.UseVisualStyleBackColor = true; this.CancelCurrentActionButton.Click += new System.EventHandler(this.CancelCurrentActionButton_Click); - // + // + // RetryCurrentActionButton + // + this.RetryCurrentActionButton.Anchor = ((System.Windows.Forms.AnchorStyles)((System.Windows.Forms.AnchorStyles.Bottom | System.Windows.Forms.AnchorStyles.Right))); + this.RetryCurrentActionButton.FlatStyle = System.Windows.Forms.FlatStyle.Flat; + this.RetryCurrentActionButton.Location = new System.Drawing.Point(1290, 951); + this.RetryCurrentActionButton.Margin = new System.Windows.Forms.Padding(4, 5, 4, 5); + this.RetryCurrentActionButton.Name = "RetryCurrentActionButton"; + this.RetryCurrentActionButton.Size = new System.Drawing.Size(112, 35); + this.RetryCurrentActionButton.TabIndex = 8; + this.RetryCurrentActionButton.Text = "Retry"; + this.RetryCurrentActionButton.UseVisualStyleBackColor = true; + this.RetryCurrentActionButton.Visible = false; + this.RetryCurrentActionButton.Click += new System.EventHandler(this.RetryCurrentActionButton_Click); + // // LogTextBox - // - this.LogTextBox.Anchor = ((System.Windows.Forms.AnchorStyles)((((System.Windows.Forms.AnchorStyles.Top | System.Windows.Forms.AnchorStyles.Bottom) - | System.Windows.Forms.AnchorStyles.Left) + // + this.LogTextBox.Anchor = ((System.Windows.Forms.AnchorStyles)((((System.Windows.Forms.AnchorStyles.Top | System.Windows.Forms.AnchorStyles.Bottom) + | System.Windows.Forms.AnchorStyles.Left) | System.Windows.Forms.AnchorStyles.Right))); this.LogTextBox.BorderStyle = System.Windows.Forms.BorderStyle.FixedSingle; this.LogTextBox.Location = new System.Drawing.Point(14, 89); @@ -1152,6 +1168,7 @@ private void InitializeComponent() private System.Windows.Forms.ColumnHeader Reason; private System.Windows.Forms.TabPage WaitTabPage; private System.Windows.Forms.Button CancelCurrentActionButton; + private System.Windows.Forms.Button RetryCurrentActionButton; private System.Windows.Forms.TextBox LogTextBox; private System.Windows.Forms.ProgressBar DialogProgressBar; private System.Windows.Forms.TextBox MessageTextBox; diff --git a/GUI/MainChangeset.cs b/GUI/MainChangeset.cs index 72fcb1429b..313c660d47 100644 --- a/GUI/MainChangeset.cs +++ b/GUI/MainChangeset.cs @@ -110,6 +110,7 @@ private void ConfirmChangesButton_Click(object sender, EventArgs e) return; menuStrip1.Enabled = false; + RetryCurrentActionButton.Visible = false; RelationshipResolverOptions install_ops = RelationshipResolver.DefaultOpts(); install_ops.with_recommends = false; diff --git a/GUI/MainDialogs.cs b/GUI/MainDialogs.cs index 3dd5e94275..fa323312c8 100644 --- a/GUI/MainDialogs.cs +++ b/GUI/MainDialogs.cs @@ -22,8 +22,12 @@ public void RecreateDialogs() public void AddStatusMessage(string text, params object[] args) { - Util.Invoke(statusStrip1, () => StatusLabel.Text = String.Format(text, args)); - AddLogMessage(String.Format(text, args)); + string msg = String.Format(text, args); + // No newlines in status bar + Util.Invoke(statusStrip1, () => + StatusLabel.Text = msg.Replace("\r\n", " ").Replace("\n", " ") + ); + AddLogMessage(msg); } public void ErrorDialog(string text, params object[] args) diff --git a/GUI/MainInstall.cs b/GUI/MainInstall.cs index 82ca6f8f7f..270835f6d2 100644 --- a/GUI/MainInstall.cs +++ b/GUI/MainInstall.cs @@ -322,6 +322,7 @@ private void PostInstallMods(object sender, RunWorkerCompletedEventArgs e) HideWaitDialog(true); tabController.HideTab("ChangesetTabPage"); ApplyToolButton.Enabled = false; + RetryCurrentActionButton.Visible = false; UpdateChangesDialog(null, installWorker); } else @@ -331,6 +332,7 @@ private void PostInstallMods(object sender, RunWorkerCompletedEventArgs e) AddStatusMessage("Error!"); SetDescription("An error occurred, check the log for information"); UpdateChangesDialog(result.Value, installWorker); + RetryCurrentActionButton.Visible = true; Util.Invoke(DialogProgressBar, () => DialogProgressBar.Style = ProgressBarStyle.Continuous); Util.Invoke(DialogProgressBar, () => DialogProgressBar.Value = 0); } diff --git a/GUI/MainWait.cs b/GUI/MainWait.cs index b9150ff5ca..df31580507 100644 --- a/GUI/MainWait.cs +++ b/GUI/MainWait.cs @@ -90,6 +90,11 @@ public void AddLogMessage(string message) Util.Invoke(LogTextBox, () => LogTextBox.AppendText(message + "\r\n")); } + private void RetryCurrentActionButton_Click(object sender, EventArgs e) + { + tabController.ShowTab("ChangesetTabPage", 1); + } + private void CancelCurrentActionButton_Click(object sender, EventArgs e) { if (cancelCallback != null) From 9852b4cf41f1ae3ba86e2950314f23de9d21c8d3 Mon Sep 17 00:00:00 2001 From: Paul Hebble Date: Wed, 7 Feb 2018 21:26:04 +0000 Subject: [PATCH 4/6] Show host and size in GUI decision points --- ConsoleUI/ModInfoScreen.cs | 2 +- ConsoleUI/ModListScreen.cs | 2 +- .../Toolkit/ConsoleFileMultiSelectDialog.cs | 4 ++-- ConsoleUI/Toolkit/Formatting.cs | 21 ------------------- Core/ModuleInstaller.cs | 7 ++++++- Core/Net/NetAsyncDownloader.cs | 6 +++--- Core/Types/CkanModule.cs | 21 +++++++++++++++++++ GUI/GUIMod.cs | 9 +++----- GUI/Main.Designer.cs | 2 +- GUI/MainChangeset.cs | 7 ++++++- GUI/MainInstall.cs | 10 +++++++-- GUI/SettingsDialog.cs | 8 +++---- 12 files changed, 56 insertions(+), 43 deletions(-) diff --git a/ConsoleUI/ModInfoScreen.cs b/ConsoleUI/ModInfoScreen.cs index 2742dd1802..3da57fbaac 100644 --- a/ConsoleUI/ModInfoScreen.cs +++ b/ConsoleUI/ModInfoScreen.cs @@ -65,7 +65,7 @@ public ModInfoScreen(KSPManager mgr, ChangePlan cp, CkanModule m, bool dbg) )); AddObject(new ConsoleLabel( 13, 5, midL - 2, - () => Formatting.FmtSize(mod.download_size) + () => CkanModule.FmtSize(mod.download_size) )); AddObject(new ConsoleLabel( 3, 6, midL - 2, diff --git a/ConsoleUI/ModListScreen.cs b/ConsoleUI/ModListScreen.cs index ae6fe10635..6af181c455 100644 --- a/ConsoleUI/ModListScreen.cs +++ b/ConsoleUI/ModListScreen.cs @@ -203,7 +203,7 @@ public ModListScreen(KSPManager mgr, bool dbg) // Show total download size of all installed mods AddObject(new ConsoleLabel( 1, -1, searchWidth, - () => $"{Formatting.FmtSize(totalInstalledDownloadSize())} installed", + () => $"{CkanModule.FmtSize(totalInstalledDownloadSize())} installed", null, () => ConsoleTheme.Current.DimLabelFg )); diff --git a/ConsoleUI/Toolkit/ConsoleFileMultiSelectDialog.cs b/ConsoleUI/Toolkit/ConsoleFileMultiSelectDialog.cs index 8a3e02ea01..73b8b3f007 100644 --- a/ConsoleUI/Toolkit/ConsoleFileMultiSelectDialog.cs +++ b/ConsoleUI/Toolkit/ConsoleFileMultiSelectDialog.cs @@ -48,7 +48,7 @@ public ConsoleFileMultiSelectDialog(string title, string startPath, string filPa AddObject(new ConsoleLabel( left + 2, bottom - 1, right - 2, - () => $"{chosenFiles.Count} selected, {Formatting.FmtSize(totalChosenSize())}", + () => $"{chosenFiles.Count} selected, {CkanModule.FmtSize(totalChosenSize())}", () => ConsoleTheme.Current.PopupBg, () => ConsoleTheme.Current.PopupFg )); @@ -231,7 +231,7 @@ private string getLength(FileSystemInfo fi) } else { FileInfo file = fi as FileInfo; if (file != null) { - return Formatting.FmtSize(file.Length); + return CkanModule.FmtSize(file.Length); } else { return dirSize; } diff --git a/ConsoleUI/Toolkit/Formatting.cs b/ConsoleUI/Toolkit/Formatting.cs index e9b6164ef0..a9e96f0189 100644 --- a/ConsoleUI/Toolkit/Formatting.cs +++ b/ConsoleUI/Toolkit/Formatting.cs @@ -99,27 +99,6 @@ public static List WordWrap(string msg, int w) return messageLines; } - /// - /// Format a byte count into readable file size - /// - /// Number of bytes in a file - /// - /// ### bytes or ### KB or ### MB or ### GB - /// - public static string FmtSize(long bytes) - { - const double K = 1024; - if (bytes < K) { - return $"{bytes} B"; - } else if (bytes < K * K) { - return $"{bytes / K :N1} KB"; - } else if (bytes < K * K * K) { - return $"{bytes / K / K :N1} MB"; - } else { - return $"{bytes / K / K / K :N1} GB"; - } - } - } } diff --git a/Core/ModuleInstaller.cs b/Core/ModuleInstaller.cs index 4f3ed0d43c..e87825e9c7 100644 --- a/Core/ModuleInstaller.cs +++ b/Core/ModuleInstaller.cs @@ -168,7 +168,12 @@ public void InstallList(ICollection modules, RelationshipResolverOpt { if (!ksp.Cache.IsCachedZip(module)) { - User.RaiseMessage(" * {0} {1} ({2})", module.name, module.version, module.download.Host); + User.RaiseMessage(" * {0} {1} ({2}, {3})", + module.name, + module.version, + module.download.Host, + CkanModule.FmtSize(module.download_size) + ); downloads.Add(module); } else diff --git a/Core/Net/NetAsyncDownloader.cs b/Core/Net/NetAsyncDownloader.cs index b5487265a4..3924542b7e 100644 --- a/Core/Net/NetAsyncDownloader.cs +++ b/Core/Net/NetAsyncDownloader.cs @@ -395,9 +395,9 @@ private void FileProgressReport(int index, int percent, long bytesDownloaded, lo { // Math.Ceiling was added to avoid showing 0 MiB left when finishing User.RaiseProgress( - String.Format("{0} kbps - downloading - {1:f0} MB left", - totalBytesPerSecond/1024, - Math.Ceiling((double)totalBytesLeft/1024/1024)), + String.Format("{0}/sec - downloading - {1} left", + CkanModule.FmtSize(totalBytesPerSecond), + CkanModule.FmtSize(totalBytesLeft)), totalPercentage); } } diff --git a/Core/Types/CkanModule.cs b/Core/Types/CkanModule.cs index d735ed0e74..2671d6046b 100644 --- a/Core/Types/CkanModule.cs +++ b/Core/Types/CkanModule.cs @@ -622,6 +622,27 @@ public string DescribeInstallStanzas() } return string.Join(", ", descriptions); } + + /// + /// Format a byte count into readable file size + /// + /// Number of bytes in a file + /// + /// ### bytes or ### KB or ### MB or ### GB + /// + public static string FmtSize(long bytes) + { + const double K = 1024; + if (bytes < K) { + return $"{bytes} B"; + } else if (bytes < K * K) { + return $"{bytes / K :N1} KB"; + } else if (bytes < K * K * K) { + return $"{bytes / K / K :N1} MB"; + } else { + return $"{bytes / K / K / K :N1} GB"; + } + } } public class InvalidModuleAttributesException : Exception diff --git a/GUI/GUIMod.cs b/GUI/GUIMod.cs index db8d9a4a15..9abd948f61 100644 --- a/GUI/GUIMod.cs +++ b/GUI/GUIMod.cs @@ -173,12 +173,9 @@ public GUIMod(CkanModule mod, IRegistryQuerier registry, KspVersionCriteria curr Identifier = mod.identifier; - if (mod.download_size == 0) - DownloadSize = "N/A"; - else if (mod.download_size / 1024.0 < 1) - DownloadSize = "1 s.Length > 0).Select(s => s[0]).ToArray()); diff --git a/GUI/Main.Designer.cs b/GUI/Main.Designer.cs index 38d6eb921a..de49513944 100644 --- a/GUI/Main.Designer.cs +++ b/GUI/Main.Designer.cs @@ -554,7 +554,7 @@ private void InitializeComponent() // // SizeCol // - this.SizeCol.HeaderText = "Download (KB)"; + this.SizeCol.HeaderText = "Download"; this.SizeCol.Name = "SizeCol"; this.SizeCol.ReadOnly = true; this.SizeCol.SortMode = System.Windows.Forms.DataGridViewColumnSortMode.Programmatic; diff --git a/GUI/MainChangeset.cs b/GUI/MainChangeset.cs index 313c660d47..323f2c5389 100644 --- a/GUI/MainChangeset.cs +++ b/GUI/MainChangeset.cs @@ -46,7 +46,12 @@ public void UpdateChangesDialog(List changeset, BackgroundWorker inst continue; } - var item = new ListViewItem {Text = String.Format("{0} {1}", change.Mod.Name, change.Mod.Version)}; + ListViewItem item = new ListViewItem() + { + Text = CurrentInstance.Cache.IsCachedZip(change.Mod.ToModule()) + ? $"{change.Mod.Name} {change.Mod.Version} (cached)" + : $"{change.Mod.Name} {change.Mod.Version} ({change.Mod.ToModule()?.download.Host ?? ""}, {change.Mod.DownloadSize})" + }; var sub_change_type = new ListViewItem.ListViewSubItem {Text = change.ChangeType.ToString()}; diff --git a/GUI/MainInstall.cs b/GUI/MainInstall.cs index 270835f6d2..b96d827432 100644 --- a/GUI/MainInstall.cs +++ b/GUI/MainInstall.cs @@ -477,8 +477,14 @@ private void UpdateRecommendedDialog(Dictionary mods, bool s foreach (var pair in mods) { CkanModule module = pair.Key; - ListViewItem item = new ListViewItem {Tag = module, Checked = !suggested, Text = pair.Key.name}; - + ListViewItem item = new ListViewItem() + { + Tag = module, + Checked = !suggested, + Text = CurrentInstance.Cache.IsCachedZip(pair.Key) + ? $"{pair.Key.name} {pair.Key.version} (cached)" + : $"{pair.Key.name} {pair.Key.version} ({pair.Key.download.Host ?? ""}, {CkanModule.FmtSize(pair.Key.download_size)})" + }; ListViewItem.ListViewSubItem recommendedBy = new ListViewItem.ListViewSubItem() { Text = pair.Value }; diff --git a/GUI/SettingsDialog.cs b/GUI/SettingsDialog.cs index 6bb4d61580..93dfdff733 100644 --- a/GUI/SettingsDialog.cs +++ b/GUI/SettingsDialog.cs @@ -78,9 +78,9 @@ private void UpdateCacheInfo() CKANCacheLabel.Text = String.Format ( - "There are currently {0} cached files using {1} MB in total", + "There are currently {0} cached files using {1} in total", m_cacheFileCount, - m_cacheSize / 1024 / 1024 + CkanModule.FmtSize(m_cacheSize) ); } @@ -89,9 +89,9 @@ private void ClearCKANCacheButton_Click(object sender, EventArgs e) YesNoDialog deleteConfirmationDialog = new YesNoDialog(); string confirmationText = String.Format ( - "Do you really want to delete {0} cached files, freeing {1} MB?", + "Do you really want to delete {0} cached files, freeing {1}?", m_cacheFileCount, - m_cacheSize / 1024 / 1024 + CkanModule.FmtSize(m_cacheSize) ); if (deleteConfirmationDialog.ShowYesNoDialog(confirmationText) == System.Windows.Forms.DialogResult.Yes) From 990e9239a47ee052c1fcfc29a67161800bd8bcfc Mon Sep 17 00:00:00 2001 From: Paul Hebble Date: Fri, 9 Feb 2018 00:33:51 +0000 Subject: [PATCH 5/6] Show special message for GitHub 403 with option to jump to auth tokens --- Cmdline/Action/Install.cs | 6 ++++++ ConsoleUI/InstallScreen.cs | 7 +++++++ Core/Net/Net.cs | 8 ++++++++ Core/Net/NetAsyncDownloader.cs | 20 +++++++++++++++++--- Core/Types/Kraken.cs | 18 ++++++++++++++++++ GUI/MainInstall.cs | 23 +++++++++++++++++++++++ 6 files changed, 79 insertions(+), 3 deletions(-) diff --git a/Cmdline/Action/Install.cs b/Cmdline/Action/Install.cs index b7f1655f27..9027fa0942 100644 --- a/Cmdline/Action/Install.cs +++ b/Cmdline/Action/Install.cs @@ -244,6 +244,12 @@ public int RunCommand(CKAN.KSP ksp, object raw_options) user.RaiseMessage(kraken.ToString()); return Exit.ERROR; } + catch (DownloadThrottledKraken kraken) + { + user.RaiseMessage(kraken.ToString()); + user.RaiseMessage($"Try the authtoken command. See {kraken.infoUrl} for details."); + return Exit.ERROR; + } catch (DownloadErrorsKraken) { user.RaiseMessage("One or more files failed to download, stopped."); diff --git a/ConsoleUI/InstallScreen.cs b/ConsoleUI/InstallScreen.cs index c6dcc4510a..f0b28f8384 100644 --- a/ConsoleUI/InstallScreen.cs +++ b/ConsoleUI/InstallScreen.cs @@ -96,6 +96,13 @@ public override void Run(Action process = null) RaiseError(ex.ToString()); } catch (ModuleDownloadErrorsKraken ex) { RaiseError(ex.ToString()); + } catch (DownloadThrottledKraken ex) { + if (RaiseYesNoDialog($"{ex.ToString()}\n\nEdit authentication tokens now?")) { + if (ex.infoUrl != null) { + ModInfoScreen.LaunchURL(ex.infoUrl); + } + LaunchSubScreen(new AuthTokenScreen()); + } } catch (MissingCertificateKraken ex) { RaiseError(ex.ToString()); } catch (InconsistentKraken ex) { diff --git a/Core/Net/Net.cs b/Core/Net/Net.cs index c123d9911b..3c1172e4a1 100644 --- a/Core/Net/Net.cs +++ b/Core/Net/Net.cs @@ -23,6 +23,14 @@ public class Net private static readonly ILog Log = LogManager.GetLogger(typeof (Net)); private static readonly TxFileManager FileTransaction = new TxFileManager(); + public static readonly Dictionary ThrottledHosts = new Dictionary() + { + { + "api.github.com", + new Uri("https://github.com/KSP-CKAN/CKAN/wiki/Adding-a-GitHub-API-authtoken") + } + }; + /// /// Downloads the specified url, and stores it in the filename given. /// If no filename is supplied, a temporary file will be generated. diff --git a/Core/Net/NetAsyncDownloader.cs b/Core/Net/NetAsyncDownloader.cs index 3924542b7e..b1b402ecfa 100644 --- a/Core/Net/NetAsyncDownloader.cs +++ b/Core/Net/NetAsyncDownloader.cs @@ -301,10 +301,24 @@ public void DownloadAndWait(ICollection urls) { // Check if it's a certificate error. If so, report that instead, // as this is common (and user-fixable) under Linux. - if (downloads[i].error is WebException - && certificatePattern.IsMatch(downloads[i].error.Message)) + if (downloads[i].error is WebException) { - throw new MissingCertificateKraken(); + WebException wex = downloads[i].error as WebException; + if (certificatePattern.IsMatch(wex.Message)) + { + throw new MissingCertificateKraken(); + } + else switch ((wex.Response as HttpWebResponse)?.StatusCode) + { + // Handle HTTP 403 used for throttling + case HttpStatusCode.Forbidden: + Uri infoUrl; + if (Net.ThrottledHosts.TryGetValue(downloads[i].url.Host, out infoUrl)) + { + throw new DownloadThrottledKraken(downloads[i].url, infoUrl); + } + break; + } } // Otherwise just note the error and which download it came from, // then throw them all at once later. diff --git a/Core/Types/Kraken.cs b/Core/Types/Kraken.cs index 41a695b981..7d137c04e9 100644 --- a/Core/Types/Kraken.cs +++ b/Core/Types/Kraken.cs @@ -1,4 +1,5 @@ using System; +using System.Net; using System.Text; using System.Collections.Generic; @@ -380,6 +381,23 @@ public override string ToString() } } + public class DownloadThrottledKraken : Kraken + { + public readonly Uri throttledUrl; + public readonly Uri infoUrl; + + public DownloadThrottledKraken(Uri url, Uri info) : base() + { + throttledUrl = url; + infoUrl = info; + } + + public override string ToString() + { + return $"Download from {throttledUrl.Host} was throttled.\r\nConsider adding an authentication token to increase the throtting limit."; + } + } + public class RegistryInUseKraken : Kraken { public readonly string lockfilePath; diff --git a/GUI/MainInstall.cs b/GUI/MainInstall.cs index b96d827432..ed1d2f6f60 100644 --- a/GUI/MainInstall.cs +++ b/GUI/MainInstall.cs @@ -1,4 +1,5 @@ using System; +using System.Diagnostics; using System.Collections.Generic; using System.ComponentModel; using System.Linq; @@ -194,6 +195,28 @@ private void InstallMods(object sender, DoWorkEventArgs e) // this probably need GUI.user.RaiseMessage(kraken.ToString()); return; } + catch (DownloadThrottledKraken kraken) + { + string msg = kraken.ToString(); + GUI.user.RaiseMessage(msg); + if (YesNoDialog($"{msg}\r\n\r\nOpen settings now?")) + { + // Launch the URL describing this host's throttling practices, if any + if (kraken.infoUrl != null) + { + Process.Start(new ProcessStartInfo() + { + UseShellExecute = true, + FileName = kraken.infoUrl.ToString() + }); + } + // Now pretend they clicked the menu option for the settings + Enabled = false; + settingsDialog.ShowDialog(); + Enabled = true; + } + return; + } catch (ModuleDownloadErrorsKraken kraken) { GUI.user.RaiseMessage(kraken.ToString()); From 17cbcac72cada7ace29fb5fcd454f140ccef534b Mon Sep 17 00:00:00 2001 From: Paul Hebble Date: Fri, 9 Feb 2018 14:27:38 +0000 Subject: [PATCH 6/6] Improve handling of repo update failures --- Core/Net/Repo.cs | 36 +++++++++++++++++++++++++----------- GUI/MainRepo.cs | 20 +++++++++++++------- GUI/MainWait.cs | 4 ++-- 3 files changed, 40 insertions(+), 20 deletions(-) diff --git a/Core/Net/Repo.cs b/Core/Net/Repo.cs index 4e34745ef2..1701b705af 100644 --- a/Core/Net/Repo.cs +++ b/Core/Net/Repo.cs @@ -38,9 +38,18 @@ public static int UpdateAllRepositories(RegistryManager registry_manager, KSP ks { log.InfoFormat("About to update {0}", repository.Value.name); List avail = UpdateRegistry(repository.Value.uri, ksp, user); - log.InfoFormat("Updated {0}", repository.Value.name); - // Merge all the lists - allAvail.AddRange(avail); + if (avail == null) + { + // Report failure if any repo fails, rather than losing half the list. + // UpdateRegistry will have alerted the user to specific errors already. + return 0; + } + else + { + log.InfoFormat("Updated {0}", repository.Value.name); + // Merge all the lists + allAvail.AddRange(avail); + } } // Save allAvail to the registry if we found anything if (allAvail.Count > 0) @@ -48,18 +57,23 @@ public static int UpdateAllRepositories(RegistryManager registry_manager, KSP ks registry_manager.registry.SetAllAvailable(allAvail); // Save our changes. registry_manager.Save(enforce_consistency: false); - } - ShowUserInconsistencies(registry_manager.registry, user); + ShowUserInconsistencies(registry_manager.registry, user); - List metadataChanges = GetChangedInstalledModules(registry_manager.registry); - if (metadataChanges.Count > 0) + List metadataChanges = GetChangedInstalledModules(registry_manager.registry); + if (metadataChanges.Count > 0) + { + HandleModuleChanges(metadataChanges, user, ksp); + } + + // Return how many we got! + return registry_manager.registry.Available(ksp.VersionCriteria()).Count; + } + else { - HandleModuleChanges(metadataChanges, user, ksp); + // Return failure + return 0; } - - // Return how many we got! - return registry_manager.registry.Available(ksp.VersionCriteria()).Count; } /// diff --git a/GUI/MainRepo.cs b/GUI/MainRepo.cs index e8662dc3c0..d8265f6563 100644 --- a/GUI/MainRepo.cs +++ b/GUI/MainRepo.cs @@ -68,8 +68,8 @@ private void UpdateRepo(object sender, DoWorkEventArgs e) { try { - KSP current_instance1 = CurrentInstance; - Repo.UpdateAllRepositories(RegistryManager.Instance(CurrentInstance), current_instance1, GUI.user); + AddStatusMessage("Updating repositories..."); + e.Result = Repo.UpdateAllRepositories(RegistryManager.Instance(CurrentInstance), CurrentInstance, GUI.user); } catch (UriFormatException ex) { @@ -89,11 +89,17 @@ private void UpdateRepo(object sender, DoWorkEventArgs e) private void PostUpdateRepo(object sender, RunWorkerCompletedEventArgs e) { - UpdateModsList(repo_updated: true); - - HideWaitDialog(true); - AddStatusMessage("Repository successfully updated"); - ShowRefreshQuestion(); + if ((e.Result as int? ?? 0) > 0) + { + UpdateModsList(repo_updated: true); + AddStatusMessage("Repositories successfully updated."); + ShowRefreshQuestion(); + HideWaitDialog(true); + } + else + { + AddStatusMessage("Repository update failed!"); + } Util.Invoke(this, SwitchEnabledState); Util.Invoke(this, RecreateDialogs); diff --git a/GUI/MainWait.cs b/GUI/MainWait.cs index df31580507..e37a0e26f9 100644 --- a/GUI/MainWait.cs +++ b/GUI/MainWait.cs @@ -100,9 +100,9 @@ private void CancelCurrentActionButton_Click(object sender, EventArgs e) if (cancelCallback != null) { cancelCallback(); - CancelCurrentActionButton.Enabled = false; - HideWaitDialog(true); } + CancelCurrentActionButton.Enabled = false; + HideWaitDialog(true); } }