fix!: close streams gracefully #1864
Conversation
achingbrain
force-pushed
the
fix/close-streams-gracefully
branch
from
7957e84
to
ff1c673
Compare
| - [License](#license) | ||
| - [Contribution](#contribution) | ||
|
|
||
| ## Install | ||
| ## Usage |
There was a problem hiding this comment.
This is helpful information but I think it would be more appropriate to open a PR against test plan's README since that repo is responsible for this workflow
There was a problem hiding this comment.
I think the audiences are different.
The README in this repo is useful for people who want to run the tests with their local copy of js-libp2p+changes against release versions of other libp2p implementations and is what the usage examples have in mind.
The users of the test plan repo are a little less well defined.
There was a problem hiding this comment.
My understanding is that if someone wants to test their js-libp2p changes against other libp2p implementation versions, they would follow a pattern similar to the one utilized here and run the changes following the README
There was a problem hiding this comment.
I did not find the documentation in the test plans repo to be sufficient to actually run the tests locally; a number of extra steps were required.
Perhaps we can consolidate these in a future PR.
|
I assume we had to the encryption libraries here as well for a similar reason to #1856 (comment) ? |
Yes, that's correct - they'll be removed after release. |
achingbrain
force-pushed
the
fix/close-streams-gracefully
branch
3 times, most recently
from
a4d77b5
to
e1e55d3
Compare
- Refactors `.close`, `closeRead` and `.closeWrite` methods on the `Stream` interface to be async - The `Connection` interface now has `.close` and `.abort` methods - `.close` on `Stream`s and `Connection`s wait for the internal message queues to empty before closing - `.abort` on `Stream`s and `Connection`s close the underlying stream immediately and discards any unsent data - `@chainsafe/libp2p-yamux` now uses the `AbstractStream` class from `@libp2p/interface` the same as `@libp2p/mplex` and `@libp2p/webrtc` Follow-up PRs will be necessary to `@chainsafe/libp2p-yamux`, `@chainsafe/libp2p-gossipsub` and `@chainsafe/libp2p-noise` though they will not block the release as their code is temporarily added to this repo to let CI run. Fixes #1793 Fixes #656 BREAKING CHANGE: the `.close`, `closeRead` and `closeWrite` methods on the `Stream` interface are now asynchronous
achingbrain
force-pushed
the
fix/close-streams-gracefully
branch
from
e1e55d3
to
a541748
Compare
.close,closeReadand.closeWritemethods on theStreaminterface to be asyncConnectioninterface now has.closeand.abortmethods.closeonStreams andConnections wait for the internal message queues to empty before closing.abortonStreams andConnections close the underlying stream immediately and discards any unsent data.resetis removed from theStreaminterface - instead call.abort(err)to signal a local error.resetis still present on theAbstractStreamclass - the muxer implementation should call this to signal a remote error@chainsafe/libp2p-yamuxnow uses theAbstractStreamclass from@libp2p/interfacethe same as@libp2p/mplexand@libp2p/webrtc- all the logic around the *checks notes* 17 different ways to close a stream is contained thereFollow-up PRs will be necessary to
@chainsafe/libp2p-yamux,@chainsafe/libp2p-gossipsuband@chainsafe/libp2p-noisethough they will not block the release as their code is temporarily added to this repo to let CI run.Fixes #1793
Fixes #656
BREAKING CHANGE: the
.close,closeReadandcloseWritemethods on theStreaminterface are now asynchronous