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

Fix reporting of wrong download as failed #4236

Merged
merged 1 commit into from
Oct 18, 2024

Conversation

HebaruSan
Copy link
Member

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.

@HebaruSan HebaruSan added Bug Something is not working as intended Core (ckan.dll) Issues affecting the core part of CKAN Network Issues affecting internet connections of CKAN labels Oct 18, 2024
@HebaruSan HebaruSan merged commit 3efbeb6 into KSP-CKAN:master Oct 18, 2024
3 checks passed
@HebaruSan HebaruSan deleted the fix/failed-dl-index branch October 18, 2024 21:21
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 Core (ckan.dll) Issues affecting the core part of CKAN Network Issues affecting internet connections of CKAN
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant