Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

no non-internal way to catch exception from decompress? #9

Closed
elaforge opened this issue Oct 29, 2016 · 16 comments
Closed

no non-internal way to catch exception from decompress? #9

elaforge opened this issue Oct 29, 2016 · 16 comments

Comments

@elaforge
Copy link

I was surprised to discover (via a production crash) that Zlib.decompress throws its own exception, not an IOError. Also, it looks like the only place its exception is exported is Codec.Compression.Zlib.Internal, which implies I shouldn't be using it normally.

How about re-exporting the exception from Zlib and GZip, and then augmenting the documentation to say exactly what exception will be thrown? As a bonus it would be nice to have an example, because I initially thought ByteString.Lazy.toStrict should be enough, but you also need an Exception.evaluate in there.

I can make a pull request if you want.

@NorfairKing
Copy link

+1, would also like a Maybe ByteString or Either ZLibException ByteString version of the API.

@dcoutts
Copy link
Contributor

dcoutts commented Mar 29, 2017

The new incremental API is stable enough that we should promote it from the internal module.

@hvr
Copy link
Member

hvr commented Mar 30, 2017

@dcoutts wait... we still need #6 which would break the compression-side of the API ;-)

@ocramz
Copy link

ocramz commented Nov 23, 2017

@hvr @dcoutts I think it would be sufficient to export a decompressM that uses foldDecompressStream and some variation of throwM. Explicit exception handling is especially crucial for processing http responses, which are gzipped only if successful. Would you accept such a patch?

@ocramz
Copy link

ocramz commented Mar 14, 2018

@hvr @dcoutts If I understand correctly, the latest version of zlib released on March 8 doesn't carry add this functionality. Is it in the development plans for the foreseeable future, or could you please advise what other zlib/gzip streaming bindings expose a monadic interface that throws relevant exceptions ?

@hvr
Copy link
Member

hvr commented Mar 14, 2018

I think there's a good chance that the next one will... personally I'd like to see zlib's API be harmonised with http://hackage.haskell.org/package/lzma-0.0.0.3/docs/Codec-Compression-Lzma.html but I still need to negotiate w/ Duncan :-)

@ocramz
Copy link

ocramz commented Jul 4, 2018

@dcoutts @hvr is there anything I can do to help with this issue? Perhaps you have an idea on how should the common zlib/lzma interface evolve and can advise for a PR? Thanks!

@ocramz
Copy link

ocramz commented Jan 26, 2019

Ping !

tfausak added a commit to tfausak/zlib that referenced this issue Mar 9, 2019
This fixes haskell#9. It also re-implements `compress` in terms of `safeCompress`
so that the two functions will not drift over time.
@adamglowacki
Copy link

What are the plans? I would appreciate either providing a safe decompress function or moving streaming functions out of .Internal. So that I feel more confident when using them...

@Bodigrim
Copy link
Contributor

No plans, unless someone is passionate enough to prepare a PR for discussion.

@adamglowacki
Copy link

adamglowacki commented Oct 30, 2023 via email

@Bodigrim
Copy link
Contributor

But if it's no longer "internal", I can sleep sound not being afraid that one day it will be changed without a warning.

I don't follow. What makes you think that Codec.Compression.Zlib.Internal is less stable than the rest of the package?

@adamglowacki
Copy link

adamglowacki commented Oct 30, 2023 via email

@Bodigrim
Copy link
Contributor

I still don't get the link between "internal" aka "low-level interface" and supposed stability.

Any part of this package could be changed at any time, no hard guarantees indeed. The only promise is that such change will be properly reflected by bumping the major version, as PVP requires, and *.Internal namespace is no different.

@adamglowacki
Copy link

adamglowacki commented Oct 30, 2023 via email

@Bodigrim
Copy link
Contributor

Bodigrim commented Feb 6, 2024

Fixed in 10f8392.

@Bodigrim Bodigrim closed this as completed Feb 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants