-
Notifications
You must be signed in to change notification settings - Fork 108
Fix decoding of empty response streams. #444
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
Conversation
Previously we were returning "zstd stream did not finish", but now we notice we got no data and transition directly to the done state.
Summary: Work around a bug in the async-compression crate by handling empty zstd payloads specially instead of using the streaming decoder. D91803401 upgraded the async-compression Rust crate, introducing a bug where the streaming decoder does not handle empty streams. Me and claude came up with this workaround where we avoid the streaming decode when there is no data. I opened Nullus157/async-compression#444 for upstream fix. Reviewed By: genevievehelsel Differential Revision: D92462715 fbshipit-source-id: 947d70bb28b88510fbd0340b97a61a4e612081ff
NobodyXu
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you!
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #444 +/- ##
===========================
===========================
☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
## 🤖 New release * `async-compression`: 0.4.37 -> 0.4.38 (✓ API compatible changes) <details><summary><i><b>Changelog</b></i></summary><p> <blockquote> ## [0.4.38](async-compression-v0.4.37...async-compression-v0.4.38) - 2026-02-06 ### Other - Fix decoding of empty response streams. ([#444](#444)) - *(deps)* update proptest-derive requirement from 0.7 to 0.8 ([#442](#442)) </blockquote> </p></details> --- This PR was generated with [release-plz](https://github.com/release-plz/release-plz/). Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
|
almost duplicate of #349 I personally think the same point applies here, which is that it's an expected error: |
|
Hmm, interesting. I was thinking about an "empty HTTP response body" but hadn't considered that an empty body encoded with zstd is actually not empty, presumably. Maybe our issue is on the server side. I will note that after downgrading this library, the error went away. But maybe that was incorrect behavior all along. |
|
Okay - I think the issue was the server sending an invalid Content-Length:0 body when Content-Encoding:zstd. Sorry for the noise - we should revert this. |
Previously we were returning "zstd stream did not finish", but now we notice we got no data and transition directly to the done state.