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

Honor 429 Retry-After #3718

Merged
merged 18 commits into from
Oct 4, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions .github/actions/spelling/expect.txt
Original file line number Diff line number Diff line change
Expand Up @@ -245,6 +245,7 @@ LOWORD
LPARAM
LPBYTE
LPCWSTR
lpdw
LPDWORD
lpfn
LPGRPICONDIR
Expand Down Expand Up @@ -483,6 +484,7 @@ ucasemap
UChars
ucnv
uec
UNAVAIL
uninitialize
unins
uninstallation
Expand Down
1 change: 1 addition & 0 deletions doc/windows/package-manager/winget/returnCodes.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
1 change: 1 addition & 0 deletions src/AppInstallerCLICore/Resources.h
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
28 changes: 27 additions & 1 deletion src/AppInstallerCLICore/Workflows/DownloadFlow.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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)
Expand Down
12 changes: 10 additions & 2 deletions src/AppInstallerCLICore/Workflows/SourceFlow.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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
{
Expand Down
3 changes: 3 additions & 0 deletions src/AppInstallerCLIPackage/Shared/Strings/en-us/winget.resw
Original file line number Diff line number Diff line change
Expand Up @@ -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>
2 changes: 1 addition & 1 deletion src/AppInstallerCLITests/HttpClientHelper.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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]")
Expand Down
148 changes: 137 additions & 11 deletions src/AppInstallerCommonCore/Downloader.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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.
msftrubengu marked this conversation as resolved.
Show resolved Hide resolved
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,
Expand Down Expand Up @@ -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.");
}
Expand All @@ -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,
Expand Down Expand Up @@ -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;

Expand Down Expand Up @@ -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
{
Expand All @@ -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());
}
}
32 changes: 29 additions & 3 deletions src/AppInstallerCommonCore/HttpStream/HttpClientWrapper.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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"))
Expand Down Expand Up @@ -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
Expand Down
Loading
Loading