-
Notifications
You must be signed in to change notification settings - Fork 8
refactor: switch to async iterators #81
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
base: master
Are you sure you want to change the base?
Conversation
BREAKING CHANGE: Switch to using async/await and async iterators. The transport and connection interfaces have changed.
4d249c5
to
585ea74
Compare
585ea74
to
c707499
Compare
c707499
to
252166e
Compare
@jacobheun similarly to what we experienced in So, I believe that there is a bug in alanshaw/stream-to-it/ using |
This seems to be the type of error that will haunt us for life (been there, had to open many cans of worms). To move forward, there should be a reproducible test case (or test cases) that show this behavior on the stream-to-it module and a respective comment (and issue describing the situation) on the transports (this and WebRTC) that that test needs to pass in order for the tests here to pass. |
I created some tests in vasco-santos/stream-to-it@872e44b In this case here, after the abort is performed, the |
Consider creating a test with libp2p-tcp/websockets in stream-to-it to ensure that error gets caught. |
With In |
I'm looking into it, it should work. I think there is just some order of operations issue ending the connections that's causing them to stay open. |
There may be some issues with utp-native. Oddly, the outbound connection emits the |
So, I believe the problem is that I did a quick hack locally of utp-native to use the following and things are passing for me locally. (These changes aren't verified against the rest of the utp-native code) Connection.prototype._destroy = function (err, cb) {
if (this._needsConnect) {
process.nextTick(onbindingclose, this)
return cb(err)
}
binding.utp_napi_connection_close(this._handle)
cb(err)
} |
BREAKING CHANGE: Switch to using async/await and async iterators. The transport and connection interfaces have changed.