Re: [EGIT PATCH 1/3] Give NoRemoteRepositoryException better message in BasePackConnection

["Shawn O. Pearce" <spearce@spearce.org>]

Marek Zawirski <marek.zawirski@gmail.com> wrote:
> Shawn O. Pearce wrote:
> (...)
>> +	@Override
>> +	protected TransportException noRepository() {
>> +		// Sadly we cannot tell the "invalid URI" case from "push not allowed".
>> +		// Opening a fetch connection can help us tell the difference, as any
>> +		// useful repository is going to support fetch if it also would allow
>> +		// push. So if fetch throws NoRemoteRepositoryException we know the
>> +		// URI is wrong. Otherwise we can correctly state push isn't allowed
>> +		// as the fetch connection opened successfully.
>> +		//
>> +		try {
>> +			transport.openFetch().close();
>> +		} catch (NotSupportedException e) {
>> +			// Fall through.
>> +		} catch (NoRemoteRepositoryException e) {
>> +			// Fetch concluded the repository doesn't exist.
>> +			//
>> +			return e;
>> +		} catch (TransportException e) {
>> +			// Fall through.
>> +		}
>> +		return new TransportException(uri, "push not permitted");
>> +	}
>> +
>
> Nice idea, even if it's crazy and time-consuming, it's probably better  
> than my previous one.

I'm not too worried about the extra time used here.

This happens only after we have already opened a connection and
received no refs at all from the remote peer.  So the user has
already had to wait to get this far.

By asking the same transport to open the fetch we can reuse an
existing SSH tunnel for the new command if this is an SSH connection,
so the setup costs are a lot lower then the original connection.

We are already in a bad error condition; we cannot continue and
the user is about to get an error.  I would rather give them the
best error message we can determine than abort early and give them
something misleading.

-- 
Shawn.