Skip to content

Commit

Permalink
if curl fails, print a warning instead of exiting (#1137)
Browse files Browse the repository at this point in the history 
* Fix microsoft/vcpkg#32522

Currently, the `download_files` function is only used by binary caching providers (`HttpGetBinaryProvider` and `GHABinaryProvider`), so I think it is okay to change the function so that it does not exit with error.

I manually tested following scenarios:

| **--only-binarycaching** | **server up** | **server down**    |
| ------------------------ | ------------- | ------------------ |
| **Without**              | OK            | Continue with Warn |
| **With**                 | OK            | Exit with Error    |

* Move retries for curl operations into curl itself.
* Added function to parse curl status lines with error messages embedded and tests.
* Add getting exit code and error message for each curl operation, and return those operation results through ExpectedL<int> rather than terminating vcpkg.
* Add test case handling for old curl tested on Ubuntu 20.04.


Co-authored-by: Billy Robert O'Neal III <[email protected]>
  • Loading branch information
@lesomnus @BillyONeal
lesomnus and BillyONeal authored Dec 26, 2024
1 parent b45524b commit 7fd552f
Show file tree
Hide file tree
Showing 6 changed files with 289 additions and 82 deletions.
14 changes: 10 additions & 4 deletions include/vcpkg/base/downloads.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,9 +32,15 @@ namespace vcpkg

View<std::string> azure_blob_headers();

std::vector<int> download_files(View<std::pair<std::string, Path>> url_pairs,
View<std::string> headers,
View<std::string> secrets);
// Parses a curl output line for curl invoked with
// -w "PREFIX%{http_code} %{exitcode} %{errormsg}"
// with specific handling for curl version < 7.75.0 which does not understand %{exitcode} %{errormsg}
// If the line is malformed for any reason, no entry to http_codes is added.
void parse_curl_status_line(std::vector<ExpectedL<int>>& http_codes, StringLiteral prefix, StringView this_line);

std::vector<ExpectedL<int>> download_files(View<std::pair<std::string, Path>> url_pairs,
View<std::string> headers,
View<std::string> secrets);

bool submit_github_dependency_graph_snapshot(const Optional<std::string>& maybe_github_server_url,
const std::string& github_token,
Expand All @@ -54,7 +60,7 @@ namespace vcpkg

std::string format_url_query(StringView base_url, View<std::string> query_params);

std::vector<int> url_heads(View<std::string> urls, View<std::string> headers, View<std::string> secrets);
std::vector<ExpectedL<int>> url_heads(View<std::string> urls, View<std::string> headers, View<std::string> secrets);

struct DownloadManagerConfig
{
Expand Down
26 changes: 11 additions & 15 deletions include/vcpkg/base/message-data.inc.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -869,12 +869,6 @@ DECLARE_MESSAGE(CommandFailed,
"command:\n"
"{command_line}\n"
"failed with the following output:")
DECLARE_MESSAGE(CommandFailedCode,
(msg::command_line, msg::exit_code),
"",
"command:\n"
"{command_line}\n"
"failed with exit code {exit_code} and the following output:")
DECLARE_MESSAGE(CommunityTriplets, (), "", "Community Triplets:")
DECLARE_MESSAGE(CompilerPath, (msg::path), "", "Compiler found: {path}")
DECLARE_MESSAGE(CompressFolderFailed, (msg::path), "", "Failed to compress folder \"{path}\":")
Expand Down Expand Up @@ -940,6 +934,10 @@ DECLARE_MESSAGE(Creating7ZipArchive, (), "", "Creating 7zip archive...")
DECLARE_MESSAGE(CreatingNugetPackage, (), "", "Creating NuGet package...")
DECLARE_MESSAGE(CreatingZipArchive, (), "", "Creating zip archive...")
DECLARE_MESSAGE(CreationFailed, (msg::path), "", "Creating {path} failed.")
DECLARE_MESSAGE(CurlFailedGeneric,
(msg::exit_code),
"curl is the name of a program, see curl.se.",
"curl operation failed with error code {exit_code}.")
DECLARE_MESSAGE(CurlFailedToPut,
(msg::exit_code, msg::url),
"curl is the name of a program, see curl.se",
Expand All @@ -948,15 +946,13 @@ DECLARE_MESSAGE(CurlFailedToPutHttp,
(msg::exit_code, msg::url, msg::value),
"curl is the name of a program, see curl.se. {value} is an HTTP status code",
"curl failed to put file to {url} with exit code {exit_code} and http code {value}.")
DECLARE_MESSAGE(CurlResponseTruncatedRetrying,
(msg::value),
"{value} is the number of milliseconds for which we are waiting this time",
"curl returned a partial response; waiting {value} milliseconds and trying again")
DECLARE_MESSAGE(CurlTimeout,
(msg::command_line),
"",
"curl was unable to perform all requested HTTP operations, even after timeout and retries. The last "
"command line was: {command_line}")
DECLARE_MESSAGE(
CurlFailedToReturnExpectedNumberOfExitCodes,
(msg::exit_code, msg::command_line),
"",
"curl failed to return the expected number of exit codes; this can happen if something terminates curl "
"before it has finished. curl exited with {exit_code} which is normally the result code for the last operation, "
"but may be the result of a crash. The command line was {command_line}, and all output is below:")
DECLARE_MESSAGE(CurrentCommitBaseline,
(msg::commit_sha),
"",
Expand Down
10 changes: 4 additions & 6 deletions locales/messages.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -502,8 +502,6 @@
"_CommandEnvExample2.comment": "This is a command line, only the <path> part should be localized",
"CommandFailed": "command:\n{command_line}\nfailed with the following output:",
"_CommandFailed.comment": "An example of {command_line} is vcpkg install zlib.",
"CommandFailedCode": "command:\n{command_line}\nfailed with exit code {exit_code} and the following output:",
"_CommandFailedCode.comment": "An example of {command_line} is vcpkg install zlib. An example of {exit_code} is 127.",
"CommunityTriplets": "Community Triplets:",
"CompilerPath": "Compiler found: {path}",
"_CompilerPath.comment": "An example of {path} is /foo/bar.",
Expand Down Expand Up @@ -544,14 +542,14 @@
"CreatingZipArchive": "Creating zip archive...",
"CreationFailed": "Creating {path} failed.",
"_CreationFailed.comment": "An example of {path} is /foo/bar.",
"CurlFailedGeneric": "curl operation failed with error code {exit_code}.",
"_CurlFailedGeneric.comment": "curl is the name of a program, see curl.se. An example of {exit_code} is 127.",
"CurlFailedToPut": "curl failed to put file to {url} with exit code {exit_code}.",
"_CurlFailedToPut.comment": "curl is the name of a program, see curl.se An example of {exit_code} is 127. An example of {url} is https://github.com/microsoft/vcpkg.",
"CurlFailedToPutHttp": "curl failed to put file to {url} with exit code {exit_code} and http code {value}.",
"_CurlFailedToPutHttp.comment": "curl is the name of a program, see curl.se. {value} is an HTTP status code An example of {exit_code} is 127. An example of {url} is https://github.com/microsoft/vcpkg.",
"CurlResponseTruncatedRetrying": "curl returned a partial response; waiting {value} milliseconds and trying again",
"_CurlResponseTruncatedRetrying.comment": "{value} is the number of milliseconds for which we are waiting this time",
"CurlTimeout": "curl was unable to perform all requested HTTP operations, even after timeout and retries. The last command line was: {command_line}",
"_CurlTimeout.comment": "An example of {command_line} is vcpkg install zlib.",
"CurlFailedToReturnExpectedNumberOfExitCodes": "curl failed to return the expected number of exit codes; this can happen if something terminates curl before it has finished. curl exited with {exit_code} which is normally the result code for the last operation, but may be the result of a crash. The command line was {command_line}, and all output is below:",
"_CurlFailedToReturnExpectedNumberOfExitCodes.comment": "An example of {exit_code} is 127. An example of {command_line} is vcpkg install zlib.",
"CurrentCommitBaseline": "You can use the current commit as a baseline, which is:\n\t\"builtin-baseline\": \"{commit_sha}\"",
"_CurrentCommitBaseline.comment": "An example of {commit_sha} is 7cfad47ae9f68b183983090afd6337cd60fd4949.",
"CycleDetectedDuring": "cycle detected during {spec}:",
Expand Down
95 changes: 95 additions & 0 deletions src/vcpkg-test/downloads.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,101 @@ TEST_CASE ("split_uri_view", "[downloads]")
}
}

TEST_CASE ("parse_curl_status_line", "[downloads]")
{
std::vector<ExpectedL<int>> http_codes;
StringLiteral malformed_examples[] = {
"asdfasdf", // wrong prefix
"curl: unknown --write-out variable: 'exitcode'", // wrong prefixes, and also what old curl does
"curl: unknown --write-out variable: 'errormsg'",
"prefix", // missing spaces
"prefix42", // missing spaces
"prefix42 2", // missing space
"prefix42 2a", // non numeric exitcode
};

for (auto&& malformed : malformed_examples)
{
parse_curl_status_line(http_codes, "prefix", malformed);
REQUIRE(http_codes.empty());
}

// old curl output
parse_curl_status_line(http_codes, "prefix", "prefix200 ");
REQUIRE(http_codes.size() == 1);
REQUIRE(http_codes[0].value_or_exit(VCPKG_LINE_INFO) == 200);
http_codes.clear();

parse_curl_status_line(http_codes, "prefix", "prefix404 ");
REQUIRE(http_codes.size() == 1);
REQUIRE(http_codes[0].value_or_exit(VCPKG_LINE_INFO) == 404);
http_codes.clear();

parse_curl_status_line(http_codes, "prefix", "prefix0 "); // a failure, but we don't know that yet
REQUIRE(http_codes.size() == 1);
REQUIRE(http_codes[0].value_or_exit(VCPKG_LINE_INFO) == 0);
http_codes.clear();

// current curl output
parse_curl_status_line(http_codes, "prefix", "prefix200 0 ");
REQUIRE(http_codes.size() == 1);
REQUIRE(http_codes[0].value_or_exit(VCPKG_LINE_INFO) == 200);
http_codes.clear();

parse_curl_status_line(http_codes,
"prefix",
"prefix0 60 schannel: SNI or certificate check failed: SEC_E_WRONG_PRINCIPAL (0x80090322) "
"- The target principal name is incorrect.");
REQUIRE(http_codes.size() == 1);
REQUIRE(http_codes[0].error().data() ==
"curl operation failed with error code 60. schannel: SNI or certificate check failed: "
"SEC_E_WRONG_PRINCIPAL (0x80090322) - The target principal name is incorrect.");
http_codes.clear();
}

TEST_CASE ("download_files", "[downloads]")
{
auto const dst = Test::base_temporary_directory() / "download_files";
auto const url = [&](std::string l) -> auto { return std::pair(l, dst); };

std::vector<std::string> headers;
std::vector<std::string> secrets;
auto results =
download_files(std::vector{url("unknown://localhost:9/secret"), url("http://localhost:9/not-exists/secret")},
headers,
secrets);
REQUIRE(results.size() == 2);
if (auto first_result = results[0].get())
{
// old curl
REQUIRE(*first_result == 0);
}
else
{
// current curl
REQUIRE(results[0].error().data() ==
"curl operation failed with error code 1. Protocol \"unknown\" not supported");
}

auto&& second_error = results[1].error().data();
std::puts(second_error.c_str());
// curl operation failed with error code 7. Failed to connect to localhost port 9 after 2241 ms: Could not connect
// to server
if (second_error == "curl operation failed with error code 7.")
{
// old curl
}
else
{
// new curl
REQUIRE_THAT(
second_error,
Catch::Matches("curl operation failed with error code 7. Failed to connect to localhost port 9 after "
"[0-9]+ ms: Could not connect to server",
Catch::CaseSensitive::Yes));
}
}

TEST_CASE ("try_parse_curl_max5_size", "[downloads]")
{
REQUIRE(!try_parse_curl_max5_size("").has_value());
Expand Down
Loading

0 comments on commit 7fd552f

Please sign in to comment.