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

Suppress libp2p:ip-port-to-multiaddr:err invalid ip:port for creating a multiaddr #147

Merged
merged 2 commits into from
Mar 15, 2022

Conversation

D4nte
Copy link
Contributor

@D4nte D4nte commented Feb 28, 2022

When running https://github.com/libp2p/js-libp2p/tree/master/examples/webrtc-direct with Firefox, the connection fails due to the following error:

14:17:11.471 libp2p:webrtcdirect connection opened 127.0.0.1:9090 +5s common.js:113:9
14:17:11.472 libp2p:ip-port-to-multiaddr:err invalid ip:port for creating a multiaddr: c67a8623-319c-4b31-96fa-e35c6b25a83a.local:58088 +10s common.js:113:9
14:17:11.472 libp2p:dialer token 1 released +5s common.js:113:9
14:17:11.473 libp2p:dialer:err AggregateError@http://localhost:1234/index.93e19648.js:42430:9
maybeSettle@http://localhost:1234/index.93e19648.js:42373:24
.hLj2A</pSome/</<@http://localhost:1234/index.93e19648.js:42394:39
async*.hLj2A</pSome/<@http://localhost:1234/index.93e19648.js:42396:15
.MqLlq</PCancelable/this._promise<@http://localhost:1234/index.93e19648.js:42534:20
PCancelable@http://localhost:1234/index.93e19648.js:42509:25
pSome@http://localhost:1234/index.93e19648.js:42355:36
["6p5iW"]</module.exports@http://localhost:1234/index.93e19648.js:42335:32
run@http://localhost:1234/index.93e19648.js:42257:26
_createPendingDial@http://localhost:1234/index.93e19648.js:42096:34
connectToPeer@http://localhost:1234/index.93e19648.js:41975:75
async*_autoDial@http://localhost:1234/index.93e19648.js:37850:47
Async*.gwg31</Retimer/this._timerWrapper@http://localhost:1234/index.93e19648.js:33295:26
setTimeout handler*Retimer@http://localhost:1234/index.93e19648.js:33298:23
retimer@http://localhost:1234/index.93e19648.js:33330:12
_autoDial@http://localhost:1234/index.93e19648.js:37858:33
Async*start@http://localhost:1234/index.93e19648.js:37810:14
_onDidStart@http://localhost:1234/index.93e19648.js:8822:33
async*start@http://localhost:1234/index.93e19648.js:8592:24
async*.aLC0o</<@http://localhost:1234/index.93e19648.js:576:18
async*.aLC0o<@http://localhost:1234/index.93e19648.js:528:10
newRequire@http://localhost:1234/index.93e19648.js:71:24
@http://localhost:1234/index.93e19648.js:122:15
@http://localhost:1234/index.93e19648.js:145:3
 +10s common.js:113:9

This is due to the fact that toMultiaddr throws an error because socket.localAddress is a fqdn, not an ip (c67a8623-319c-4b31-96fa-e35c6b25a83a.local). It works fine in Chromium because socket.localAddress is undefined.

I contemplated the option to still attempt building an address out of the local fqdn. However, it does not seem possible to know whether it is an ip4 or ip6 address.

socket has localFamily that containsIPv4 in my tests. However, the way it is set is unreliable for fqdn:

https://github.com/ipfs-shipyard/simple-peer/blob/1039b90f91fc8e91492ff1af52f9fc9bfff97c35/index.js#L773

Note: tested with https://github.com/libp2p/js-libp2p/tree/master/examples/webrtc-direct?rgh-link-date=2022-02-28T04%3A20%3A39Z

try {
return toMultiaddr(socket.localAddress, socket.localPort)
} catch {
// Might fail if the socket.localAddress is fqdn
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@wemeetagain what about this catch case? Should it be handled somehow?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we want to ensure we get a multiaddr from a fqdn then we would need to change upstream in simple-peer and find a way to get the ip version of the connection https://github.com/ipfs-shipyard/simple-peer/blob/1039b90f91fc8e91492ff1af52f9fc9bfff97c35/index.js#L773 with a fqdn domain.

I am not convinced that is worth it as it works fine in Chrome with undefined.

@D4nte
Copy link
Contributor Author

D4nte commented Mar 14, 2022

@wemeetagain any chance we could merge this? Thanks!

@wemeetagain wemeetagain merged commit 4525596 into libp2p:master Mar 15, 2022
@D4nte D4nte deleted the invalid-ip-port branch March 15, 2022 03:38
@D4nte
Copy link
Contributor Author

D4nte commented Mar 15, 2022

@wemeetagain thanks! Can we do a release please? I am happy to help, let me know.

@github-actions
Copy link

🎉 This PR is included in version 1.0.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants