Many improvements for failed downloads

Cmdline/Main.cs

@@ -288,7 +288,10 @@ private static int Gui(GameInstanceManager manager, GuiOptions options, string[]
             // TODO: Sometimes when the GUI exits, we get a System.ArgumentException,
             // but trying to catch it here doesn't seem to help. Dunno why.
 
-            GUI.GUI.Main_(args, manager, options.ShowConsole);
+            // GUI expects its first param to be an identifier, don't confuse it
+            GUI.GUI.Main_(args.Except(new string[] {"--verbose", "--debug", "--show-console"})
+                              .ToArray(),
+                          manager, options.ShowConsole);
 
             return Exit.OK;
         }

ConsoleUI/InstallScreen.cs

@@ -1,6 +1,7 @@
 using System;
 using System.Transactions;
 using System.Collections.Generic;
+
 using CKAN.ConsoleUI.Toolkit;
 
 namespace CKAN.ConsoleUI {

Core/HelpURLs.cs

@@ -17,6 +17,7 @@ public static class HelpURLs
         public const string Filters = "https://github.com/KSP-CKAN/CKAN/pull/3458";
         public const string Labels = "https://github.com/KSP-CKAN/CKAN/pull/2936";
         public const string PlayTime = "https://github.com/KSP-CKAN/CKAN/pull/3543";
+        public const string DownloadsFailed = "https://github.com/KSP-CKAN/CKAN/pull/3635";
 
         public const string Discord = "https://discord.gg/Mb4nXQD";
     }

Core/ModuleInstaller.cs

@@ -4,11 +4,13 @@
 using System.Linq;
 using System.Text.RegularExpressions;
 using System.Transactions;
+
 using ICSharpCode.SharpZipLib.Core;
 using ICSharpCode.SharpZipLib.Zip;
 using log4net;
 using ChinhDo.Transactions.FileManager;
 using Autofac;
+
 using CKAN.Extensions;
 using CKAN.Versioning;
 using CKAN.Configuration;

Core/Net/NetAsyncDownloader.cs

@@ -105,6 +105,7 @@ private void ResetAgent()
         /// </summary>
         public event Action<Net.DownloadTarget, long, long> Progress;
 
+        private readonly object dlMutex = new object();
         private List<NetAsyncDownloaderDownloadPart> downloads = new List<NetAsyncDownloaderDownloadPart>();
         private List<Net.DownloadTarget> queuedDownloads = new List<Net.DownloadTarget>();
         private int completed_downloads;
@@ -143,11 +144,13 @@ private void DownloadModule(Net.DownloadTarget target)
         {
             if (shouldQueue(target))
             {
+                log.DebugFormat("Enqueuing download of {0}", target.url);
                 // Throttled host already downloading, we will get back to this later
                 queuedDownloads.Add(target);
             }
             else
             {
+                log.DebugFormat("Beginning download of {0}", target.url);
                 // We need a new variable for our closure/lambda, hence index = 1+prev max
                 int index = downloads.Count;
 
@@ -167,7 +170,7 @@ private void DownloadModule(Net.DownloadTarget target)
 
                 // And schedule a notification if we're done (or if something goes wrong)
                 dl.Done += (sender, args) =>
-                    FileDownloadComplete(index, args.Error);
+                    FileDownloadComplete(index, args.Error, args.Cancelled);
 
                 // Start the download!
                 dl.Download(dl.target.url, dl.path);
@@ -187,7 +190,9 @@ private bool shouldQueue(Net.DownloadTarget target)
         {
             return downloads.Any(dl =>
                     dl.target.url.Host == target.url.Host
-                    && dl.bytesLeft > 0);
+                    && dl.bytesLeft > 0
+                    // Consider done if already tried and failed
+                    && dl.error == null);
         }
 
         /// <summary>
@@ -196,7 +201,7 @@ private bool shouldQueue(Net.DownloadTarget target)
         /// <param name="urls">The downloads to begin</param>
         public void DownloadAndWait(ICollection<Net.DownloadTarget> urls)
         {
-            if (downloads.Count > completed_downloads)
+            if (downloads.Count + queuedDownloads.Count > completed_downloads)
             {
                 // Some downloads are still in progress, add to the current batch
                 foreach (Net.DownloadTarget target in urls)
@@ -230,6 +235,8 @@ public void DownloadAndWait(ICollection<Net.DownloadTarget> urls)
             // If the user cancelled our progress, then signal that.
             if (old_download_canceled)
             {
+                // Ditch anything we haven't started
+                queuedDownloads.Clear();
                 // Abort all our traditional downloads, if there are any.
                 foreach (var download in downloads.ToList())
                 {
@@ -378,51 +385,55 @@ private void FileProgressReport(int index, int percent, long bytesDownloaded, lo
         /// This method gets called back by `WebClient` when a download is completed.
         /// It in turncalls the onCompleted hook when *all* downloads are finished.
         /// </summary>
-        private void FileDownloadComplete(int index, Exception error)
+        private void FileDownloadComplete(int index, Exception error, bool canceled)
         {
-            if (error != null)
+            // Make sure the threads don't trip on one another
+            lock (dlMutex)
             {
-                log.InfoFormat("Error downloading {0}: {1}", downloads[index].target.url, error);
-
-                // Check whether we were already downloading the fallback url
-                if (!downloads[index].triedFallback && downloads[index].target.fallbackUrl != null)
+                if (error != null)
                 {
-                    log.InfoFormat("Trying fallback URL: {0}", downloads[index].target.fallbackUrl);
-                    // Encode spaces to avoid confusing URL parsers
-                    User.RaiseMessage(Properties.Resources.NetAsyncDownloaderTryingFallback,
-                        downloads[index].target.url.ToString().Replace(" ", "%20"),
-                        downloads[index].target.fallbackUrl.ToString().Replace(" ", "%20")
-                    );
-                    // Try the fallbackUrl
-                    downloads[index].triedFallback = true;
-                    downloads[index].Download(downloads[index].target.fallbackUrl, downloads[index].path);
-                    // Short circuit the completion process so the fallback can run
-                    return;
+                    log.InfoFormat("Error downloading {0}: {1}", downloads[index].target.url, error);
+
+                    // Check whether we were already downloading the fallback url
+                    if (!canceled && !downloads[index].triedFallback && downloads[index].target.fallbackUrl != null)
+                    {
+                        log.InfoFormat("Trying fallback URL: {0}", downloads[index].target.fallbackUrl);
+                        // Encode spaces to avoid confusing URL parsers
+                        User.RaiseMessage(Properties.Resources.NetAsyncDownloaderTryingFallback,
+                            downloads[index].target.url.ToString().Replace(" ", "%20"),
+                            downloads[index].target.fallbackUrl.ToString().Replace(" ", "%20")
+                        );
+                        // Try the fallbackUrl
+                        downloads[index].triedFallback = true;
+                        downloads[index].Download(downloads[index].target.fallbackUrl, downloads[index].path);
+                        // Short circuit the completion process so the fallback can run
+                        return;
+                    }
+                    else
+                    {
+                        downloads[index].error = error;
+                    }
                 }
                 else
                 {
-                    downloads[index].error = error;
+                    log.InfoFormat("Finished downloading {0}", downloads[index].target.url);
                 }
-            }
-            else
-            {
-                log.InfoFormat("Finished downloading {0}", downloads[index].target.url);
-            }
 
-            var next = queuedDownloads.FirstOrDefault(dl =>
-                dl.url.Host == downloads[index].target.url.Host);
-            if (next != null)
-            {
-                // Start this host's next queued download
-                queuedDownloads.Remove(next);
-                DownloadModule(next);
-            }
+                var next = queuedDownloads.FirstOrDefault(dl =>
+                    dl.url.Host == downloads[index].target.url.Host);
+                if (next != null)
+                {
+                    // Start this host's next queued download
+                    queuedDownloads.Remove(next);
+                    DownloadModule(next);
+                }
 
-            onOneCompleted.Invoke(downloads[index].target.url, downloads[index].path, downloads[index].error);
+                onOneCompleted.Invoke(downloads[index].target.url, downloads[index].path, downloads[index].error);
 
-            if (++completed_downloads >= downloads.Count + queuedDownloads.Count)
-            {
-                triggerCompleted();
+                if (++completed_downloads >= downloads.Count + queuedDownloads.Count)
+                {
+                    triggerCompleted();
+                }
             }
         }
     }

Core/Net/NetAsyncModulesDownloader.cs

@@ -91,15 +91,16 @@ public void DownloadModules(IEnumerable<CkanModule> modules)
                             : $"{item.Value.download_content_type};q=1.0,{defaultMimeType};q=0.9"
                     )).ToList()
                 );
-                if (AllComplete != null)
-                {
-                    AllComplete();
-                }
+                this.modules.Clear();
+                AllComplete?.Invoke();
             }
             catch (DownloadErrorsKraken kraken)
             {
                 // Associate the errors with the affected modules
-                throw new ModuleDownloadErrorsKraken(this.modules, kraken);
+                var exc = new ModuleDownloadErrorsKraken(this.modules.ToList(), kraken);
+                // Clear this.modules because we're done with these
+                this.modules.Clear();
+                throw exc;
             }
         }
 

Core/Registry/Registry.cs

@@ -1098,7 +1098,7 @@ IDictionary<string, ModuleVersion> dlc
         )
         {
             modulesToRemove = modulesToRemove.Memoize();
-            origInstalled    = origInstalled.Memoize();
+            origInstalled   = origInstalled.Memoize();
             var dllSet = dlls.ToHashSet();
             // The empty list has no reverse dependencies
             // (Don't remove broken modules if we're only installing)

Core/Types/Kraken.cs

@@ -291,19 +291,19 @@ public FileExistsKraken(string filename, string reason = null, Exception innerEx
     /// </summary>
     public class DownloadErrorsKraken : Kraken
     {
-        public readonly List<KeyValuePair<int, Exception>> exceptions
+        public readonly List<KeyValuePair<int, Exception>> Exceptions
             = new List<KeyValuePair<int, Exception>>();
 
         public DownloadErrorsKraken(List<KeyValuePair<int, Exception>> errors) : base()
         {
-            exceptions = new List<KeyValuePair<int, Exception>>(errors);
+            Exceptions = new List<KeyValuePair<int, Exception>>(errors);
         }
 
         public override string ToString()
         {
             return String.Join("\r\n",
                 new string[] { Properties.Resources.KrakenDownloadErrorsHeader, "" }
-                .Concat(exceptions.Select(e => e.ToString())));
+                .Concat(Exceptions.Select(e => e.ToString())));
         }
     }
 
@@ -313,6 +313,9 @@ public override string ToString()
     /// </summary>
     public class ModuleDownloadErrorsKraken : Kraken
     {
+        public readonly List<KeyValuePair<CkanModule, Exception>> Exceptions
+            = new List<KeyValuePair<CkanModule, Exception>>();
+
         /// <summary>
         /// Initialize the exception.
         /// </summary>
@@ -321,9 +324,9 @@ public class ModuleDownloadErrorsKraken : Kraken
         public ModuleDownloadErrorsKraken(IList<CkanModule> modules, DownloadErrorsKraken kraken)
             : base()
         {
-            foreach (var kvp in kraken.exceptions)
+            foreach (var kvp in kraken.Exceptions)
             {
-                exceptions.Add(new KeyValuePair<CkanModule, Exception>(
+                Exceptions.Add(new KeyValuePair<CkanModule, Exception>(
                     modules[kvp.Key], kvp.Value.GetBaseException() ?? kvp.Value
                 ));
             }
@@ -345,7 +348,7 @@ public override string ToString()
                 builder = new StringBuilder();
                 builder.AppendLine(Properties.Resources.KrakenModuleDownloadErrorsHeader);
                 builder.AppendLine("");
-                foreach (KeyValuePair<CkanModule, Exception> kvp in exceptions)
+                foreach (KeyValuePair<CkanModule, Exception> kvp in Exceptions)
                 {
                     builder.AppendLine(string.Format(
                         Properties.Resources.KrakenModuleDownloadError, kvp.Key.ToString(), kvp.Value.Message));
@@ -354,8 +357,6 @@ public override string ToString()
             return builder.ToString();
         }
 
-        private readonly List<KeyValuePair<CkanModule, Exception>> exceptions
-            = new List<KeyValuePair<CkanModule, Exception>>();
         private StringBuilder builder = null;
     }
 

GUI/CKAN-GUI.csproj

@@ -81,6 +81,12 @@
     <Compile Include="Dialogs\CompatibleGameVersionsDialog.Designer.cs">
       <DependentUpon>CompatibleGameVersionsDialog.cs</DependentUpon>
     </Compile>
+    <Compile Include="Dialogs\DownloadsFailedDialog.cs">
+      <SubType>Form</SubType>
+    </Compile>
+    <Compile Include="Dialogs\DownloadsFailedDialog.Designer.cs">
+      <DependentUpon>DownloadsFailedDialog.cs</DependentUpon>
+    </Compile>
     <Compile Include="Dialogs\EditLabelsDialog.cs">
       <SubType>Form</SubType>
     </Compile>
@@ -572,6 +578,9 @@
     <EmbeddedResource Include="Localization\ja-JP\DeleteDirectories.ja-JP.resx">
       <DependentUpon>..\..\Controls\DeleteDirectories.cs</DependentUpon>
     </EmbeddedResource>
+    <EmbeddedResource Include="Dialogs\DownloadsFailedDialog.resx">
+      <DependentUpon>DownloadsFailedDialog.cs</DependentUpon>
+    </EmbeddedResource>
     <EmbeddedResource Include="Dialogs\EditLabelsDialog.resx">
       <DependentUpon>EditLabelsDialog.cs</DependentUpon>
     </EmbeddedResource>