[rfc] [patch] sftp unit tests without ssh

Robey Pointer robey at lag.net
Thu Jan 26 21:04:37 GMT 2006


On 26 Jan 2006, at 8:01, John A Meinel wrote:

> John A Meinel wrote:
>> Robey Pointer wrote:
>>> On 25 Jan 2006, at 10:57, John A Meinel wrote:
>>>
>>>> Robey Pointer wrote:
>>>>> On 24 Jan 2006, at 12:00, John Arbash Meinel wrote:
>>>>>
>>>>>> However, after merging you I get:
>>>>>> ~bzr: WARNING: Exception from within unit test server thread:
>>>>>> <socket.error instance at 0xb6b6894c>
>>>>>> ~bzr: WARNING: Exception from within unit test server thread:
>>>>>> <socket.error instance at 0xb6b68bac>
>>>>>> ~bzr: WARNING: Exception from within unit test server thread:
>>>>>> <socket.error instance at 0xb6b6582c>
>>>>>>
>>>>>> Probably we should also us 'log_exception_quietly()' which puts a
>>>>>> traceback into ~/.bzr.log, which would help debug this.
>>>>> I see these sometimes, though I can't reproduce them right now  
>>>>> (probably
>>>>> laptop slowness).  log_exception_quietly() won't help much,  
>>>>> because the
>>>>> logs are erased right after each test, so you'll never see the
>>>>> exception. :)
>>>>>
>>>>> I think the error is just a shutdown synchronization problem:  
>>>>> one side
>>>>> closing before the other is finished.  Probably the unit test's
>>>>> destructors abruptly closing sockets that the server thinks are  
>>>>> still in
>>>>> play -- which is why the don't cause the test to fail.  Maybe  
>>>>> we should
>>>>> just catch and squelch socket.errors, but I wish I could catch  
>>>>> one in
>>>>> the act just to make sure.
>>>> You could always print the traceback to sys.stdout.
>>>> traceback.print_exc() works just fine. Only run_bzr is setup to  
>>>> redirect
>>>> sys.stdout.
>>> Of course, I was able to reproduce this minutes after I sent the  
>>> email.
>>> :)  Turns out that it's just a socket.error from a failed write  
>>> after
>>> the socket is closed, so it's harmless.  I checked in the  
>>> attached patch
>>> to my bzr.dev.sftp branch to squelch the warning.
>>>
>>> robey
>>>
>>>
>>
>> Is it a time when we are trying to write to a closed socket? Like  
>> on the
>> client side we got an exception an exited, rather than continuing?
>>
>> I'm just worried that if we catch all socket exceptions and throw  
>> them
>> away, we might miss future bugs.


Paramiko implements destructors for file objects, so they can behave  
as much like python file objects as possible,  The destructor does an  
equivalent of "close()" except, since it's being executed in what may  
be an inconvenient place, it doesn't wait for a server response.  It  
makes one quick attempt to send a clean "close the file" request to  
the server, ignores errors, and then returns.

At least a few of the unit tests rely on this behavior instead of  
explicit close() -- which is fine.  It just means the stub "server"  
may try to send a status response to the close() request and find out  
that the socket has already been closed.

I think it's safe to just catch and throw away *all* socket errors in  
the stub server, because it's running in a different thread.  If it's  
a bug in the server that causes an operation to fail, we'll notice in  
the unit test thread and fail an assert.

(The normal behavior for an sftp server that gets disconnected like  
that would be to log a debug message maybe, and then shut down the  
session.  That's effectively what we're doing.)


> Does socket.error have any more specific information, so we can detect
> "connection closed" or something like that? I would really prefer a
> specific error trap, rather than an unspecified one.

It looks like socket.error on posix includes errno info -- unportable  
garbage.  I believe I've heard reports that on Windows it's a wrapper  
for a completely different winsock error object.

But, as I said above, since this is part of the stub sftp server, I  
think it's okay to just catch and ignore all socket errors.  If it  
was in the unit test or SFTPTransport it would be a different story.

robey





More information about the bazaar mailing list