# [PATCH] remote-curl: Include curl_errorstr on SSL setup failures 3 messages from 2016-02-14 to 2016-02-14. Participants: Shawn Pearce, Jeff King. Thread: https://gitlist.dev/t/41408 ## Shawn Pearce, 2016-02-14 01:39 Subject: [PATCH] remote-curl: Include curl_errorstr on SSL setup failures Message-ID: <1455413974-25147-1-git-send-email-spearce@spearce.org> URL: https://gitlist.dev/e/1455413974-25147-1-git-send-email-spearce%40spearce.org ``` For curl error 35 (CURLE_SSL_CONNECT_ERROR) users need the additional text stored in CURLOPT_ERRORBUFFER to debug why the connection did not start. This is curl_errorstr inside of http.c, so include that in the message if it is non-empty. Sometimes HTTP response codes aren't yet available, such as when the SSL setup fails. Don't include HTTP 0 in the message. Signed-off-by: Shawn Pearce --- remote-curl.c | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/remote-curl.c b/remote-curl.c index c704857..f611432 100644 --- a/remote-curl.c +++ b/remote-curl.c @@ -439,8 +439,20 @@ static int run_slot(struct active_request_slot *slot, err = run_one_slot(slot, results); if (err != HTTP_OK && err != HTTP_REAUTH) { - error("RPC failed; result=%d, HTTP code = %ld", - results->curl_result, results->http_code); + struct strbuf msg = STRBUF_INIT; + if (results->http_code && results->http_code != 200) + strbuf_addf(&msg, "HTTP %ld", results->http_code); + if (results->curl_result != CURLE_OK) { + if (msg.len) + strbuf_addch(&msg, ' '); + strbuf_addf(&msg, "curl %d", results->curl_result); + if (curl_errorstr[0]) { + strbuf_addch(&msg, ' '); + strbuf_addstr(&msg, curl_errorstr); + } + } + error("RPC failed; %s", msg.buf); + strbuf_release(&msg); } return err; -- 2.7.0 ``` ## Jeff King, 2016-02-14 16:50 Subject: Re: [PATCH] remote-curl: Include curl_errorstr on SSL setup failures Message-ID: <20160214165048.GA10219@sigill.intra.peff.net> URL: https://gitlist.dev/e/20160214165048.GA10219%40sigill.intra.peff.net In-Reply-To: <1455413974-25147-1-git-send-email-spearce@spearce.org> ``` On Sat, Feb 13, 2016 at 05:39:34PM -0800, Shawn Pearce wrote: > For curl error 35 (CURLE_SSL_CONNECT_ERROR) users need the > additional text stored in CURLOPT_ERRORBUFFER to debug why > the connection did not start. This is curl_errorstr inside > of http.c, so include that in the message if it is non-empty. I think this is an all-around improvement. GitHub sometimes get support requests for result=18 on a git-push, which is curl's code for "I dunno, the remote end hung up". Having a human-readable message may make things less confusing. Unfortunately I cannot seem to create the problem at will to confirm that it kicks in in this case[1], but it seems like it should just based on reading your patch. -Peff [1] I tried inserting "exit(0)" in various places of receive-pack, and it seems make the protocol deadlock. Yikes. ``` ## Shawn Pearce, 2016-02-14 18:04 Subject: Re: [PATCH] remote-curl: Include curl_errorstr on SSL setup failures Message-ID: URL: https://gitlist.dev/e/CAJo%3DhJsks9h-PDxrTYWOh0xEz150-q3gjyGNz9p3qhdqqQUM2A%40mail.gmail.com In-Reply-To: <20160214165048.GA10219@sigill.intra.peff.net> ``` On Sun, Feb 14, 2016 at 8:50 AM, Jeff King wrote: > On Sat, Feb 13, 2016 at 05:39:34PM -0800, Shawn Pearce wrote: > >> For curl error 35 (CURLE_SSL_CONNECT_ERROR) users need the >> additional text stored in CURLOPT_ERRORBUFFER to debug why >> the connection did not start. This is curl_errorstr inside >> of http.c, so include that in the message if it is non-empty. > > I think this is an all-around improvement. GitHub sometimes get support > requests for result=18 on a git-push, which is curl's code for "I dunno, > the remote end hung up". Having a human-readable message may make things > less confusing. I have been trying to recreate the error 35 scenario, but after almost a day of continuous attempts with a suspected broken Git I thus far have not been able to reproduce it. *sigh* What made me pick this up was I got another recent report of a Debian system having trouble connecting to $DAY_JOB's HTTPS server, which reminded me of [2]. I was going to revive that patch, but instead decided to expand on the error reporting. [2] http://article.gmane.org/gmane.comp.version-control.git/206770 > Unfortunately I cannot seem to create the problem at will to confirm > that it kicks in in this case[1], but it seems like it should just based on > reading your patch. > > -Peff > > [1] I tried inserting "exit(0)" in various places of receive-pack, and > it seems make the protocol deadlock. Yikes. This actually doesn't surprise me. *sigh* With all the pipes and libcurl in there, we must somewhere be ignoring the fact that the HTTP server closed the connection. ```