commit 21de1607ed5c90174e6bb931406975c18681a5dd parent 265301e1876709de78846d29ae4018f959a01242 Author: Ruben Guerrero <rubengu@microsoft.com> Date: Wed, 4 Oct 2023 15:26:06 -0700 Honor 429 Retry-After (#3718) Change This pull request honors 429 Retry-After for pre indexed sources and throws ServiceUnavailableException when the response is 429 or 503. If there's a Retry-After header and its value is more than 60s or the date is more than 60s into the future Save the next time it can be updated in the source details and not try again for that command. Subsequent updates or open that source will be skipped if the time lapsed hasn't passed. If there's a Retry-After header and its value is less than 60s or the date value is less than 60 seconds into the future Retry once after that amount of time. If no Retry-After header Retry after a random amount of time. For open, the failed to update source message will show up. For update, a warning with Unavailable will show up. The next available time will show up in the logs. Diffstat:
25 files changed, 470 insertions(+), 87 deletions(-)
diff --git a/.github/actions/spelling/expect.txt b/.github/actions/spelling/expect.txt @@ -245,6 +245,7 @@ LOWORD LPARAM LPBYTE LPCWSTR +lpdw LPDWORD lpfn LPGRPICONDIR @@ -483,6 +484,7 @@ ucasemap UChars ucnv uec +UNAVAIL uninitialize unins uninstallation diff --git a/doc/windows/package-manager/winget/returnCodes.md b/doc/windows/package-manager/winget/returnCodes.md @@ -120,6 +120,7 @@ ms.localizationpriority: medium | 0x8A15006A | -1978335126 | APPINSTALLER_CLI_ERROR_APPTERMINATION_RECEIVED | Application shutdown signal received | | 0x8A15006B | -1978335125 | APPINSTALLER_CLI_ERROR_DOWNLOAD_DEPENDENCIES | Failed to download package dependencies. | | 0x8A15006C | -1978335124 | APPINSTALLER_CLI_ERROR_DOWNLOAD_COMMAND_PROHIBITED | Failed to download package. Download for offline installation is prohibited. | +| 0x8A15006D | -1978335123 | APPINSTALLER_CLI_ERROR_SERVICE_UNAVAILABLE | A required service is busy or unavailable. Try again later. | ## Install errors. diff --git a/src/AppInstallerCLICore/Resources.h b/src/AppInstallerCLICore/Resources.h @@ -538,6 +538,7 @@ namespace AppInstaller::CLI::Resource WINGET_DEFINE_RESOURCE_STRINGID(TooManyArgError); WINGET_DEFINE_RESOURCE_STRINGID(TooManyBehaviorsError); WINGET_DEFINE_RESOURCE_STRINGID(UnableToPurgeInstallDirectory); + WINGET_DEFINE_RESOURCE_STRINGID(Unavailable); WINGET_DEFINE_RESOURCE_STRINGID(UnexpectedErrorExecutingCommand); WINGET_DEFINE_RESOURCE_STRINGID(UninstallAbandoned); WINGET_DEFINE_RESOURCE_STRINGID(UninstallCommandLongDescription); diff --git a/src/AppInstallerCLICore/Workflows/DownloadFlow.cpp b/src/AppInstallerCLICore/Workflows/DownloadFlow.cpp @@ -307,7 +307,8 @@ namespace AppInstaller::CLI::Workflow std::optional<std::vector<BYTE>> hash; - const int MaxRetryCount = 2; + constexpr int MaxRetryCount = 2; + constexpr std::chrono::seconds maximumWaitTimeAllowed = 60s; for (int retryCount = 0; retryCount < MaxRetryCount; ++retryCount) { bool success = false; @@ -323,6 +324,31 @@ namespace AppInstaller::CLI::Workflow success = true; } + catch (const ServiceUnavailableException& sue) + { + if (retryCount < MaxRetryCount - 1) + { + auto waitSecondsForRetry = sue.RetryAfter(); + if (waitSecondsForRetry > maximumWaitTimeAllowed) + { + throw; + } + + bool waitCompleted = context.Reporter.ExecuteWithProgress([&waitSecondsForRetry](IProgressCallback& progress) + { + return ProgressCallback::Wait(progress, waitSecondsForRetry); + }); + + if (!waitCompleted) + { + break; + } + } + else + { + throw; + } + } catch (...) { if (retryCount < MaxRetryCount - 1) diff --git a/src/AppInstallerCLICore/Workflows/SourceFlow.cpp b/src/AppInstallerCLICore/Workflows/SourceFlow.cpp @@ -198,9 +198,17 @@ namespace AppInstaller::CLI::Workflow Repository::Source source{ sd.Name }; context.Reporter.Info() << Resource::String::SourceUpdateOne(Utility::LocIndView{ sd.Name }) << std::endl; auto updateFunction = [&](IProgressCallback& progress)->std::vector<Repository::SourceDetails> { return source.Update(progress); }; - if (!context.Reporter.ExecuteWithProgress(updateFunction).empty()) + auto sourceDetails = context.Reporter.ExecuteWithProgress(updateFunction); + if (!sourceDetails.empty()) { - context.Reporter.Info() << Resource::String::Cancelled << std::endl; + if (std::chrono::system_clock::now() < sourceDetails[0].DoNotUpdateBefore) + { + context.Reporter.Warn() << Resource::String::Unavailable << std::endl; + } + else + { + context.Reporter.Info() << Resource::String::Cancelled << std::endl; + } } else { diff --git a/src/AppInstallerCLIPackage/Shared/Strings/en-us/winget.resw b/src/AppInstallerCLIPackage/Shared/Strings/en-us/winget.resw @@ -2581,4 +2581,7 @@ Please specify one of them using the --source option to proceed.</value> <data name="WINGET_CONFIG_ERROR_UNIT_SETTING_CONFIG_ROOT" xml:space="preserve"> <value>A unit contains a setting that requires the config root.</value> </data> + <data name="Unavailable" xml:space="preserve"> + <value>Unavailable</value> + </data> </root> \ No newline at end of file diff --git a/src/AppInstallerCLITests/HttpClientHelper.cpp b/src/AppInstallerCLITests/HttpClientHelper.cpp @@ -24,7 +24,7 @@ TEST_CASE("ExtractJsonResponse_UnsupportedMimeType", "[RestSource][RestSearch]") TEST_CASE("ValidateAndExtractResponse_ServiceUnavailable", "[RestSource]") { HttpClientHelper helper{ GetTestRestRequestHandler(web::http::status_codes::ServiceUnavailable) }; - REQUIRE_THROWS_HR(helper.HandleGet(L"https://testUri"), MAKE_HRESULT(SEVERITY_ERROR, FACILITY_HTTP, web::http::status_codes::ServiceUnavailable)); + REQUIRE_THROWS_HR(helper.HandleGet(L"https://testUri"), APPINSTALLER_CLI_ERROR_SERVICE_UNAVAILABLE); } TEST_CASE("ValidateAndExtractResponse_NotFound", "[RestSource]") diff --git a/src/AppInstallerCommonCore/Downloader.cpp b/src/AppInstallerCommonCore/Downloader.cpp @@ -16,12 +16,80 @@ using namespace AppInstaller::Runtime; using namespace AppInstaller::Settings; using namespace AppInstaller::Filesystem; +using namespace AppInstaller::Utility::HttpStream; using namespace winrt::Windows::Web::Http; using namespace winrt::Windows::Web::Http::Headers; using namespace winrt::Windows::Web::Http::Filters; namespace AppInstaller::Utility { + namespace + { + // Gets the retry after value in terms of a delay in seconds + std::chrono::seconds GetRetryAfter(const HttpDateOrDeltaHeaderValue& retryAfter) + { + if (retryAfter) + { + auto delta = retryAfter.Delta(); + if (delta) + { + return std::chrono::duration_cast<std::chrono::seconds>(delta.GetTimeSpan()); + } + + auto dateTimeRef = retryAfter.Date(); + if (dateTimeRef) + { + auto dateTime = dateTimeRef.GetDateTime(); + auto now = winrt::clock::now(); + + if (dateTime > now) + { + return std::chrono::duration_cast<std::chrono::seconds>(dateTime - now); + } + } + } + + return 0s; + } + + std::chrono::seconds GetRetryAfter(const wil::unique_hinternet& urlFile) + { + std::wstring retryAfter = {}; + DWORD length = 0; + if (!HttpQueryInfoW(urlFile.get(), + HTTP_QUERY_RETRY_AFTER, + &retryAfter, + &length, + nullptr)) + { + auto lastError = GetLastError(); + if (lastError == ERROR_INSUFFICIENT_BUFFER) + { + // lpdwBufferLength contains the size, in bytes, of a buffer large enough to receive the requested information + // without the nul char. not the exact buffer size. + auto size = static_cast<size_t>(length) / sizeof(wchar_t); + retryAfter.resize(size + 1); + if (HttpQueryInfoW(urlFile.get(), + HTTP_QUERY_RETRY_AFTER, + &retryAfter[0], + &length, + nullptr)) + { + // because the buffer can be bigger remove possible null chars + retryAfter.erase(retryAfter.find(L'\0')); + return AppInstaller::Utility::GetRetryAfter(retryAfter); + } + } + else + { + AICLI_LOG(Core, Error, << "Error retrieving Retry-After header: " << GetLastError()); + } + } + + return 0s; + } + } + std::optional<std::vector<BYTE>> WinINetDownloadToStream( const std::string& url, std::ostream& dest, @@ -57,14 +125,25 @@ namespace AppInstaller::Utility DWORD requestStatus = 0; DWORD cbRequestStatus = sizeof(requestStatus); - THROW_LAST_ERROR_IF_MSG(!HttpQueryInfo(urlFile.get(), + THROW_LAST_ERROR_IF_MSG(!HttpQueryInfoW(urlFile.get(), HTTP_QUERY_STATUS_CODE | HTTP_QUERY_FLAG_NUMBER, &requestStatus, &cbRequestStatus, nullptr), "Query download request status failed."); - if (requestStatus != HTTP_STATUS_OK) + constexpr DWORD TooManyRequest = 429; + + switch (requestStatus) + { + case HTTP_STATUS_OK: + // All good + break; + case TooManyRequest: + case HTTP_STATUS_SERVICE_UNAVAIL: { + THROW_EXCEPTION(ServiceUnavailableException(GetRetryAfter(urlFile))); + } + default: AICLI_LOG(Core, Error, << "Download request failed. Returned status: " << requestStatus); THROW_HR_MSG(MAKE_HRESULT(SEVERITY_ERROR, FACILITY_HTTP, requestStatus), "Download request status is not success."); } @@ -75,7 +154,7 @@ namespace AppInstaller::Utility LONGLONG contentLength = 0; DWORD cbContentLength = sizeof(contentLength); - HttpQueryInfo( + HttpQueryInfoW( urlFile.get(), HTTP_QUERY_CONTENT_LENGTH | HTTP_QUERY_FLAG_NUMBER64, &contentLength, @@ -160,9 +239,19 @@ namespace AppInstaller::Utility HttpResponseMessage response = client.SendRequestAsync(request, HttpCompletionOption::ResponseHeadersRead).get(); - THROW_HR_IF( - MAKE_HRESULT(SEVERITY_ERROR, FACILITY_HTTP, response.StatusCode()), - response.StatusCode() != HttpStatusCode::Ok); + switch (response.StatusCode()) + { + case HttpStatusCode::Ok: + // All good + break; + case HttpStatusCode::TooManyRequests: + case HttpStatusCode::ServiceUnavailable: + { + THROW_EXCEPTION(ServiceUnavailableException(GetRetryAfter(response.Headers().RetryAfter()))); + } + default: + THROW_HR(MAKE_HRESULT(SEVERITY_ERROR, FACILITY_HTTP, response.StatusCode())); + } std::map<std::string, std::string> result; @@ -409,12 +498,27 @@ namespace AppInstaller::Utility { // Get an IStream from the input uri and try to create package or bundler reader. winrt::Windows::Foundation::Uri uri(Utility::ConvertToUTF16(uriStr)); - auto randomAccessStream = HttpStream::HttpRandomAccessStream::CreateAsync(uri).get(); - ::IUnknown* rasAsIUnknown = (::IUnknown*)winrt::get_abi(randomAccessStream); - THROW_IF_FAILED(CreateStreamOverRandomAccessStream( - rasAsIUnknown, - IID_PPV_ARGS(inputStream.ReleaseAndGetAddressOf()))); + winrt::com_ptr<HttpRandomAccessStream> httpRandomAccessStream = winrt::make_self<HttpRandomAccessStream>(); + + try + { + auto randomAccessStream = httpRandomAccessStream->InitializeAsync(uri).get(); + + ::IUnknown* rasAsIUnknown = (::IUnknown*)winrt::get_abi(randomAccessStream); + THROW_IF_FAILED(CreateStreamOverRandomAccessStream( + rasAsIUnknown, + IID_PPV_ARGS(inputStream.ReleaseAndGetAddressOf()))); + } + catch (const winrt::hresult_error& hre) + { + if (hre.code() == APPINSTALLER_CLI_ERROR_SERVICE_UNAVAILABLE) + { + THROW_EXCEPTION(AppInstaller::Utility::ServiceUnavailableException(httpRandomAccessStream->RetryAfter())); + } + + throw; + } } else { @@ -425,4 +529,26 @@ namespace AppInstaller::Utility return inputStream; } + + std::chrono::seconds GetRetryAfter(const std::wstring& retryAfter) + { + try + { + winrt::hstring hstringValue{ retryAfter }; + HttpDateOrDeltaHeaderValue headerValue = nullptr; + HttpDateOrDeltaHeaderValue::TryParse(hstringValue, headerValue); + return GetRetryAfter(headerValue); + } + catch (...) + { + AICLI_LOG(Core, Error, << "Retry-After value not supported: " << Utility::ConvertToUTF8(retryAfter)); + } + + return 0s; + } + + std::chrono::seconds GetRetryAfter(const HttpResponseMessage& response) + { + return GetRetryAfter(response.Headers().RetryAfter()); + } } diff --git a/src/AppInstallerCommonCore/HttpStream/HttpClientWrapper.cpp b/src/AppInstallerCommonCore/HttpStream/HttpClientWrapper.cpp @@ -5,6 +5,7 @@ #include "Public/AppInstallerStrings.h" #include "HttpClientWrapper.h" #include "Public/AppInstallerRuntime.h" +#include "Public/AppInstallerDownloader.h" using namespace winrt::Windows::Foundation; using namespace winrt::Windows::Security::Cryptography; @@ -47,9 +48,19 @@ namespace AppInstaller::Utility::HttpStream HttpResponseMessage response = co_await m_httpClient.SendRequestAsync(request, HttpCompletionOption::ResponseHeadersRead); - THROW_HR_IF( - MAKE_HRESULT(SEVERITY_ERROR, FACILITY_HTTP, response.StatusCode()), - response.StatusCode() != HttpStatusCode::Ok); + switch (response.StatusCode()) + { + case HttpStatusCode::Ok: + // All good + break; + case HttpStatusCode::TooManyRequests: + case HttpStatusCode::ServiceUnavailable: + { + THROW_EXCEPTION(ServiceUnavailableException(GetRetryAfter(response))); + } + default: + THROW_HR(MAKE_HRESULT(SEVERITY_ERROR, FACILITY_HTTP, response.StatusCode())); + } // Get the length from the response if (response.Content().Headers().HasKey(L"Content-Length")) @@ -106,6 +117,21 @@ namespace AppInstaller::Utility::HttpStream HttpResponseMessage response = co_await m_httpClient.SendRequestAsync(request, HttpCompletionOption::ResponseHeadersRead); HttpContentHeaderCollection contentHeaders = response.Content().Headers(); + switch (response.StatusCode()) + { + case HttpStatusCode::Ok: + case HttpStatusCode::PartialContent: + // All good + break; + case HttpStatusCode::TooManyRequests: + case HttpStatusCode::ServiceUnavailable: + { + THROW_EXCEPTION(ServiceUnavailableException(GetRetryAfter(response))); + } + default: + THROW_HR(MAKE_HRESULT(SEVERITY_ERROR, FACILITY_HTTP, response.StatusCode())); + } + if (response.StatusCode() != HttpStatusCode::PartialContent && startPosition != 0) { // throw HRESULT used for range-request error diff --git a/src/AppInstallerCommonCore/HttpStream/HttpRandomAccessStream.cpp b/src/AppInstallerCommonCore/HttpStream/HttpRandomAccessStream.cpp @@ -3,6 +3,7 @@ #include "pch.h" #include "HttpRandomAccessStream.h" +#include "Public/AppInstallerDownloader.h" using namespace winrt::Windows::Foundation; using namespace winrt::Windows::Storage::Streams; @@ -12,16 +13,23 @@ using namespace winrt::Windows::Storage::Streams; // The HRESULTs will be mapped to UI error code by the appropriate component namespace AppInstaller::Utility::HttpStream { - IAsyncOperation<IRandomAccessStream> HttpRandomAccessStream::CreateAsync(const Uri& uri) + IAsyncOperation<IRandomAccessStream> HttpRandomAccessStream::InitializeAsync(const Uri& uri) { - winrt::com_ptr<HttpRandomAccessStream> stream = winrt::make_self<HttpRandomAccessStream>(); - - stream->m_httpHelper = co_await HttpClientWrapper::CreateAsync(uri); - stream->m_size = stream->m_httpHelper->GetFullFileSize(); - stream->m_httpLocalCache = std::make_unique<HttpLocalCache>(); - - co_return stream.as<IRandomAccessStream>(); - + auto strong_this{ get_strong() }; + + try + { + strong_this->m_httpHelper = co_await HttpClientWrapper::CreateAsync(uri); + strong_this->m_size = strong_this->m_httpHelper->GetFullFileSize(); + strong_this->m_httpLocalCache = std::make_unique<HttpLocalCache>(); + } + catch (const ServiceUnavailableException& e) + { + strong_this->m_retryAfter = e.RetryAfter(); + throw; + } + + co_return strong_this.as<IRandomAccessStream>(); } uint64_t HttpRandomAccessStream::Size() const @@ -86,4 +94,9 @@ namespace AppInstaller::Utility::HttpStream co_return result; } + + std::chrono::seconds HttpRandomAccessStream::RetryAfter() const + { + return m_retryAfter; + } } \ No newline at end of file diff --git a/src/AppInstallerCommonCore/HttpStream/HttpRandomAccessStream.h b/src/AppInstallerCommonCore/HttpStream/HttpRandomAccessStream.h @@ -5,6 +5,8 @@ #include "HttpClientWrapper.h" #include "HttpLocalCache.h" +using namespace std::chrono_literals; + namespace AppInstaller::Utility::HttpStream { // Provides an implementation of a random access stream over HTTP that supports @@ -17,7 +19,7 @@ namespace AppInstaller::Utility::HttpStream winrt::Windows::Storage::Streams::IInputStream> { public: - static winrt::Windows::Foundation::IAsyncOperation<winrt::Windows::Storage::Streams::IRandomAccessStream> CreateAsync( + winrt::Windows::Foundation::IAsyncOperation<winrt::Windows::Storage::Streams::IRandomAccessStream> InitializeAsync( const winrt::Windows::Foundation::Uri& uri); uint64_t Size() const; void Size(uint64_t value); @@ -32,11 +34,13 @@ namespace AppInstaller::Utility::HttpStream winrt::Windows::Storage::Streams::IBuffer buffer, uint32_t count, winrt::Windows::Storage::Streams::InputStreamOptions options); + std::chrono::seconds RetryAfter() const; private: std::shared_ptr<HttpClientWrapper> m_httpHelper; std::unique_ptr<HttpLocalCache> m_httpLocalCache; unsigned long long m_size = 0; unsigned long long m_requestedPosition = 0; + std::chrono::seconds m_retryAfter = 0s; }; } \ No newline at end of file diff --git a/src/AppInstallerCommonCore/MsixInfo.cpp b/src/AppInstallerCommonCore/MsixInfo.cpp @@ -531,6 +531,7 @@ namespace AppInstaller::Msix MsixInfo::MsixInfo(std::string_view uriStr) { m_stream = Utility::GetReadOnlyStreamFromURI(uriStr); + if (GetBundleReader(m_stream.Get(), &m_bundleReader)) { m_isBundle = true; diff --git a/src/AppInstallerCommonCore/Progress.cpp b/src/AppInstallerCommonCore/Progress.cpp @@ -64,6 +64,24 @@ namespace AppInstaller } } + bool ProgressCallback::Wait(IProgressCallback& progress, std::chrono::milliseconds millisecondsToWait) + { + wil::unique_event calledEvent; + calledEvent.create(); + + auto cancellationFunc = progress.SetCancellationFunction([&calledEvent]() + { + calledEvent.SetEvent(); + }); + + if (calledEvent.wait(static_cast<DWORD>(millisecondsToWait.count()))) + { + return false; + } + + return true; + } + void ProgressCallback::Cancel(CancelReason reason) { m_cancelReason = reason; diff --git a/src/AppInstallerCommonCore/Public/AppInstallerDownloader.h b/src/AppInstallerCommonCore/Public/AppInstallerDownloader.h @@ -1,11 +1,15 @@ // Copyright (c) Microsoft Corporation. // Licensed under the MIT License. #pragma once +#include <AppInstallerErrors.h> #include <AppInstallerProgress.h> +#include <winrt/Windows.Web.Http.h> + #include <urlmon.h> #include <wrl/client.h> +#include <chrono> #include <filesystem> #include <map> #include <optional> @@ -14,6 +18,8 @@ #include <string_view> #include <vector> +using namespace std::chrono_literals; + namespace AppInstaller::Utility { // The type of data being downloaded; determines what code should @@ -34,6 +40,17 @@ namespace AppInstaller::Utility std::string ContentId; }; + // An exception that indicates that a remote service is too busy/unavailable and may contain data on when to try again. + struct ServiceUnavailableException : public wil::ResultException + { + ServiceUnavailableException(std::chrono::seconds retryAfter = 0s) : wil::ResultException(APPINSTALLER_CLI_ERROR_SERVICE_UNAVAILABLE), m_retryAfter(retryAfter) {} + + std::chrono::seconds RetryAfter() const { return m_retryAfter; } + + private: + std::chrono::seconds m_retryAfter; + }; + // Downloads a file from the given URL and places it in the given location. // url: The url to be downloaded from. http->https redirection is allowed. // dest: The stream to be downloaded to. @@ -83,4 +100,10 @@ namespace AppInstaller::Utility // Function to read-only create a stream from a uri string (url address or file system path) Microsoft::WRL::ComPtr<IStream> GetReadOnlyStreamFromURI(std::string_view uriStr); + + // Gets the retry after value in terms of a delay in seconds. + std::chrono::seconds GetRetryAfter(const std::wstring& retryAfter); + + // Gets the retry after value in terms of a delay in seconds. + std::chrono::seconds GetRetryAfter(const winrt::Windows::Web::Http::HttpResponseMessage& response); } diff --git a/src/AppInstallerCommonCore/Public/AppInstallerProgress.h b/src/AppInstallerCommonCore/Public/AppInstallerProgress.h @@ -78,6 +78,8 @@ namespace AppInstaller ProgressCallback() = default; ProgressCallback(IProgressSink* sink); + static bool Wait(IProgressCallback& progress, std::chrono::milliseconds ms); + void BeginProgress() override; void OnProgress(uint64_t current, uint64_t maximum, ProgressType type) override; diff --git a/src/AppInstallerRepositoryCore/Microsoft/PreIndexedPackageSourceFactory.cpp b/src/AppInstallerRepositoryCore/Microsoft/PreIndexedPackageSourceFactory.cpp @@ -112,7 +112,7 @@ namespace AppInstaller::Repository::Microsoft std::string alternateLocation = GetAlternatePackageLocation(details, s_PreIndexedPackageSourceFactory_PackageFileName); // Try getting the primary location's info - HRESULT primaryHR = S_OK; + std::exception_ptr primaryException; try { @@ -125,7 +125,8 @@ namespace AppInstaller::Repository::Microsoft { throw; } - primaryHR = LOG_CAUGHT_EXCEPTION_MSG("PreIndexedPackageUpdateCheck failed on primary location"); + LOG_CAUGHT_EXCEPTION_MSG("PreIndexedPackageUpdateCheck failed on primary location"); + primaryException = std::current_exception(); } // Try alternate location @@ -138,7 +139,7 @@ namespace AppInstaller::Repository::Microsoft } CATCH_LOG_MSG("PreIndexedPackageUpdateCheck failed on alternate location"); - THROW_HR(primaryHR); + std::rethrow_exception(primaryException); } const std::string& PackageLocation() const { return m_packageLocation; } @@ -152,31 +153,27 @@ namespace AppInstaller::Repository::Microsoft { if (Utility::IsUrlRemote(packageLocation)) { - try + std::map<std::string, std::string> headers = Utility::GetHeaders(packageLocation); + auto itr = headers.find(std::string{ s_PreIndexedPackageSourceFactory_PackageVersionHeader }); + if (itr != headers.end()) { - std::map<std::string, std::string> headers = Utility::GetHeaders(packageLocation); - auto itr = headers.find(std::string{ s_PreIndexedPackageSourceFactory_PackageVersionHeader }); - if (itr != headers.end()) - { - AICLI_LOG(Repo, Verbose, << "Header indicates version is: " << itr->second); - return { itr->second }; - } + AICLI_LOG(Repo, Verbose, << "Header indicates version is: " << itr->second); + return { itr->second }; + } - // We did not find the header we were looking for, log the ones we did find - AICLI_LOG(Repo, Verbose, << "Did not find " << s_PreIndexedPackageSourceFactory_PackageVersionHeader << " in:\n" << [&]() + // We did not find the header we were looking for, log the ones we did find + AICLI_LOG(Repo, Verbose, << "Did not find " << s_PreIndexedPackageSourceFactory_PackageVersionHeader << " in:\n" << [&]() + { + std::ostringstream headerLog; + for (const auto& header : headers) { - std::ostringstream headerLog; - for (const auto& header : headers) - { - headerLog << " " << header.first << " : " << header.second << '\n'; - } - return std::move(headerLog).str(); - }()); - } - CATCH_LOG(); + headerLog << " " << header.first << " : " << header.second << '\n'; + } + return std::move(headerLog).str(); + }()); } - AICLI_LOG(Repo, Verbose, << "Falling back to reading the package data"); + AICLI_LOG(Repo, Verbose, << "Reading package data to determine version"); Msix::MsixInfo info{ packageLocation }; auto manifest = info.GetAppPackageManifests(); diff --git a/src/AppInstallerRepositoryCore/Microsoft/SQLiteIndexSource.cpp b/src/AppInstallerRepositoryCore/Microsoft/SQLiteIndexSource.cpp @@ -142,7 +142,8 @@ namespace AppInstaller::Repository::Microsoft AICLI_LOG(Repo, Info, << "Downloading manifest"); ProgressCallback emptyCallback; - const int MaxRetryCount = 2; + constexpr int MaxRetryCount = 2; + constexpr std::chrono::seconds maximumWaitTimeAllowed = 10s; for (int retryCount = 0; retryCount < MaxRetryCount; ++retryCount) { try @@ -157,6 +158,25 @@ namespace AppInstaller::Repository::Microsoft break; } + catch (const ServiceUnavailableException& sue) + { + if (retryCount < MaxRetryCount - 1) + { + auto waitSecondsForRetry = sue.RetryAfter(); + if (waitSecondsForRetry > maximumWaitTimeAllowed) + { + throw; + } + + // TODO: Get real progress callback to allow cancelation. + auto ms = std::chrono::duration_cast<std::chrono::milliseconds>(waitSecondsForRetry); + Sleep(static_cast<DWORD>(ms.count())); + } + else + { + throw; + } + } catch (...) { if (retryCount < MaxRetryCount - 1) diff --git a/src/AppInstallerRepositoryCore/Public/winget/RepositorySource.h b/src/AppInstallerRepositoryCore/Public/winget/RepositorySource.h @@ -119,6 +119,9 @@ namespace AppInstaller::Repository // The last time that this source was updated. std::chrono::system_clock::time_point LastUpdateTime = {}; + // Stores the earliest time that a background update should be attempted. + std::chrono::system_clock::time_point DoNotUpdateBefore = {}; + // Whether the source supports InstalledSource correlation. bool SupportInstalledSearchCorrelation = true; diff --git a/src/AppInstallerRepositoryCore/RepositorySource.cpp b/src/AppInstallerRepositoryCore/RepositorySource.cpp @@ -34,52 +34,111 @@ namespace AppInstaller::Repository return ISourceFactory::GetForType(details.Type)->Create(details); } + std::chrono::milliseconds GetMillisecondsToWait(std::chrono::seconds retryAfter, size_t randomMultiplier = 1) + { + if (retryAfter != 0s) + { + return std::chrono::duration_cast<std::chrono::milliseconds>(retryAfter); + } + else + { + // Add a bit of randomness to the retry wait time + std::default_random_engine randomEngine(std::random_device{}()); + std::uniform_int_distribution<long long> distribution(2000, 10000); + + return std::chrono::milliseconds(distribution(randomEngine) * randomMultiplier); + } + } + + bool IsUpdateSuppressed(const SourceDetails& details) + { + return std::chrono::system_clock::now() < details.DoNotUpdateBefore; + } + + struct AddOrUpdateResult + { + bool UpdateChecked = false; + bool MetadataWritten = false; + }; + template <typename MemberFunc> - bool AddOrUpdateFromDetails(SourceDetails& details, MemberFunc member, IProgressCallback& progress) + AddOrUpdateResult AddOrUpdateFromDetails(SourceDetails& details, MemberFunc member, IProgressCallback& progress) { - bool result = false; + AddOrUpdateResult result; + auto factory = ISourceFactory::GetForType(details.Type); + // If we are instructed to wait longer than this, just fail rather than retrying. + constexpr std::chrono::seconds maximumWaitTimeAllowed = 60s; + std::chrono::seconds waitSecondsForRetry = 0s; + // Attempt; if it fails, wait a short time and retry. try { - result = (factory.get()->*member)(details, progress); - if (result) + result.UpdateChecked = (factory.get()->*member)(details, progress); + if (result.UpdateChecked) { details.LastUpdateTime = std::chrono::system_clock::now(); + result.MetadataWritten = true; } return result; } + catch (const Utility::ServiceUnavailableException& sue) + { + waitSecondsForRetry = sue.RetryAfter(); + + // Do not retry if the server tell us to wait more than the max time allowed. + if (waitSecondsForRetry > maximumWaitTimeAllowed) + { + details.DoNotUpdateBefore = std::chrono::system_clock::now() + waitSecondsForRetry; + AICLI_LOG(Repo, Info, << "Source `" << details.Name << "` unavailable first try, setting DoNotUpdateBefore to " << details.DoNotUpdateBefore); + result.MetadataWritten = true; + return result; + } + } CATCH_LOG(); - AICLI_LOG(Repo, Info, << "Source add/update failed, waiting a bit and retrying: " << details.Name); + std::chrono::milliseconds millisecondsToWait = GetMillisecondsToWait(waitSecondsForRetry); - // Add a bit of randomness to the retry wait time - std::default_random_engine randomEngine(std::random_device{}()); - std::uniform_int_distribution<long long> distribution(2000, 10000); + AICLI_LOG(Repo, Info, << "Source add/update failed, waiting " << millisecondsToWait.count() << " milliseconds and retrying: " << details.Name); - std::this_thread::sleep_for(std::chrono::milliseconds(distribution(randomEngine))); + if (!ProgressCallback::Wait(progress, millisecondsToWait)) + { + AICLI_LOG(Repo, Info, << "Source second try cancelled."); + return {}; + } - // If this one fails, maybe the problem is persistent. - result = (factory.get()->*member)(details, progress); - if (result) + try + { + // If this one fails, maybe the problem is persistent. + result.UpdateChecked = (factory.get()->*member)(details, progress); + if (result.UpdateChecked) + { + details.LastUpdateTime = std::chrono::system_clock::now(); + result.MetadataWritten = true; + } + } + catch (const Utility::ServiceUnavailableException& sue) { - details.LastUpdateTime = std::chrono::system_clock::now(); + details.DoNotUpdateBefore = std::chrono::system_clock::now() + GetMillisecondsToWait(sue.RetryAfter(), 3); + AICLI_LOG(Repo, Info, << "Source `" << details.Name << "` unavailable second try, setting DoNotUpdateBefore to " << details.DoNotUpdateBefore); + result.MetadataWritten = true; } + return result; } - bool AddSourceFromDetails(SourceDetails& details, IProgressCallback& progress) + AddOrUpdateResult AddSourceFromDetails(SourceDetails& details, IProgressCallback& progress) { return AddOrUpdateFromDetails(details, &ISourceFactory::Add, progress); } - bool UpdateSourceFromDetails(SourceDetails& details, IProgressCallback& progress) + AddOrUpdateResult UpdateSourceFromDetails(SourceDetails& details, IProgressCallback& progress) { return AddOrUpdateFromDetails(details, &ISourceFactory::Update, progress); } - bool BackgroundUpdateSourceFromDetails(SourceDetails& details, IProgressCallback& progress) + AddOrUpdateResult BackgroundUpdateSourceFromDetails(SourceDetails& details, IProgressCallback& progress) { return AddOrUpdateFromDetails(details, &ISourceFactory::BackgroundUpdate, progress); } @@ -104,6 +163,13 @@ namespace AppInstaller::Repository return false; } + // Do not update if we are still before the update block time. + if (IsUpdateSuppressed(details)) + { + AICLI_LOG(Repo, Info, << "Background update is suppressed until: " << details.DoNotUpdateBefore); + return false; + } + constexpr static TimeSpan s_ZeroMins = 0min; TimeSpan autoUpdateTime; if (backgroundUpdateInterval.has_value()) @@ -639,7 +705,9 @@ namespace AppInstaller::Repository { // TODO: Consider adding a context callback to indicate we are doing the same action // to avoid the progress bar fill up multiple times. - if (BackgroundUpdateSourceFromDetails(details, progress)) + AddOrUpdateResult updateResult = BackgroundUpdateSourceFromDetails(details, progress); + + if (updateResult.MetadataWritten) { if (sourceList == nullptr) { @@ -647,10 +715,11 @@ namespace AppInstaller::Repository } auto detailsInternal = sourceList->GetSource(details.Name); - detailsInternal->LastUpdateTime = details.LastUpdateTime; + detailsInternal->CopyMetadataFieldsFrom(details); sourceList->SaveMetadata(*detailsInternal); } - else + + if (!updateResult.UpdateChecked) { AICLI_LOG(Repo, Error, << "Failed to update source: " << details.Name); result.emplace_back(details); @@ -747,7 +816,7 @@ namespace AppInstaller::Repository sourceDetails.Origin = SourceOrigin::User; } - bool result = AddSourceFromDetails(sourceDetails, progress); + bool result = AddSourceFromDetails(sourceDetails, progress).UpdateChecked; if (result) { sourceList.AddSource(sourceDetails); @@ -777,13 +846,16 @@ namespace AppInstaller::Repository { // TODO: Consider adding a context callback to indicate we are doing the same action // to avoid the progress bar fill up multiple times. - if (UpdateSourceFromDetails(details, progress)) + AddOrUpdateResult updateResult = UpdateSourceFromDetails(details, progress); + + if (updateResult.MetadataWritten) { auto detailsInternal = sourceList.GetSource(details.Name); - detailsInternal->LastUpdateTime = details.LastUpdateTime; + detailsInternal->CopyMetadataFieldsFrom(details); sourceList.SaveMetadata(*detailsInternal); } - else + + if (!updateResult.UpdateChecked) { AICLI_LOG(Repo, Error, << "Failed to update source: " << details.Name); result.emplace_back(details); diff --git a/src/AppInstallerRepositoryCore/Rest/Schema/HttpClientHelper.cpp b/src/AppInstallerRepositoryCore/Rest/Schema/HttpClientHelper.cpp @@ -29,6 +29,17 @@ namespace AppInstaller::Repository::Rest::Schema THROW_HR_IF(APPINSTALLER_CLI_ERROR_PINNED_CERTIFICATE_MISMATCH, !pinningConfiguration.Validate(certContext.get())); } + + std::chrono::seconds GetRetryAfter(const web::http::http_headers& headers) + { + auto retryAfterHeader = headers.find(web::http::header_names::retry_after); + if (retryAfterHeader != headers.end()) + { + return AppInstaller::Utility::GetRetryAfter(retryAfterHeader->second.c_str()); + } + + return 0s; + } } HttpClientHelper::HttpClientHelper(std::shared_ptr<web::http::http_pipeline_stage> stage) : m_defaultRequestHandlerStage(std::move(stage)) {} @@ -135,7 +146,6 @@ namespace AppInstaller::Repository::Rest::Schema case web::http::status_codes::NotFound: THROW_HR(APPINSTALLER_CLI_ERROR_RESTSOURCE_ENDPOINT_NOT_FOUND); - break; case web::http::status_codes::NoContent: result = {}; @@ -143,11 +153,13 @@ namespace AppInstaller::Repository::Rest::Schema case web::http::status_codes::BadRequest: THROW_HR(APPINSTALLER_CLI_ERROR_RESTSOURCE_INTERNAL_ERROR); - break; + + case web::http::status_codes::TooManyRequests: + case web::http::status_codes::ServiceUnavailable: + THROW_EXCEPTION(AppInstaller::Utility::ServiceUnavailableException(GetRetryAfter(response.headers()))); default: THROW_HR(MAKE_HRESULT(SEVERITY_ERROR, FACILITY_HTTP, response.status_code())); - break; } return result; diff --git a/src/AppInstallerRepositoryCore/SourceList.cpp b/src/AppInstallerRepositoryCore/SourceList.cpp @@ -28,12 +28,12 @@ namespace AppInstaller::Repository constexpr std::string_view s_MetadataYaml_Sources = "Sources"sv; constexpr std::string_view s_MetadataYaml_Source_Name = "Name"sv; constexpr std::string_view s_MetadataYaml_Source_LastUpdate = "LastUpdate"sv; + constexpr std::string_view s_MetadataYaml_Source_DoNotUpdateBefore = "DoNotUpdateBefore"sv; constexpr std::string_view s_MetadataYaml_Source_AcceptedAgreementsIdentifier = "AcceptedAgreementsIdentifier"sv; constexpr std::string_view s_MetadataYaml_Source_AcceptedAgreementFields = "AcceptedAgreementFields"sv; constexpr std::string_view s_Source_WingetCommunityDefault_Name = "winget"sv; constexpr std::string_view s_Source_WingetCommunityDefault_Arg = "https://cdn.winget.microsoft.com/cache"sv; - constexpr std::string_view s_Source_WingetCommunityDefault_AlternateArg = "https://winget-cache-pme-cxfsgwfxarb8hwg0.z01.azurefd.net/cache"sv; constexpr std::string_view s_Source_WingetCommunityDefault_Data = "Microsoft.Winget.Source_8wekyb3d8bbwe"sv; constexpr std::string_view s_Source_WingetCommunityDefault_Identifier = "Microsoft.Winget.Source_8wekyb3d8bbwe"sv; @@ -43,7 +43,6 @@ namespace AppInstaller::Repository constexpr std::string_view s_Source_DesktopFrameworks_Name = "microsoft.builtin.desktop.frameworks"sv; constexpr std::string_view s_Source_DesktopFrameworks_Arg = "https://cdn.winget.microsoft.com/platform"sv; - constexpr std::string_view s_Source_DesktopFrameworks_AlternateArg = "https://winget-cache-pme-cxfsgwfxarb8hwg0.z01.azurefd.net/platform"sv; constexpr std::string_view s_Source_DesktopFrameworks_Data = "Microsoft.Winget.Platform.Source_8wekyb3d8bbwe"sv; constexpr std::string_view s_Source_DesktopFrameworks_Identifier = "Microsoft.Winget.Platform.Source_8wekyb3d8bbwe"sv; @@ -205,11 +204,26 @@ namespace AppInstaller::Repository void SourceDetailsInternal::CopyMetadataFieldsTo(SourceDetailsInternal& target) { - target.LastUpdateTime = LastUpdateTime; + if (LastUpdateTime > target.LastUpdateTime) + { + target.LastUpdateTime = LastUpdateTime; + } + + if (DoNotUpdateBefore > target.DoNotUpdateBefore) + { + target.DoNotUpdateBefore = DoNotUpdateBefore; + } + target.AcceptedAgreementFields = AcceptedAgreementFields; target.AcceptedAgreementsIdentifier = AcceptedAgreementsIdentifier; } + void SourceDetailsInternal::CopyMetadataFieldsFrom(const SourceDetails& source) + { + LastUpdateTime = source.LastUpdateTime; + DoNotUpdateBefore = source.DoNotUpdateBefore; + } + std::string_view GetWellKnownSourceName(WellKnownSource source) { switch (source) @@ -286,10 +300,6 @@ namespace AppInstaller::Repository details.Name = s_Source_WingetCommunityDefault_Name; details.Type = Microsoft::PreIndexedPackageSourceFactory::Type(); details.Arg = s_Source_WingetCommunityDefault_Arg; - if (Settings::User().Get<Settings::Setting::NetworkWingetAlternateSourceURL>()) - { - details.AlternateArg = s_Source_WingetCommunityDefault_AlternateArg; - } details.Data = s_Source_WingetCommunityDefault_Data; details.Identifier = s_Source_WingetCommunityDefault_Identifier; details.TrustLevel = SourceTrustLevel::Trusted | SourceTrustLevel::StoreOrigin; @@ -340,7 +350,6 @@ namespace AppInstaller::Repository details.Name = s_Source_DesktopFrameworks_Name; details.Type = Microsoft::PreIndexedPackageSourceFactory::Type(); details.Arg = s_Source_DesktopFrameworks_Arg; - details.AlternateArg = s_Source_DesktopFrameworks_AlternateArg; details.Data = s_Source_DesktopFrameworks_Data; details.Identifier = s_Source_DesktopFrameworks_Identifier; details.TrustLevel = SourceTrustLevel::Trusted | SourceTrustLevel::StoreOrigin; @@ -710,9 +719,17 @@ namespace AppInstaller::Repository details.Origin = SourceOrigin::Metadata; std::string_view name = m_metadataStream.GetName(); if (!TryReadScalar(name, settingValue, source, s_MetadataYaml_Source_Name, details.Name)) { return false; } + int64_t lastUpdateInEpoch{}; if (!TryReadScalar(name, settingValue, source, s_MetadataYaml_Source_LastUpdate, lastUpdateInEpoch)) { return false; } details.LastUpdateTime = Utility::ConvertUnixEpochToSystemClock(lastUpdateInEpoch); + + int64_t doNotUpdateBeforeInEpoch{}; + if (TryReadScalar(name, settingValue, source, s_MetadataYaml_Source_DoNotUpdateBefore, doNotUpdateBeforeInEpoch, false)) + { + details.DoNotUpdateBefore = Utility::ConvertUnixEpochToSystemClock(doNotUpdateBeforeInEpoch); + } + TryReadScalar(name, settingValue, source, s_MetadataYaml_Source_AcceptedAgreementsIdentifier, details.AcceptedAgreementsIdentifier, false); TryReadScalar(name, settingValue, source, s_MetadataYaml_Source_AcceptedAgreementFields, details.AcceptedAgreementFields, false); return true; @@ -731,6 +748,7 @@ namespace AppInstaller::Repository out << YAML::BeginMap; out << YAML::Key << s_MetadataYaml_Source_Name << YAML::Value << details.Name; out << YAML::Key << s_MetadataYaml_Source_LastUpdate << YAML::Value << Utility::ConvertSystemClockToUnixEpoch(details.LastUpdateTime); + out << YAML::Key << s_MetadataYaml_Source_DoNotUpdateBefore << YAML::Value << Utility::ConvertSystemClockToUnixEpoch(details.DoNotUpdateBefore); out << YAML::Key << s_MetadataYaml_Source_AcceptedAgreementsIdentifier << YAML::Value << details.AcceptedAgreementsIdentifier; out << YAML::Key << s_MetadataYaml_Source_AcceptedAgreementFields << YAML::Value << details.AcceptedAgreementFields; out << YAML::EndMap; diff --git a/src/AppInstallerRepositoryCore/SourceList.h b/src/AppInstallerRepositoryCore/SourceList.h @@ -19,9 +19,12 @@ namespace AppInstaller::Repository SourceDetailsInternal() = default; SourceDetailsInternal(const SourceDetails& details) : SourceDetails(details) {} - // Copies the metadata fields from this to target. + // Copies the metadata fields to this target. void CopyMetadataFieldsTo(SourceDetailsInternal& target); + // Copies the metadata fields from this source. This only include partial metadata. + void CopyMetadataFieldsFrom(const SourceDetails& source); + // If true, this is a tombstone, marking the deletion of a source at a lower priority origin. bool IsTombstone = false; diff --git a/src/AppInstallerSharedLib/Errors.cpp b/src/AppInstallerSharedLib/Errors.cpp @@ -195,6 +195,7 @@ namespace AppInstaller WINGET_HRESULT_INFO(APPINSTALLER_CLI_ERROR_APPTERMINATION_RECEIVED, "Application shutdown signal received"), WINGET_HRESULT_INFO(APPINSTALLER_CLI_ERROR_DOWNLOAD_DEPENDENCIES, "Failed to download package dependencies."), WINGET_HRESULT_INFO(APPINSTALLER_CLI_ERROR_DOWNLOAD_COMMAND_PROHIBITED, "Failed to download package. Download for offline installation is prohibited."), + WINGET_HRESULT_INFO(APPINSTALLER_CLI_ERROR_SERVICE_UNAVAILABLE, "A required service is busy or unavailable. Try again later."), // Install errors. WINGET_HRESULT_INFO(APPINSTALLER_CLI_ERROR_INSTALL_PACKAGE_IN_USE, "Application is currently running. Exit the application then try again."), diff --git a/src/AppInstallerSharedLib/Public/AppInstallerErrors.h b/src/AppInstallerSharedLib/Public/AppInstallerErrors.h @@ -125,6 +125,7 @@ #define APPINSTALLER_CLI_ERROR_APPTERMINATION_RECEIVED ((HRESULT)0x8A15006A) #define APPINSTALLER_CLI_ERROR_DOWNLOAD_DEPENDENCIES ((HRESULT)0x8A15006B) #define APPINSTALLER_CLI_ERROR_DOWNLOAD_COMMAND_PROHIBITED ((HRESULT)0x8A15006C) +#define APPINSTALLER_CLI_ERROR_SERVICE_UNAVAILABLE ((HRESULT)0x8A15006D) // Install errors. #define APPINSTALLER_CLI_ERROR_INSTALL_PACKAGE_IN_USE ((HRESULT)0x8A150101) diff --git a/src/WinGetUtil/pch.h b/src/WinGetUtil/pch.h @@ -17,3 +17,5 @@ #include <memory> #include <mutex> #include <string> + +#include <winrt/Windows.Foundation.h>