Skip to content
This repository has been archived by the owner on Apr 24, 2023. It is now read-only.

Unskip subset of libp2p-interface tests for transport #214

Closed
vasco-santos opened this issue Apr 21, 2020 · 5 comments · Fixed by #244
Closed

Unskip subset of libp2p-interface tests for transport #214

vasco-santos opened this issue Apr 21, 2020 · 5 comments · Fixed by #244
Labels
help wanted Seeking public contribution on this issue kind/bug A bug in existing code (including security flaws) status/ready Ready to be worked

Comments

@vasco-santos
Copy link
Member

This module is currently not using the latest libp2p-interfaces module, but a tag with a few tests skipped. Context #183 (comment)

We should use the latest tests and fix these issues as discussed in #213

@vasco-santos vasco-santos added kind/bug A bug in existing code (including security flaws) help wanted Seeking public contribution on this issue status/ready Ready to be worked labels Apr 21, 2020
@AustinFoss
Copy link

AustinFoss commented Apr 25, 2020

Oooooooh! This explains why I've been bashing my head in trying to figure out why my project suddenly stopped working that morning. I've never really followed a dev chat on github before but just wondering if this will be returned to emitting a PeerInfo object or should I adjust my code to implement this new object format of just the id & multiaddrs?

@vasco-santos
Copy link
Member Author

Hey @AustinFoss

Can you let me know when that happen? I am not sure this is the reason for that, since we are skipping these tests since last year (#183 (comment)). In addition, this module did not support in the past the features that are being skipped on the tests.

The goal is to deprecate PeerInfo, as described in libp2p/js-libp2p#589. If you are using this module with libp2p@0.27, you should only remove the PeerInfo when we release libp2p@0.28 and you update it. If you are simply using this module directly, you can already update, but bear in mind that this is still in a beta release.:

Current Tags:
0.17.9 - latest
0.18.0 - beta

We will move it to latest once we release libp2p@0.28

@AustinFoss
Copy link

AustinFoss commented Apr 27, 2020

Preface the following with that I'm pretty sure I went to bed on the 20, saved, woke up on the 21st and then my project wasn't connecting to peers over webRTC. But there's a chance it's still user(my) error.

You can actually recreate the issue directly from "js-libp2p/examples/libp2p-in-the-browser". You are able to connect to the boot strapped nodes just fine but the connection to any additional nodes opened in subsequent tabs will not connect. They will find each other, but not connect. I could be mistaken, but last week I was building on top of that example and I thought I remember that it would auto dial the webRTC peers as well. The example's Read Me also says that "Your node will be notified of any peer that connects to the same signaling server you are connected to. Once libp2p discovers this new peer, it will attempt to establish a direct WebRTC connection."

So when I finally looked at issue #213(perhaps my comments here actually belong over there), that it read "BREAKING CHANGE: peer event emits an object with id and multiaddr instead of a peer-info", and that it was merged to master the same morning I started having the problem it seemed like too much of a coincidence.

When I insert a console.log(peerInfo) after line 54 in index.js of the example, the emitted objects appear to still be identical in format regardless whether the event was emitted from a discovered Bootstrap or WebRTC peer. So I am still a little confused as to why autodialing of WebRTC peers stopped working.

the example is using 0.17.3 and my project is using 0.17.8 of webRTC

@vasco-santos
Copy link
Member Author

Hello @AustinFoss

I just tried out the libp2p-in-the-browser example with two browsers and everything worked as expected. Are you running the signalling server before running the example?

Regarding versions, #213 was released as beta for 0.18.0 and this should not be installed for you, unless you explicitly installed it. This way, both the example and your project should be using the latest libp2p-webrtc-star release, which is the 0.17.8. The example is installing "libp2p-webrtc-star": "^0.17.3", which will fetch the latest release from 0.17.x.

If you still have issues, I am happy to help. But please open an issue for that, since this issue is not related. This exists since October 2019 and the tests are being skipped because the features being tested by the interface were not supported by this module now. So, this issue consists on implementing the abortable features.

@AustinFoss
Copy link

AustinFoss commented Apr 29, 2020

Are you running the signalling server before running the example?

Yep. Other wise peers wouldn't be found at all. But for some reason the connection to the webRTC discovered peers don't connect, only found.

I did find something that might be causing the issue but I'll expand it in an issue over there. libp2p/js-libp2p#621

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
help wanted Seeking public contribution on this issue kind/bug A bug in existing code (including security flaws) status/ready Ready to be worked
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants