Skip to content

Commit

Permalink
Clean up download error reporting
Browse files Browse the repository at this point in the history
  • Loading branch information
@HebaruSan
HebaruSan committed Feb 7, 2018
1 parent 413a290 commit 923f069
Show file tree
Hide file tree
Showing 6 changed files with 134 additions and 74 deletions.
5 changes: 5 additions & 0 deletions Cmdline/Action/Install.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
2 changes: 2 additions & 0 deletions ConsoleUI/InstallScreen.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
68 changes: 40 additions & 28 deletions Core/Net/NetAsyncDownloader.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -294,20 +294,23 @@ public void DownloadAndWait(ICollection<Net.DownloadTarget> 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<KeyValuePair<int, Exception>> exceptions = new List<KeyValuePair<int, Exception>>();
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<int, Exception>(i, downloads[i].error));
}
}

if (exceptions.Count > 0)
{
throw new DownloadErrorsKraken(exceptions);
Expand All @@ -316,6 +319,11 @@ public void DownloadAndWait(ICollection<Net.DownloadTarget> urls)
// Yay! Everything worked!
}

private static readonly Regex certificatePattern = new Regex(
@"authentication or decryption has failed",
RegexOptions.Compiled
);

/// <summary>
/// <see cref="IDownloader.CancelDownload()"/>
/// This will also call onCompleted with all null arguments.
Expand Down Expand Up @@ -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);
}

}
}
66 changes: 25 additions & 41 deletions Core/Net/NetAsyncModulesDownloader.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -51,20 +51,28 @@ public void DownloadModules(NetModuleCache cache, IEnumerable<CkanModule> 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);
}
}

/// <summary>
Expand All @@ -74,33 +82,13 @@ public void DownloadModules(NetModuleCache cache, IEnumerable<CkanModule> module
/// </summary>
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());
Expand All @@ -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);
Expand Down
63 changes: 59 additions & 4 deletions Core/Types/Kraken.cs
View file
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
using System;
using System.Text;
using System.Collections.Generic;

namespace CKAN
Expand Down Expand Up @@ -218,18 +219,72 @@ public FileExistsKraken(string filename, string reason = null, Exception innerEx
/// </summary>
public class DownloadErrorsKraken : Kraken
{
public List<Exception> exceptions;
public readonly List<KeyValuePair<int, Exception>> exceptions
= new List<KeyValuePair<int, Exception>>();

public DownloadErrorsKraken(IEnumerable<Exception> errors, string reason = null, Exception innerException = null)
: base(reason, innerException)
public DownloadErrorsKraken(List<KeyValuePair<int, Exception>> errors) : base()
{
exceptions = new List<Exception>(errors);
exceptions = new List<KeyValuePair<int, Exception>>(errors);
}

public override string ToString()
{
return "Uh oh, the following things went wrong when downloading...\r\n\r\n" + String.Join("\r\n", exceptions);
}

}

/// <summary>
/// A download errors exception that knows about modules,
/// to make the error message nicer.
/// </summary>
public class ModuleDownloadErrorsKraken : Kraken
{
/// <summary>
/// Initialize the exception.
/// </summary>
/// <param name="modules">List of modules that we tried to download</param>
/// <param name="kraken">Download errors from URL-level downloader</param>
public ModuleDownloadErrorsKraken(IList<CkanModule> modules, DownloadErrorsKraken kraken)
: base()
{
foreach (var kvp in kraken.exceptions)
{
exceptions.Add(new KeyValuePair<CkanModule, Exception>(
modules[kvp.Key], kvp.Value
));
}
}

/// <summary>
/// Generate a user friendly description of this error.
/// </summary>
/// <returns>
/// One or more downloads were unsuccessful:
///
/// Error downloading Astrogator v0.7.8: The remote server returned an error: (404) Not Found.
/// Etc.
/// </returns>
public override string ToString()
{
if (builder == null)
{
builder = new StringBuilder();
builder.AppendLine("One or more downloads were unsuccessful:");
builder.AppendLine("");
foreach (KeyValuePair<CkanModule, Exception> kvp in exceptions)
{
builder.AppendLine(
$"Error downloading {kvp.Key.ToString()}: {kvp.Value.Message}"
);
}
}
return builder.ToString();
}

private readonly List<KeyValuePair<CkanModule, Exception>> exceptions
= new List<KeyValuePair<CkanModule, Exception>>();
private StringBuilder builder = null;
}

/// <summary>
Expand Down
4 changes: 3 additions & 1 deletion GUI/MainInstall.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -194,9 +194,11 @@ 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)
Expand Down

0 comments on commit 923f069

Please sign in to comment.