Skip to content

Fix decoding of empty response streams.#444

Merged
NobodyXu merged 1 commit intoNullus157:mainfrom
muirdm:fix-empty-streaming-response
Feb 6, 2026
Merged

Fix decoding of empty response streams.#444
NobodyXu merged 1 commit intoNullus157:mainfrom
muirdm:fix-empty-streaming-response

Conversation

@muirdm
Copy link
Contributor

@muirdm muirdm commented Feb 6, 2026

Previously we were returning "zstd stream did not finish", but now we notice we got no data and transition directly to the done state.

Previously we were returning "zstd stream did not finish", but now we notice we got no data and transition directly to the done state.
meta-codesync bot pushed a commit to facebook/sapling that referenced this pull request Feb 6, 2026
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
Copy link
Collaborator

@NobodyXu NobodyXu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you!

@NobodyXu NobodyXu added this pull request to the merge queue Feb 6, 2026
Merged via the queue into Nullus157:main with commit e67f0b1 Feb 6, 2026
23 checks passed
@codecov
Copy link

codecov bot commented Feb 6, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 0.00%. Comparing base (f5e532e) to head (ee7f86c).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files
@@     Coverage Diff     @@
##   main   #444   +/-   ##
===========================
===========================

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

github-merge-queue bot pushed a commit that referenced this pull request Feb 6, 2026
## 🤖 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>
@robjtede
Copy link
Member

robjtede commented Feb 6, 2026

almost duplicate of #349

I personally think the same point applies here, which is that it's an expected error:

 › touch a.zstd
 › unzstd a.zstd
zstd: a.zstd: unexpected end of file

@muirdm
Copy link
Contributor Author

muirdm commented Feb 6, 2026

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.

@muirdm
Copy link
Contributor Author

muirdm commented Feb 6, 2026

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.

NobodyXu added a commit that referenced this pull request Feb 7, 2026
github-merge-queue bot pushed a commit that referenced this pull request Feb 7, 2026
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 this pull request may close these issues.

3 participants