feat!: add stream muxer close #254
Conversation
wemeetagain
force-pushed
the
feat/stream-muxer-close
branch
4 times, most recently
from
5b7f6bc
to
fb70709
Compare
| this.streams.forEach(s => s.abort(err)) | ||
| } | ||
| this.closeController.abort() | ||
| this.input.end(err) |
There was a problem hiding this comment.
Calling this.input.end(err) may not be necessary here.
I think what will happen is calling this.closeController.abort() will cause the pipe in sink to throw, the error will be caught and passed to this.input.end on line 316.
Ending the input should then cause the connection to close? It's not clear what use a muxed connection with a closed muxer is, so that's probably good.
There was a problem hiding this comment.
Right, I think the only big difference is that by calling this.input.end(err) explicitly, the error is passed to the source.
If we rely on the pipe in sink to throw, the error that the source will get will be some generic "aborted" error.
Not sure which is the desired behavior, or that either behavior matters to the interface.
There was a problem hiding this comment.
Maybe this.input.end(err) is better - it's not async and the received error is more useful than 'aborted'.
packages/interface-stream-muxer-compliance-tests/src/close-test.ts
Outdated
Show resolved
Hide resolved
wemeetagain
force-pushed
the
feat/stream-muxer-close
branch
2 times, most recently
from
f184744
to
619fae7
Compare
BREAKING CHANGE: StreamMuxer now has a `close` method
wemeetagain
force-pushed
the
feat/stream-muxer-close
branch
from
619fae7
to
dd83845
Compare
## [@libp2p/interface-stream-muxer-v2.0.0](https://github.com/libp2p/js-libp2p-interfaces/compare/@libp2p/interface-stream-muxer-v1.1.0...@libp2p/interface-stream-muxer-v2.0.0) (2022-06-22) ### ⚠ BREAKING CHANGES * StreamMuxer now has a `close` method ### Features * add stream muxer close ([#254](#254)) ([d1f511e](d1f511e))
|
🎉 This PR is included in version @libp2p/interface-stream-muxer-v2.0.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
| if (this.closeController.signal.aborted) return | ||
| this.log('closing muxed streams') | ||
|
|
||
| if (err == null) { |
There was a problem hiding this comment.
I think there's a linter rule against implicitly coersing to a boolean.
foo == null does the same tho, will match on both null and undefined.
## [@libp2p/interface-stream-muxer-compliance-tests-v3.0.0](https://github.com/libp2p/js-libp2p-interfaces/compare/@libp2p/interface-stream-muxer-compliance-tests-v2.1.0...@libp2p/interface-stream-muxer-compliance-tests-v3.0.0) (2022-06-24) ### ⚠ BREAKING CHANGES * StreamMuxer now has a `close` method ### Features * add stream muxer close ([#254](#254)) ([d1f511e](d1f511e)) ### Trivial Changes * update sibling dependencies [skip ci] ([c522241](c522241))
|
🎉 This PR is included in version @libp2p/interface-stream-muxer-compliance-tests-v3.0.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
## [@libp2p/interface-mocks-v3.0.0](https://github.com/libp2p/js-libp2p-interfaces/compare/@libp2p/interface-mocks-v2.1.0...@libp2p/interface-mocks-v3.0.0) (2022-06-24) ### ⚠ BREAKING CHANGES * StreamMuxer now has a `close` method ### Features * add stream muxer close ([#254](#254)) ([d1f511e](d1f511e)) ### Trivial Changes * update sibling dependencies [skip ci] ([7f7fb67](7f7fb67)) * update sibling dependencies [skip ci] ([c522241](c522241))
|
🎉 This PR is included in version @libp2p/interface-mocks-v3.0.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
BREAKING CHANGE: StreamMuxer now has a
closemethodResolves #249