experiments/STT-tensorflow
1
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity

Don't retry HTTP requests that fail due to unknown hosts or SSL certs.

Browse Source 
Currently, TF's HTTP library defaults to retrying almost all failed requests;
for some forms of error, including host resolution failure and errors loading
SSL certs from disk, there's no need to retry, as the situation won't recover
on its own.

A concrete situation where this arises is attempting to detect the GCE
metadata service, where looking up `http://metadata` can have all sorts of
pathological behavior. (In fact, here's me debugging the same issue with a
different library:
  https://github.com/googleapis/oauth2client/issues/93
... in 2014.)

This change fixes the issue by adding `CURLE_COULDNT_RESOLVE_HOST` and
`CURLE_SSL_CACERT_BADFILE` as errors we don't retry in our HTTP client, and
adds a test.

PiperOrigin-RevId: 289687412
Change-Id: I4e0d4f0d5efca111a7b66ef1570db82e1c6c5d43
This commit is contained in:
Craig Citro 2020-01-14 11:02:10 -08:00 committed by TensorFlower Gardener
parent be57d362ad
commit 857b55bc2d
2 changed files with 49 additions and 0 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

7
tensorflow/core/platform/cloud/curl_http_request.cc
View File

@ -641,6 +641,13 @@ Status CurlHttpRequest::CURLcodeToStatus(CURLcode code,
return errors::FailedPrecondition(
strings::StrCat(error_message, overflow_message));
}
// Domain resolution errors and certificate problems aren't going to improve
// on retry, so we return a FailedPrecondition (as the caller must take action
// before this can succeed).
if (code == CURLE_COULDNT_RESOLVE_HOST || code == CURLE_SSL_CACERT_BADFILE) {
return errors::FailedPrecondition(
strings::StrCat(error_message, error_buffer));
}
// Return Unavailable to retry by default. There may be other permanent
// failures that should be distinguished.
return errors::Unavailable(

42
tensorflow/core/platform/cloud/curl_http_request_test.cc
View File

@ -443,6 +443,48 @@ TEST(CurlHttpRequestTest, GetRequest_HttpCode0) {
EXPECT_EQ(0, http_request.GetResponseCode());
}
TEST(CurlHttpRequestTest, GetRequest_CouldntResolveHost) {
FakeLibCurl libcurl("get response", 0);
libcurl.curl_easy_perform_result_ = CURLE_COULDNT_RESOLVE_HOST;
libcurl.curl_easy_perform_error_message_ =
"Could not resolve host 'metadata'";
CurlHttpRequest http_request(&libcurl);
std::vector<char> scratch;
scratch.insert(scratch.end(), kTestContent.begin(), kTestContent.end());
http_request.SetUri("http://metadata");
const auto& status = http_request.Send();
EXPECT_EQ(error::FAILED_PRECONDITION, status.code());
EXPECT_EQ(
"Error executing an HTTP request: libcurl code 6 meaning "
"'Couldn't resolve host name', error details: Could not resolve host "
"'metadata'",
status.error_message());
EXPECT_EQ(0, http_request.GetResponseCode());
}
TEST(CurlHttpRequestTest, GetRequest_SslBadCertfile) {
FakeLibCurl libcurl("get response", 0);
libcurl.curl_easy_perform_result_ = CURLE_SSL_CACERT_BADFILE;
libcurl.curl_easy_perform_error_message_ =
"error setting certificate verify locations:";
CurlHttpRequest http_request(&libcurl);
std::vector<char> scratch;
scratch.insert(scratch.end(), kTestContent.begin(), kTestContent.end());
http_request.SetUri("http://metadata");
const auto& status = http_request.Send();
EXPECT_EQ(error::FAILED_PRECONDITION, status.code());
EXPECT_EQ(
"Error executing an HTTP request: libcurl code 77 meaning "
"'Problem with the SSL CA cert (path? access rights?)', error details: "
"error setting certificate verify locations:",
status.error_message());
EXPECT_EQ(0, http_request.GetResponseCode());
}
TEST(CurlHttpRequestTest, ResponseHeaders) {
FakeLibCurl libcurl(
"get response", 200,