On Mon, May 20, 2024 at 09:09:26PM +0000, Boris Mbarga via GitGitGadget wrote:
> From: elhmn <elhmn@xxxxxxxxxx>
>
> When Git sends a GET request and receives an HTTP code indicating
> failure (and that failure does not indicate an authentication problem),
> it shows the body of the response, i.e. the error message.
Hmm, do we always do so? Long ago, I implemented that for the initial
HTTP connection in 426e70d4a1 (remote-curl: show server content on http
errors, 2013-04-05). That disables CURLOPT_FAILONERROR for that request,
and then shows the result even if we saw an error.
After that, we turned off CURLOPT_FAILONERROR for all requests in
e6cf87b12d (http: enable keep_error for HTTP requests, 2019-01-10), with
the rationale that we'd show them with GIT_CURL_VERBOSE (and presumably
GIT_TRACE_CURL, too). But do we actually write them out in most cases?
I'm not opposed to doing so, but just trying to understand what the
implications might be (and whether we are really bringing POSTs in line,
or if this is a new area).
In particular...
> @@ -837,6 +840,8 @@ static int run_slot(struct active_request_slot *slot,
> strbuf_addch(&msg, ' ');
> strbuf_addstr(&msg, curl_errorstr);
> }
> + if (slot->errstr && slot->errstr->len)
> + error(_("%s"), slot->errstr->buf);
> }
> error(_("RPC failed; %s"), msg.buf);
> strbuf_release(&msg);
If I understand correctly, slot->errstr is just the raw body content
returned by the request. That _might_ be something human-readable, but
it might not. For that initial connection, we have show_http_message(),
which shows only messages that come back as text/plain, does some light
cleanup, and shows the error with the "remote:" prefix. We'd want to use
that here, I'd think?
-Peff
