[addresses #2648] add Resource URL to HTTP response object in case of an error (#3427)

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
This commit is contained in:
saleh-amazon
2025-07-16 07:08:21 -07:00
committed by GitHub
parent 635c3727bb
commit d350dc39e0
2 changed files with 21 additions and 13 deletions

View File

@@ -305,6 +305,8 @@ std::unique_ptr<AsyncRequest> HTTPFileSource::request(const Resource& resource,
Response response;
using Error = Response::Error;
std::string stringResourceURL = " - Resource URL: " + resource.url;
if (error) {
[MLNNativeNetworkManager.sharedManager errorLog:@"Requesting: %@ failed with error: %@", req.URL, error.debugDescription];
@@ -316,7 +318,8 @@ std::unique_ptr<AsyncRequest> HTTPFileSource::request(const Resource& resource,
switch ([error code]) {
case NSURLErrorBadServerResponse: // 5xx errors
response.error = std::make_unique<Error>(
Error::Reason::Server, [[error localizedDescription] UTF8String]);
Error::Reason::Server, std::string{ [[error localizedDescription] UTF8String]} + stringResourceURL);
break;
case NSURLErrorNetworkConnectionLost:
@@ -329,12 +332,12 @@ std::unique_ptr<AsyncRequest> HTTPFileSource::request(const Resource& resource,
case NSURLErrorDataNotAllowed:
case NSURLErrorTimedOut:
response.error = std::make_unique<Error>(
Error::Reason::Connection, [[error localizedDescription] UTF8String]);
Error::Reason::Connection, [[error localizedDescription] UTF8String] + stringResourceURL);
break;
default:
response.error = std::make_unique<Error>(
Error::Reason::Other, [[error localizedDescription] UTF8String]);
Error::Reason::Other, [[error localizedDescription] UTF8String] + stringResourceURL);
break;
}
} else if ([res isKindOfClass:[NSHTTPURLResponse class]]) {
@@ -372,7 +375,7 @@ std::unique_ptr<AsyncRequest> HTTPFileSource::request(const Resource& resource,
response.notModified = true;
} else if (responseCode == 404) {
response.error =
std::make_unique<Error>(Error::Reason::NotFound, "HTTP status code 404");
std::make_unique<Error>(Error::Reason::NotFound, "HTTP status code 404" + stringResourceURL);
} else if (responseCode == 429) {
// Get the standard header
std::optional<std::string> retryAfter;
@@ -388,22 +391,22 @@ std::unique_ptr<AsyncRequest> HTTPFileSource::request(const Resource& resource,
xRateLimitReset = std::string([xReset UTF8String]);
}
response.error = std::make_unique<Error>(Error::Reason::RateLimit, "HTTP status code 429", http::parseRetryHeaders(retryAfter, xRateLimitReset));
response.error = std::make_unique<Error>(Error::Reason::RateLimit, "HTTP status code 429" + stringResourceURL, http::parseRetryHeaders(retryAfter, xRateLimitReset));
} else if (responseCode >= 500 && responseCode < 600) {
response.error =
std::make_unique<Error>(Error::Reason::Server, std::string{ "HTTP status code " } +
std::to_string(responseCode));
std::to_string(responseCode) + stringResourceURL);
} else {
response.error =
std::make_unique<Error>(Error::Reason::Other, std::string{ "HTTP status code " } +
std::to_string(responseCode));
std::to_string(responseCode) + stringResourceURL);
}
} else if ([url isFileURL]) {
response.data = std::make_shared<std::string>((const char *)[data bytes], [data length]);
} else {
// This should never happen.
response.error = std::make_unique<Error>(Error::Reason::Other,
"Response class is not NSHTTPURLResponse");
"Response class is not NSHTTPURLResponse" + stringResourceURL);
}
shared->notify(response);
@@ -430,5 +433,4 @@ void HTTPFileSource::setClientOptions(ClientOptions options) {
ClientOptions HTTPFileSource::getClientOptions() {
return impl->getClientOptions();
}
}

View File

@@ -62,10 +62,13 @@ TEST(HTTPFileSource, TEST_REQUIRES_SERVER(HTTP404)) {
util::RunLoop loop;
HTTPFileSource fs(ResourceOptions::Default(), ClientOptions());
auto req = fs.request({Resource::Unknown, "http://127.0.0.1:3000/doesnotexist"}, [&](Response res) {
std::string stringResourceURL = "http://127.0.0.1:3000/doesnotexist";
std::string resourceURLInterpolation = " - Resource URL: " + stringResourceURL;
auto req = fs.request({Resource::Unknown, stringResourceURL}, [&](Response res) {
ASSERT_NE(nullptr, res.error);
EXPECT_EQ(Response::Error::Reason::NotFound, res.error->reason);
EXPECT_EQ("HTTP status code 404", res.error->message);
EXPECT_EQ("HTTP status code 404" + resourceURLInterpolation, res.error->message + resourceURLInterpolation);
EXPECT_FALSE(bool(res.data));
EXPECT_FALSE(bool(res.expires));
EXPECT_FALSE(res.mustRevalidate);
@@ -135,10 +138,13 @@ TEST(HTTPFileSource, TEST_REQUIRES_SERVER(HTTP500)) {
util::RunLoop loop;
HTTPFileSource fs(ResourceOptions::Default(), ClientOptions());
auto req = fs.request({Resource::Unknown, "http://127.0.0.1:3000/permanent-error"}, [&](Response res) {
std::string stringResourceURL = "http://127.0.0.1:3000/permanent-error";
std::string resourceURLInterpolation = " - Resource URL: " + stringResourceURL;
auto req = fs.request({Resource::Unknown, stringResourceURL}, [&](Response res) {
ASSERT_NE(nullptr, res.error);
EXPECT_EQ(Response::Error::Reason::Server, res.error->reason);
EXPECT_EQ("HTTP status code 500", res.error->message);
EXPECT_EQ("HTTP status code 500" + resourceURLInterpolation, res.error->message + resourceURLInterpolation);
EXPECT_FALSE(bool(res.data));
EXPECT_FALSE(bool(res.expires));
EXPECT_FALSE(res.mustRevalidate);