Add source fallback retry logic to NuGet package downloader#54228
Open
marcpopMSFT wants to merge 1 commit into
Open
Add source fallback retry logic to NuGet package downloader#54228marcpopMSFT wants to merge 1 commit into
marcpopMSFT wants to merge 1 commit into
Conversation
When downloading a NuGet package fails from the initially selected source (e.g., an Azure DevOps feed with upstream sources that reports having a package via metadata but returns 401/403 on download), the downloader now retries with remaining sources instead of immediately failing. This fixes the scenario where parallel feed querying selects a feed that claims to have a package through its upstream configuration, but the user lacks permissions to pull from that upstream. Previously, this resulted in a hard failure even when another configured feed (e.g., nuget.org) could serve the package. The fix: - Catches FatalProtocolException and HttpRequestException during download - Excludes the failed source and re-resolves from remaining sources - Pins to the already-resolved package version to prevent version drift - Creates fresh file streams per attempt to avoid partial write corruption - Does NOT fall back on signing verification failures (security boundary) - Preserves original exception as inner exception in final failure Fixes #54141 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Adds fallback behavior to the CLI NuGet package downloader so that when a package download fails from an initially selected source (e.g., due to upstream 401/403), it retries with remaining configured sources rather than failing immediately.
Changes:
- Implemented multi-source fallback loop in
NuGetPackageDownloader.DownloadPackageAsync, with version pinning and per-attempt file stream creation. - Added a new localized user-facing warning string for “download failed from source; trying another”.
- Updated CLI localization
.xlffiles to include the new string.
Reviewed changes
Copilot reviewed 15 out of 15 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| src/Cli/dotnet/NugetPackageDownloader/NuGetPackageDownloader.cs | Adds download failure fallback across sources and introduces helper methods for classifying download failures and cleanup. |
| src/Cli/dotnet/CliStrings.resx | Adds the new PackageDownloadFailedFromSource string resource. |
| src/Cli/dotnet/xlf/CliStrings.cs.xlf | Adds localized entry placeholder for the new string. |
| src/Cli/dotnet/xlf/CliStrings.de.xlf | Adds localized entry placeholder for the new string. |
| src/Cli/dotnet/xlf/CliStrings.es.xlf | Adds localized entry placeholder for the new string. |
| src/Cli/dotnet/xlf/CliStrings.fr.xlf | Adds localized entry placeholder for the new string. |
| src/Cli/dotnet/xlf/CliStrings.it.xlf | Adds localized entry placeholder for the new string. |
| src/Cli/dotnet/xlf/CliStrings.ja.xlf | Adds localized entry placeholder for the new string. |
| src/Cli/dotnet/xlf/CliStrings.ko.xlf | Adds localized entry placeholder for the new string. |
| src/Cli/dotnet/xlf/CliStrings.pl.xlf | Adds localized entry placeholder for the new string. |
| src/Cli/dotnet/xlf/CliStrings.pt-BR.xlf | Adds localized entry placeholder for the new string. |
| src/Cli/dotnet/xlf/CliStrings.ru.xlf | Adds localized entry placeholder for the new string. |
| src/Cli/dotnet/xlf/CliStrings.tr.xlf | Adds localized entry placeholder for the new string. |
| src/Cli/dotnet/xlf/CliStrings.zh-Hans.xlf | Adds localized entry placeholder for the new string. |
| src/Cli/dotnet/xlf/CliStrings.zh-Hant.xlf | Adds localized entry placeholder for the new string. |
|
|
||
| FindPackageByIdResource resource = null; | ||
| SourceRepository repository = GetSourceRepository(source); | ||
| IEnumerable<PackageSource> allSources = LoadNuGetSources(packageId, packageSourceLocation, packageSourceMapping); |
Comment on lines
+124
to
+131
| SourceRepository repository = GetSourceRepository(source); | ||
| FindPackageByIdResource resource = await repository.GetResourceAsync<FindPackageByIdResource>(cancellationToken) | ||
| .ConfigureAwait(false); | ||
|
|
||
| if (resource != null) | ||
| { | ||
| try | ||
| { |
Comment on lines
+122
to
+189
| while (true) | ||
| { | ||
| throw new NuGetPackageInstallerException( | ||
| string.Format("Downloading {0} version {1} failed", packageId, | ||
| packageVersion.ToNormalizedString())); | ||
| SourceRepository repository = GetSourceRepository(source); | ||
| FindPackageByIdResource resource = await repository.GetResourceAsync<FindPackageByIdResource>(cancellationToken) | ||
| .ConfigureAwait(false); | ||
|
|
||
| if (resource != null) | ||
| { | ||
| try | ||
| { | ||
| // Use a fresh file stream for each download attempt to avoid corruption from partial writes | ||
| { | ||
| await using FileStream destinationStream = File.Create(nupkgPath); | ||
| bool success = await ExponentialRetry.ExecuteWithRetryOnFailure(async () => await resource.CopyNupkgToStreamAsync( | ||
| packageId.ToString(), | ||
| resolvedPackageVersion, | ||
| destinationStream, | ||
| _cacheSettings, | ||
| _verboseLogger, | ||
| cancellationToken)); | ||
|
|
||
| if (success) | ||
| { | ||
| // Signing verification failure should not trigger fallback to another source | ||
| try | ||
| { | ||
| await VerifySigning(nupkgPath, repository); | ||
| } | ||
| catch (NuGetPackageInstallerException) | ||
| { | ||
| File.Delete(nupkgPath); | ||
| throw; | ||
| } | ||
|
|
||
| return nupkgPath; | ||
| } | ||
| } | ||
|
|
||
| // Download returned false — clean up partial file before trying next source | ||
| TryDeleteFile(nupkgPath); | ||
| } | ||
| catch (Exception ex) when (IsDownloadFailure(ex)) | ||
| { | ||
| firstDownloadException ??= ex; | ||
| _verboseLogger.LogWarning(string.Format( | ||
| CliStrings.PackageDownloadFailedFromSource, | ||
| packageId, source.Source, ex.Message)); | ||
| TryDeleteFile(nupkgPath); | ||
| } | ||
| } | ||
|
|
||
| // Mark this source as failed and try to find another source that has the package | ||
| failedSources.Add(source.Source); | ||
| var remainingSources = allSources.Where(s => !failedSources.Contains(s.Source)); | ||
|
|
||
| if (!remainingSources.Any()) | ||
| { | ||
| break; | ||
| } | ||
|
|
||
| try | ||
| { | ||
| // Re-resolve using only the remaining sources, pinning to the already-resolved version | ||
| (source, _) = await GetPackageSourceAndVersion(packageId, resolvedPackageVersion, | ||
| remainingSources, includePreview, resolvedIncludeUnlisted).ConfigureAwait(false); | ||
| } | ||
| catch (NuGetPackageNotFoundException) | ||
| { |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
When downloading a NuGet package fails from the initially selected source (e.g., an Azure DevOps feed with upstream sources that reports having a package via metadata but returns 401/403 on download), the downloader now retries with remaining sources instead of immediately failing.
This fixes the scenario where parallel feed querying selects a feed that claims to have a package through its upstream configuration, but the user lacks permissions to pull from that upstream. Previously, this resulted in a hard failure even when another configured feed (e.g., nuget.org) could serve the package.
The fix:
Fixes #54141