Honor 429 Retry-After (#3718) (#3730)

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.

.github/actions/spelling/expect.txt

@@ -244,6 +244,7 @@ LOWORD
 LPARAM
 LPBYTE
 LPCWSTR
+lpdw
 LPDWORD
 lpfn
 LPGRPICONDIR
@@ -481,6 +482,7 @@ ucasemap
 UChars
 ucnv
 uec
+UNAVAIL
 uninitialize
 unins
 uninstallation

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.

src/AppInstallerCLICore/Resources.h

@@ -533,6 +533,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);

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)

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
             {

src/AppInstallerCLIPackage/Shared/Strings/en-us/winget.resw

@@ -2080,4 +2080,7 @@ Please specify one of them using the --source option to proceed.</value>
   <data name="PolicyEnableWinGetConfiguration" xml:space="preserve">
     <value>Enable Windows Package Manager Configuration</value>
   </data>
+  <data name="Unavailable" xml:space="preserve">
+    <value>Unavailable</value>
+  </data>
 </root>

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]")

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());
+    }
 }

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