buffer chunk after flushing buffer in write if paused#28
Closed
everett1992 wants to merge 2 commits into
Closed
Conversation
Reproduces isaacs#27, cause of npm/cli#3884
4c5a106 handled a convoluted case where there is a chunk in the buffer AND we're in a flowing state during a write call which caused out of order writes. The fix was to flush the buffer before emitting the new chunk, but it didn't account for destinations pausing the stream after flushing part of the buffer. This caused issues in npm/pacote/npm-registry-fetch. That specific issue is demonstrated in everett1992/make-fetch-happen-tar-extract-error and occurs when make-fetch-happen res.body is piped to a tar.extract stream. Fixes isaacs#27 npm/cli#3884 make-fetch-happen#63
|
this is a most excellent fix! i'm totally impressed with how far you went to chase this one down, and your patch here is undoubtedly the correct behavior. thank you for your hard work on this one! |
Author
|
Anything I need to do to ship this? And after it's shipped how do we update npm's dependencies and include that npm release in node? |
Owner
|
Thanks! Great catch! |
1 task
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
4c5a106 handled a convoluted case where there is a chunk in the buffer
AND we're in a flowing state during a write call which caused out of
order writes.
The fix was to flush the buffer before emitting the new chunk, but it
didn't account for destinations pausing the stream after flushing part
of the buffer. This caused issues in npm / pacote / npm-registry-fetch.
That specific issue is demonstrated in everett1992/make-fetch-happen-tar-extract-error
and occurs when make-fetch-happen res.body is piped to a tar.extract
stream.
Fixes #27, npm/cli#3884, npm/make-fetch-happen#63