Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
feat(swarm): allow
NetworkBehaviour
s to create and remove listeners #3292feat(swarm): allow
NetworkBehaviour
s to create and remove listeners #3292Changes from 10 commits
4f3b233
d7cc720
14d0c49
518295d
f9f50f8
ed11db4
eaff6f0
d104545
1892fd8
ee4cf30
a5331cb
ced5bb0
9d2db40
b26f4fc
84895ef
ad8856e
df1a19c
ec128ac
97be0c6
8cbf386
6c163a6
b326e32
0b0f04e
ae1089f
590f198
97ec46d
48523bf
fa64bca
4c9b192
124a150
1f6a798
15fd6e4
c8968e0
1e40992
82a9898
4d2946d
d5e36f5
3e09dc0
2666615
95e61ab
f880ba2
7068f12
0bd41ab
f4c196f
bc789b2
5ea58d1
603933c
08d7659
92bf3aa
64d0ad4
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If this fails, the
NetworkBehaviour
will never learn about it. I think we need to capture this error and report it back to theNetworkBehaviour
as well as issue aListenerError
.The documentation on
ListenerError
talks about "non-fatal" errors but I think adding another variant is even more confusing.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was thinking about this back during the initial PR, but I assumed it would be alright since
ToSwarm::Dial
ignores the error. We could do this insteadSo it would notify the behaviour of the error instead of ignoring it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are we? Probably also something that should be fixed then :)
Yep, that looks good to me!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looking at
Swarm::dial
, it does send the error to the behaviour internally before returning the error itself so it doesnt look to be necessaryEDIT: Overlooked the beginning, but it looks like it doesnt send anything for
dial_opts.get_or_parse_peer_id().map_err(DialError::InvalidPeerId)
, but not sure if this would need to be sent to the behaviour. If it does we could do this internally there in the function by doingThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That error will go away once we have multiformats/rust-multiaddr#73.