Re: [PATCH] Only print an error for the last connect() failure

From: Jeff King <peff@peff.net>
Date: Jun 9, 2011, 17:05 UTC
Message-ID: <20110609170554.GA29760@sigill.intra.peff.net>
In-Reply-To: <767D7D04-6089-4C7B-A532-C5EC9FE0CCC6@apple.com>

On Thu, Jun 09, 2011 at 09:49:28AM -0700, Dave Zarzycki wrote:

Show 8 quoted lines

> I'm happy to make that change.
> 
> Personally speaking, I don't think we're masking failures any more
> than git is masking failures when it doesn't find a ref in .git/refs
> and it falls back to .git/packed-refs. This fallback is by design, and
> the same is true of any classic loop around getaddrinfo(). Of course,
> reasonable people may disagree about what the "right" thing to do here
> is. :-)

When the fallback actually _works_, then yes, there's no reason to say anything. But when the fallback fails, it can be useful to get information on each of the steps. In your refs analogy, it would just be "I couldn't look up this ref; I tried .git/refs and packed refs". Which would be the same every time, so it's not really interesting enough to print.

But in the case of host lookup (especially with ipv6), it may be very useful to say "I tried this address, and it failed for this reason; then I tried this address, and it failed for this reason". If I see:

  $ git fetch git://example.com/foo
  example.com[0: 192.0.32.10]: errno=Connection timed out
  example.com[0: 2620:0:2d0:200::10]: errno=Network is unreachable
  fatal: unable to connect a socket (Network is unreachable)

then I don't care about the second error. Of course the IPv6 network is unreachable; I don't have IPv6 connectivity. The first line contains the useful information. But with your patch, it won't be shown at all.

We should perhaps also always print intermediate error messages as we get them in verbose mode. Even if we succeed, seeing connection timeouts on earlier addresses can explain long pauses before success (but I agree they shouldn't be shown by default).

-Peff