fix unusued fout var in "finally" block in case of exception

[alekseydevel]

No description provided.

[rsichnyi]

@tier2003

1. as discussed, connection.path_io.open(real_path, mode=mode) will raise OSError actually
2. unfortunatelly, stream.read(connection.block_size) and connection.path_io.write(fout, data) can raise OSError as well, but in these cases response code is not 421... So i assume you should expect error only on open and not on other two calls

[pohmelie]

@tier2003

1. There is same behaviour in retr_worker.
2. I think there is more straight and simple solution:

fout = None
try:
    ...
finally:
    if fout is not None:
        ...

[alekseydevel]

@pohmelie ,

1. fixed the exception type and made similar changes to retr_worker method.
2. made not by "not None" checking in order to have more informational errors.
3. I have some doubts about "download" test. The 451 is returned, but I think from the part that saves the file, not the downloading one.

[pohmelie]

Stream will not be closed if we fail on open, I think. None solution is better, I think… or I missed something.

[pohmelie]

Try-except is attractive, but it's not obvious. If it fails, it should be fail, I think.

[alekseydevel]

Well, I`ve started the issue because I had problems with opening during file upload.
The try block had been failing and in finally I was receiving the fout reference of fout before assignment

[alekseydevel]

well, if we don`t need exception mess at that place - maybe if else would be enough

[pohmelie]

@tier2003
I got your problem. And None solution is better, in my mind, cause less deep structure, and less try.
But probably you are right. It is time to think.

[rsichnyi]

maybe combine both?

fout = None
try:
    fout = yield from connection.path_io.open(real_path, mode=mode)
    while True:
        data = yield from stream.read(connection.block_size)
        if not data:
            break
        yield from connection.path_io.write(fout, data)

except OSError:
    connection.response("451", "operation aborted")
else:
    connection.response("226", "data transfer done")
finally:
    stream.close()
    if fout is not None:
        yield from connection.path_io.close(fout)

[pohmelie]

@rsichny
@tier2003
This is good theme to think. We should care about all path-io, socket, etc. operations, that can produce exception, not only this open. For the last code I can say, that path_io.close can raise OSError. That is why I'm in think of centralized error catching.

[pohmelie]

Discussion will continue at #31