From 59a86f5ea854475b365679bd8d7604e50b724783 Mon Sep 17 00:00:00 2001 From: Karen Arutyunov Date: Fri, 20 Jan 2023 14:44:54 +0300 Subject: Fix unexpected 'unable to read HTTP response status line' fetch error --- bpkg/fetch-git.cxx | 25 +++++++++++++++++++++++++ bpkg/fetch.cxx | 22 +++++++++++++++------- bpkg/fetch.hxx | 4 +++- 3 files changed, 43 insertions(+), 8 deletions(-) (limited to 'bpkg') diff --git a/bpkg/fetch-git.cxx b/bpkg/fetch-git.cxx index 4b1a242..f3c6355 100644 --- a/bpkg/fetch-git.cxx +++ b/bpkg/fetch-git.cxx @@ -654,6 +654,31 @@ namespace bpkg return capabilities::smart; } + // Fail on any other HTTP error (e.g., 404). In the case of a success + // code other than 200 (e.g. 204 (No Content)) just let the capabilities + // detection to take its course. + // + if (ps.second != 0 && (ps.second < 200 || ps.second >= 300)) + { + // Note that we don't care about the process exit code here (see above + // for the reasoning). + // + is.close (); + + // Dump the potentially redirected process stderr stream content since + // it may be helpful to the user. + // + // Note, however, that we don't know if it really contains the error + // description since the fetch program may even exit successfully (see + // start_fetch_http() for details). Thus, we additionally print the + // HTTP status code in the diagnostics. + // + dump_stderr (); + + fail << "unable to fetch " << url << + info << "HTTP status code " << ps.second << endg; + } + string l; getline (is, l); // Is empty if no refs returned by the dumb server. diff --git a/bpkg/fetch.cxx b/bpkg/fetch.cxx index 573dfde..a9e4df2 100644 --- a/bpkg/fetch.cxx +++ b/bpkg/fetch.cxx @@ -317,7 +317,6 @@ namespace bpkg cstrings args { prog.string ().c_str (), - "-f", // Fail on HTTP errors (e.g., 404). "-L", // Follow redirects. "-A", ua.c_str () }; @@ -397,12 +396,24 @@ namespace bpkg // Status code. // + // Add the --include|-i option if HTTP status code needs to be retrieved + // in order to include the HTTP response headers to the output. Otherwise, + // add the --fail|-f option not to print the response body and exit with + // non-zero status code on HTTP error (e.g., 404), so that the caller can + // recognize the request failure. + // + // Note that older versions of curl (e.g., 7.55.1) ignore the --include|-i + // option in the presence of the --fail|-f option on HTTP errors and don't + // print the response status line and headers. + // if (out_is != nullptr) { assert (!fo); // Currently unsupported (see start_fetch() for details). args.push_back ("-i"); } + else + args.push_back ("-f"); args.push_back (url.c_str ()); args.push_back (nullptr); @@ -482,8 +493,8 @@ namespace bpkg assert (e != nullptr); return *e == '\0' && c >= 100 && c < 600 - ? static_cast (c) - : 0; + ? static_cast (c) + : 0; }; // Read the CRLF-terminated line from the stream stripping the trailing @@ -535,10 +546,7 @@ namespace bpkg close_streams (); fail << "invalid HTTP response status line '" << l - << "' while fetching " << url; - - assert (false); // Can't be here. - return 0; + << "' while fetching " << url << endf; }; sc = read_status (); diff --git a/bpkg/fetch.hxx b/bpkg/fetch.hxx index 78e77a7..6578f0d 100644 --- a/bpkg/fetch.hxx +++ b/bpkg/fetch.hxx @@ -156,7 +156,9 @@ namespace bpkg // whether stderr is redirected by checking process::in_efd. // // In contrast to start_fetch() always fetch to stdout, which can be read by - // the caller from the specified stream. + // the caller from the specified stream. On HTTP errors (e.g., 404) this + // stream may contain the error description returned by the server and the + // process may exit with 0 code. // // If the quiet argument is true, then, if stderr is redirected, minimize // the amount of diagnostics printed by the fetch program by only printing -- cgit v1.1