commit 9c6d3b4a18f242038ae03b0032fae3bd14977521
parent ac30d93c9e42412dfa2bbcd1e18c5a3c4e1b2f36
Author: yao-msft <50888816+yao-msft@users.noreply.github.com>
Date: Tue, 4 Feb 2025 12:53:46 -0800
Fix certain msstore source 404 failures by treating them as empty responses (#5179)
When searching for store source with id that's recently taken down,
store source will return 404 instead of empty results (what
winget-cli-restsource does). This will be treated as a source search
error in our code. The fix is to treat 404 response with certain schema
as empty result instead of failure.
Tested manually with store source and added unit tests.
fixes #4785, fixes #4784
Diffstat:
5 files changed, 81 insertions(+), 7 deletions(-)
diff --git a/.github/actions/spelling/expect.txt b/.github/actions/spelling/expect.txt
@@ -224,6 +224,7 @@ IFACEMETHODIMP
iid
img
inet
+innererror
inproc
Insta
installinprogress
diff --git a/src/AppInstallerCLITests/RestInterface_1_0.cpp b/src/AppInstallerCLITests/RestInterface_1_0.cpp
@@ -339,6 +339,17 @@ TEST_CASE("Search_GoodResponse_AllFields", "[RestSource][Interface_1_0]")
REQUIRE(package.Versions.at(0).ProductCodes.at(1) == "pc2");
}
+TEST_CASE("Search_GoodResponse_404AsEmpty", "[RestSource][Interface_1_0]")
+{
+ utility::string_t notFoundResponse = _XPLATSTR(
+ R"delimiter({"code":"DataNotFound","data":[],"details":[],"innererror":{"code":"DataNotFound","data":[],"details":[],"message":"Product is not present","source":"StoreEdgeFD"},"message":"Product is not present","source":"StoreEdgeFD"})delimiter");
+
+ HttpClientHelper helper{ GetTestRestRequestHandler(web::http::status_codes::NotFound, std::move(notFoundResponse)) };
+ Interface v1{ TestRestUriString, std::move(helper) };
+ Schema::IRestClient::SearchResult searchResponse = v1.Search({});
+ REQUIRE(searchResponse.Matches.size() == 0);
+}
+
TEST_CASE("Search_ContinuationToken", "[RestSource][Interface_1_0]")
{
utility::string_t sample = _XPLATSTR(
@@ -447,7 +458,7 @@ TEST_CASE("GetManifests_GoodResponse", "[RestSource][Interface_1_0]")
Interface v1{ TestRestUriString, std::move(helper) };
std::vector<Manifest> manifests = v1.GetManifests("Foo.Bar");
REQUIRE(manifests.size() == 1);
-
+
// Verify manifest is populated
Manifest manifest = manifests[0];
REQUIRE(manifest.Id == "Foo.Bar");
@@ -459,6 +470,17 @@ TEST_CASE("GetManifests_GoodResponse", "[RestSource][Interface_1_0]")
sampleManifest.VerifyInstallers_AllFields(manifest);
}
+TEST_CASE("GetManifests_GoodResponse_404AsEmpty", "[RestSource][Interface_1_0]")
+{
+ utility::string_t notFoundResponse = _XPLATSTR(
+ R"delimiter({"code":"DataNotFound","data":[],"details":[],"innererror":{"code":"DataNotFound","data":[],"details":[],"message":"Product is not present","source":"StoreEdgeFD"},"message":"Product is not present","source":"StoreEdgeFD"})delimiter");
+
+ HttpClientHelper helper{ GetTestRestRequestHandler(web::http::status_codes::NotFound, std::move(notFoundResponse)) };
+ Interface v1{ TestRestUriString, std::move(helper) };
+ std::vector<Manifest> manifests = v1.GetManifests("Foo.Bar");
+ REQUIRE(manifests.size() == 0);
+}
+
TEST_CASE("GetManifests_BadResponse_SuccessCode", "[RestSource][Interface_1_0]")
{
utility::string_t badManifest = _XPLATSTR(
diff --git a/src/AppInstallerCommonCore/HttpClientHelper.cpp b/src/AppInstallerCommonCore/HttpClientHelper.cpp
@@ -95,7 +95,8 @@ namespace AppInstaller::Http
const utility::string_t& uri,
const web::json::value& body,
const HttpClientHelper::HttpRequestHeaders& headers,
- const HttpClientHelper::HttpRequestHeaders& authHeaders) const
+ const HttpClientHelper::HttpRequestHeaders& authHeaders,
+ const HttpResponseHandler& customHandler) const
{
web::http::http_response httpResponse;
Post(uri, body, headers, authHeaders).then([&httpResponse](const web::http::http_response& response)
@@ -103,6 +104,15 @@ namespace AppInstaller::Http
httpResponse = response;
}).wait();
+ if (customHandler)
+ {
+ auto handlerResult = customHandler(httpResponse);
+ if (!handlerResult.UseDefaultHandling)
+ {
+ return std::move(handlerResult.Result);
+ }
+ }
+
return ValidateAndExtractResponse(httpResponse);
}
@@ -137,7 +147,8 @@ namespace AppInstaller::Http
std::optional<web::json::value> HttpClientHelper::HandleGet(
const utility::string_t& uri,
const HttpClientHelper::HttpRequestHeaders& headers,
- const HttpClientHelper::HttpRequestHeaders& authHeaders) const
+ const HttpClientHelper::HttpRequestHeaders& authHeaders,
+ const HttpResponseHandler& customHandler) const
{
web::http::http_response httpResponse;
Get(uri, headers, authHeaders).then([&httpResponse](const web::http::http_response& response)
@@ -145,6 +156,15 @@ namespace AppInstaller::Http
httpResponse = response;
}).wait();
+ if (customHandler)
+ {
+ auto handlerResult = customHandler(httpResponse);
+ if (!handlerResult.UseDefaultHandling)
+ {
+ return std::move(handlerResult.Result);
+ }
+ }
+
return ValidateAndExtractResponse(httpResponse);
}
diff --git a/src/AppInstallerCommonCore/Public/winget/HttpClientHelper.h b/src/AppInstallerCommonCore/Public/winget/HttpClientHelper.h
@@ -13,15 +13,26 @@ namespace AppInstaller::Http
{
using HttpRequestHeaders = std::unordered_map<utility::string_t, utility::string_t>;
+ struct HttpResponseHandlerResult
+ {
+ // The custom response handler result. Default is empty.
+ std::optional<web::json::value> Result = std::nullopt;
+
+ // Indicates whether to use default handling logic by HttpClientHelper instead (i.e. the custom response handler does not handle the specific response).
+ bool UseDefaultHandling = false;
+ };
+
+ using HttpResponseHandler = std::function<HttpResponseHandlerResult(const web::http::http_response&)>;
+
HttpClientHelper(std::shared_ptr<web::http::http_pipeline_stage> = {});
pplx::task<web::http::http_response> Post(const utility::string_t& uri, const web::json::value& body, const HttpRequestHeaders& headers = {}, const HttpRequestHeaders& authHeaders = {}) const;
- std::optional<web::json::value> HandlePost(const utility::string_t& uri, const web::json::value& body, const HttpRequestHeaders& headers = {}, const HttpRequestHeaders& authHeaders = {}) const;
+ std::optional<web::json::value> HandlePost(const utility::string_t& uri, const web::json::value& body, const HttpRequestHeaders& headers = {}, const HttpRequestHeaders& authHeaders = {}, const HttpResponseHandler& customHandler = {}) const;
pplx::task<web::http::http_response> Get(const utility::string_t& uri, const HttpRequestHeaders& headers = {}, const HttpRequestHeaders& authHeaders = {}) const;
- std::optional<web::json::value> HandleGet(const utility::string_t& uri, const HttpRequestHeaders& headers = {}, const HttpRequestHeaders& authHeaders = {}) const;
+ std::optional<web::json::value> HandleGet(const utility::string_t& uri, const HttpRequestHeaders& headers = {}, const HttpRequestHeaders& authHeaders = {}, const HttpResponseHandler& customHandler = {}) const;
void SetPinningConfiguration(const Certificates::PinningConfiguration& configuration);
diff --git a/src/AppInstallerRepositoryCore/Rest/Schema/1_0/RestInterface_1_0.cpp b/src/AppInstallerRepositoryCore/Rest/Schema/1_0/RestInterface_1_0.cpp
@@ -50,6 +50,26 @@ namespace AppInstaller::Repository::Rest::Schema::V1_0
return {};
}
+
+ AppInstaller::Http::HttpClientHelper::HttpResponseHandlerResult CustomRestCallResponseHandler(const web::http::http_response& response)
+ {
+ AppInstaller::Http::HttpClientHelper::HttpResponseHandlerResult result;
+ result.UseDefaultHandling = true;
+
+ if (response.status_code() == web::http::status_codes::NotFound &&
+ response.headers().content_type()._Starts_with(web::http::details::mime_types::application_json))
+ {
+ auto responseJson = response.extract_json().get();
+ if (responseJson.is_object() && responseJson.has_field(L"code") && responseJson.has_field(L"message"))
+ {
+ // We'll treat 404 with json response containing code and message fields as empty result.
+ // Leave the HttpResponseHandlerResult result empty and disable default HttpClientHelper handling.
+ result.UseDefaultHandling = false;
+ }
+ }
+
+ return result;
+ }
}
Interface::Interface(const std::string& restApi, const Http::HttpClientHelper& httpClientHelper) : m_restApiUri(restApi), m_httpClientHelper(httpClientHelper)
@@ -94,7 +114,7 @@ namespace AppInstaller::Repository::Rest::Schema::V1_0
searchHeaders.insert_or_assign(AppInstaller::JSON::GetUtilityString(ContinuationToken), continuationToken);
}
- std::optional<web::json::value> jsonObject = m_httpClientHelper.HandlePost(m_searchEndpoint, GetValidatedSearchBody(request), searchHeaders, GetAuthHeaders());
+ std::optional<web::json::value> jsonObject = m_httpClientHelper.HandlePost(m_searchEndpoint, GetValidatedSearchBody(request), searchHeaders, GetAuthHeaders(), CustomRestCallResponseHandler);
utility::string_t ct;
if (jsonObject)
@@ -221,7 +241,7 @@ namespace AppInstaller::Repository::Rest::Schema::V1_0
std::vector<Manifest::Manifest> results;
utility::string_t continuationToken;
Http::HttpClientHelper::HttpRequestHeaders searchHeaders = m_requiredRestApiHeaders;
- std::optional<web::json::value> jsonObject = m_httpClientHelper.HandleGet(GetManifestByVersionEndpoint(m_restApiUri, packageId, validatedParams), searchHeaders, GetAuthHeaders());
+ std::optional<web::json::value> jsonObject = m_httpClientHelper.HandleGet(GetManifestByVersionEndpoint(m_restApiUri, packageId, validatedParams), searchHeaders, GetAuthHeaders(), CustomRestCallResponseHandler);
if (!jsonObject)
{
