KSP-CKAN / CKAN

The Comprehensive Kerbal Archive Network
https://forum.kerbalspaceprogram.com/index.php?/topic/197082-*
Other
1.99k stars 348 forks source link

Fix reporting of wrong download as failed #4236

Closed HebaruSan closed 1 month ago

HebaruSan commented 1 month ago

Problem

With archive.org down, taniwha's mods hosted only there (before KSP-CKAN/NetKAN#10262 and KSP-CKAN/NetKAN#10263), and the life boat modpack depending on Extraplanetary Launchpads, our failed downloads handling has been undergoing a comprehensive stress test.

KSP-CKAN/NetKAN#10262's validation log reported a download failure for KIS, after reporting that KIS downloaded successfully:

https://github.com/KSP-CKAN/NetKAN/actions/runs/11407171543/job/31743515317

  Downloading https://archive.org/download/KerbalStats-3.1.0/0E71BD9E-KerbalStats-3.1.0.zip ...

...

  Finished downloading KIS 1.29, validating and storing to cache...

  0 B/sec - downloading - 3.1 MiB left - 93%           
  Downloading https://spacedock.info/mod/1987/Kerbal%20Attachment%20System%20(KAS)/download/1.12 ...
  Finished downloading InfernalRoboticsRealismOverhaul 2.3.1, validating and storing to cache...
  Finished downloading KAS 1.12, validating and storing to cache...
  Error: 13138 [1] ERROR CKAN.CmdLine.ConsoleUser (null) - One or more downloads were unsuccessful:

  Error downloading KIS 1.29: The remote server returned an error: (503) Service Temporarily Unavailable.

Meanwhile KerbalStats is downloading from the unavailable archive.org and just disappears from the log. That's the mod that actually failed to download.

Cause

NetAsyncDownloader reports download failures by throwing DownloadErrorsKraken which contains index/exception pairs, where the index is supposed to correspond to the original list of DownloadTargets that was passed in. However, the index it actually uses comes from the internal array of DownloadParts, which is built in the order that the downloads are started, which will not be the same as the targets if there is a mix of hosts. So the calling code gets scrambled info when trying to understand the exception.

Changes

Now DownloadErrorsKraken contains pairs of DownloadTargets and exceptions, which both come directly from the same DownloadPart object that had the failure. No indexes are involved anymore, so there's nothing to confuse. All calling code is updated to examine the target objects where it previously looked at the indexes.