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

Skip duplicate repo URLs during update #3786

Merged
merged 2 commits into from
Feb 21, 2023

Conversation

HebaruSan
Copy link
Member

@HebaruSan HebaruSan commented Feb 21, 2023

Problem

A user reported this exception at start-up (partially translated from Chinese):

Unhandled exception:
System.ArgumentException: An item with the same key has already been added.
   在 System.ThrowHelper.ThrowArgumentException(ExceptionResource resource)
   在 System.Collections.Generic.Dictionary`2.Insert(TKey key, TValue value, Boolean add)
   在 CKAN.Repo.<>c__DisplayClass0_0.<UpdateAllRepositories>b__1(Uri url, String filename, Exception error, String etag)
   在 CKAN.NetAsyncDownloader.FileDownloadComplete(Int32 index, Exception error, Boolean canceled, String etag)
   在 CKAN.NetAsyncDownloader.<>c__DisplayClass20_0.<DownloadModule>b__1(Object sender, AsyncCompletedEventArgs args, String etag)
   在 System.Net.WebClient.OnDownloadFileCompleted(AsyncCompletedEventArgs e)
   在 CKAN.ResumingWebClient.OnOpenReadCompleted(OpenReadCompletedEventArgs e)
   在 System.Threading.ExecutionContext.RunInternal(ExecutionContext executionContext, ContextCallback callback, Object state, Boolean preserveSyncCtx)
   在 System.Threading.ExecutionContext.Run(ExecutionContext executionContext, ContextCallback callback, Object state, Boolean preserveSyncCtx)
   在 System.Threading.QueueUserWorkItemCallback.System.Threading.IThreadPoolWorkItem.ExecuteWorkItem()
   在 System.Threading.ThreadPoolWorkQueue.Dispatch() 

Cause

This callback could throw that exception if you had multiple repos configured to use the same URL:

CKAN/Core/Net/Repo.cs

Lines 56 to 58 in 8390a61

// Capture repo etags to be set once we're done
var savedEtags = new Dictionary<Uri, string>();
downloader.onOneCompleted += (url, filename, error, etag) => savedEtags.Add(url, etag);

Doing that would not make sense, but apparently it happened anyway.

Changes

Now we filter out repo entries with duplicate URLs early in the update process, so we'll only download them once and the exception will not be thrown.

Also some strings in repo updating that I forgot to internationalize in #3645 are now internationalized.

Fixes #3761.

@HebaruSan HebaruSan added Bug Something is not working as intended Easy This is easy to fix Core (ckan.dll) Issues affecting the core part of CKAN Pull request Network Issues affecting internet connections of CKAN labels Feb 21, 2023
Copy link
Member

@techman83 techman83 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome, LGTM!

@HebaruSan HebaruSan merged commit 04034c0 into KSP-CKAN:master Feb 21, 2023
@HebaruSan HebaruSan deleted the fix/dup-repo-url-exc branch February 26, 2023 18:56
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 Easy This is easy to fix Network Issues affecting internet connections of CKAN
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug] An item with the same key has already been added after repo update completion
2 participants