Merge #4236 Fix reporting of wrong download as failed

KSP-CKAN · Oct 18, 2024 · 3efbeb6 · 3efbeb6
2 parents 8f384bf + 8159fe7
commit 3efbeb6
Show file tree

Hide file tree

Showing 7 changed files with 48 additions and 38 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -14,6 +14,7 @@ All notable changes to this project will be documented in this file.
 - [GUI] Set focus better on Ctrl+F (#4230 by: HebaruSan)
 - [Multiple] Refactor relationship resolver to capture full resolved tree (#4232 by: HebaruSan)
 - [GUI] Fix skipping failed dependency downloads (#4235 by: HebaruSan)
+- [Core] Fix reporting of wrong download as failed (#4236 by: HebaruSan)
 
 ### Internal
 

diff --git a/Core/Net/NetAsyncDownloader.cs b/Core/Net/NetAsyncDownloader.cs
@@ -134,10 +134,10 @@ public void DownloadAndWait(IList<DownloadTarget> targets)
             }
 
             // Check to see if we've had any errors. If so, then release the kraken!
-            var exceptions = downloads.Select((dl, i) => dl.error is Exception exc
-                                                         ? new KeyValuePair<int, Exception>(i, exc)
-                                                         : (KeyValuePair<int, Exception>?)null)
-                                      .OfType<KeyValuePair<int, Exception>>()
+            var exceptions = downloads.Select(dl => dl.error != null
+                                                        ? new KeyValuePair<DownloadTarget, Exception>(dl.target, dl.error)
+                                                        : (KeyValuePair<DownloadTarget, Exception>?)null)
+                                      .OfType<KeyValuePair<DownloadTarget, Exception>>()
                                       .ToList();
 
             if (exceptions.Select(kvp => kvp.Value)
@@ -153,7 +153,7 @@ public void DownloadAndWait(IList<DownloadTarget> targets)
                                                      && wex.Response is HttpWebResponse hresp
                                                      // Handle HTTP 403 used for throttling
                                                      && hresp.StatusCode == HttpStatusCode.Forbidden
-                                                     && downloads[kvp.Key].CurrentUri is Uri url
+                                                     && kvp.Key.urls.LastOrDefault() is Uri url
                                                      && url.IsAbsoluteUri
                                                      && Net.ThrottledHosts.TryGetValue(url.Host, out Uri? infoUrl)
                                                      && infoUrl is not null

diff --git a/Core/Net/NetAsyncModulesDownloader.cs b/Core/Net/NetAsyncModulesDownloader.cs
@@ -81,24 +81,31 @@ public void DownloadModules(IEnumerable<CkanModule> modules)
             // Add all the requested modules
             this.modules.AddRange(moduleGroups.SelectMany(grp => grp));
 
+            var preferredHosts = ServiceLocator.Container.Resolve<IConfiguration>().PreferredHosts;
+            var targets = moduleGroups
+                // Skip any group that already has a URL in progress
+                .Where(grp => grp.All(mod => mod.download?.All(dlUri => !activeURLs.Contains(dlUri)) ?? false))
+                // Each group gets one target containing all the URLs
+                .Select(grp => TargetFromModuleGroup(grp, preferredHosts))
+                .ToArray();
             try
             {
                 cancelTokenSrc = new CancellationTokenSource();
-                var preferredHosts = ServiceLocator.Container.Resolve<IConfiguration>().PreferredHosts;
                 // Start the downloads!
-                downloader.DownloadAndWait(moduleGroups
-                    // Skip any group that already has a URL in progress
-                    .Where(grp => grp.All(mod => mod.download?.All(dlUri => !activeURLs.Contains(dlUri)) ?? false))
-                    // Each group gets one target containing all the URLs
-                    .Select(grp => TargetFromModuleGroup(grp, preferredHosts))
-                    .ToArray());
+                downloader.DownloadAndWait(targets);
                 this.modules.Clear();
                 AllComplete?.Invoke();
             }
             catch (DownloadErrorsKraken kraken)
             {
                 // Associate the errors with the affected modules
-                var exc = new ModuleDownloadErrorsKraken(this.modules.ToList(), kraken);
+                // Find a module for each target
+                var targetModules = targets.Select(t => this.modules
+                                                            .First(m => m.download?.Intersect(t.urls)
+                                                                                   .Any()
+                                                                                  ?? false))
+                                           .ToList();
+                var exc = new ModuleDownloadErrorsKraken(targetModules, kraken);
                 // Clear this.modules because we're done with these
                 this.modules.Clear();
                 throw exc;

diff --git a/Core/Repositories/RepositoryDataManager.cs b/Core/Repositories/RepositoryDataManager.cs
@@ -210,7 +210,7 @@ public UpdateResult Update(Repository[]       repos,
                     catch (UnsupportedKraken kraken)
                     {
                         // Show parsing errors in the Downloads Failed popup
-                        throw new DownloadErrorsKraken(Array.IndexOf(toUpdate, repo), kraken);
+                        throw new DownloadErrorsKraken(target, kraken);
                     }
                     progress.NextFile();
                 }
@@ -220,15 +220,10 @@ public UpdateResult Update(Repository[]       repos,
                 // Fire an event so affected registry objects can clear their caches
                 Updated?.Invoke(toUpdate);
             }
-            catch (DownloadErrorsKraken exc)
+            catch (DownloadErrorsKraken)
             {
                 loadETags();
-                throw new DownloadErrorsKraken(
-                    // Renumber the exceptions based on the original repo list
-                    exc.Exceptions.Select(kvp => new KeyValuePair<int, Exception>(
-                                                     Array.IndexOf(repos, toUpdate[kvp.Key]),
-                                                     kvp.Value))
-                                  .ToList());
+                throw;
             }
             catch (Exception exc)
             {

diff --git a/Core/Types/Kraken.cs b/Core/Types/Kraken.cs
@@ -324,24 +324,23 @@ public FileExistsKraken(string filename, string? reason = null, Exception? inner
     /// </summary>
     public class DownloadErrorsKraken : Kraken
     {
-        public readonly List<KeyValuePair<int, Exception>> Exceptions
-            = new List<KeyValuePair<int, Exception>>();
-
-        public DownloadErrorsKraken(List<KeyValuePair<int, Exception>> errors)
+        public DownloadErrorsKraken(List<KeyValuePair<NetAsyncDownloader.DownloadTarget, Exception>> errors)
             : base(string.Join(Environment.NewLine,
                                new string[] { Properties.Resources.KrakenDownloadErrorsHeader, "" }
                                .Concat(errors.Select(e => e.Value.Message))))
         {
-            Exceptions = new List<KeyValuePair<int, Exception>>(errors);
+            Exceptions = new List<KeyValuePair<NetAsyncDownloader.DownloadTarget, Exception>>(errors);
         }
 
-        public DownloadErrorsKraken(int index, Exception exc)
+        public DownloadErrorsKraken(NetAsyncDownloader.DownloadTarget target, Exception exc)
         {
-            Exceptions = new List<KeyValuePair<int, Exception>>
+            Exceptions = new List<KeyValuePair<NetAsyncDownloader.DownloadTarget, Exception>>
             {
-                new KeyValuePair<int, Exception>(index, exc),
+                new KeyValuePair<NetAsyncDownloader.DownloadTarget, Exception>(target, exc),
             };
         }
+
+        public readonly List<KeyValuePair<NetAsyncDownloader.DownloadTarget, Exception>> Exceptions;
     }
 
     /// <summary>
@@ -350,9 +349,6 @@ public DownloadErrorsKraken(int index, Exception exc)
     /// </summary>
     public class ModuleDownloadErrorsKraken : Kraken
     {
-        public readonly List<KeyValuePair<CkanModule, Exception>> Exceptions
-            = new List<KeyValuePair<CkanModule, Exception>>();
-
         /// <summary>
         /// Initialize the exception.
         /// </summary>
@@ -361,11 +357,16 @@ public readonly List<KeyValuePair<CkanModule, Exception>> Exceptions
         public ModuleDownloadErrorsKraken(IList<CkanModule> modules, DownloadErrorsKraken kraken)
             : base()
         {
-            foreach (var kvp in kraken.Exceptions)
+            foreach ((NetAsyncDownloader.DownloadTarget target, Exception exc) in kraken.Exceptions)
             {
-                Exceptions.Add(new KeyValuePair<CkanModule, Exception>(
-                    modules[kvp.Key], kvp.Value.GetBaseException() ?? kvp.Value
-                ));
+                foreach (var module in modules.Where(m => m.download?.Intersect(target.urls)
+                                                                     .Any()
+                                                                    ?? false))
+                {
+                    Exceptions.Add(new KeyValuePair<CkanModule, Exception>(
+                        module,
+                        exc.GetBaseException() ?? exc));
+                }
             }
         }
 
@@ -394,6 +395,9 @@ public override string ToString()
             return builder.ToString();
         }
 
+        public readonly List<KeyValuePair<CkanModule, Exception>> Exceptions
+            = new List<KeyValuePair<CkanModule, Exception>>();
+
         private StringBuilder? builder = null;
     }
 

diff --git a/GUI/Main/MainRepo.cs b/GUI/Main/MainRepo.cs
@@ -137,7 +137,9 @@ private void UpdateRepo(object? sender, DoWorkEventArgs? e)
                                     Properties.Resources.RepoDownloadsFailedColHdr,
                                     Properties.Resources.RepoDownloadsFailedAbortBtn,
                                     k.Exceptions.Select(kvp => new KeyValuePair<object[], Exception>(
-                                        new object[] { repos[kvp.Key] }, kvp.Value)),
+                                        repos.Where(r => kvp.Key.urls.Contains(r.uri))
+                                             .ToArray(),
+                                        kvp.Value)),
                                     // Rows are only linked to themselves
                                     (r1, r2) => r1 == r2);
                                 dfd.ShowDialog(this);

diff --git a/Tests/Core/Net/NetAsyncDownloaderTests.cs b/Tests/Core/Net/NetAsyncDownloaderTests.cs
@@ -206,7 +206,8 @@ public void DownloadAndWait_WithSomeInvalidUrls_ThrowsDownloadErrorsKraken(
                 foreach (var kvp in exception?.Exceptions!)
                 {
                     var baseExc = kvp.Value.GetBaseException() as FileNotFoundException;
-                    Assert.AreEqual(fromPaths[kvp.Key], baseExc?.FileName);
+                    Assert.AreEqual(fromPaths[Array.IndexOf(targets, kvp.Key)],
+                                    baseExc?.FileName);
                 }
                 foreach (var t in validTargets)
                 {