ZstdCompressionWriter.__exit__() not always calling close() is confusing

[cstoitner]

When I first tried to use this library I ended up with code that, when reduced, looked something like this:

if __name__ == "__main__":
    cctx = zstd.ZstdCompressor(level=10)
    with open('workfile', 'wb') as fh, cctx.stream_writer(fh) as compressor:
        compressor.write(b'chunk 0')
        exit(0)

It produced an empty file, and I had no idea what I had done wrong. Out of frustration I added compressor.close() before the exit call - and suddenly it worked! Why did the context manager not call close() for me?

As far as I can tell ZstdCompressionWriter (and probably other classes) will never call close() when an exception is raised inside the with statement (exit() is implemented with an exception). I don't think this is a good idea. In my opinion with statements should always try to properly release their resources. This also goes against the documentation of pythons file objects:

It is good practice to use the with keyword when dealing with file objects. The advantage is that the file is properly closed after its suite finishes, even if an exception is raised at some point.

The current design can easily lead to brittle code that seems to work in the absence of exceptions but will not actually write the whole output to a file as soon as something goes wrong somewhere else - even though the script looks like it has proper error handling implemented.

[indygreg]

This issue is very similar to #76. Should this issue be marked as a duplicate? Should discussion continue in #76?

(I concede that the behavior of context managers and close() currently has some corner cases. There's not a trivial, one-size-fits-all solution.)

[cstoitner]

I think these 2 issues don't quite belong together. Even if file handling is completely removed from ZstdCompressionWriter I still would expect the context manager to always do the same thing in __exit__(). This bug is not so much about what close() actually does, but about __exit__() behaving differently in case of exceptions (this might not have been clear enough in my initial report).

Right now the context manager implementation in ZstdCompressionWriter seems pretty useless to me. I always use an additionally finally: writer.close() block, making the with statement redundant.

[indygreg]

I agree that we should unconditionally call close() from __exit__(). I'll make this change for the next release.