fix: Added multiselect type consistency in negotiate method
#814
Conversation
…es to handle none
unniznd
changed the title
negotiate methodnegotiate method
|
Looks good, @unniznd. Please add a newsfragment and it should be good to go. |
|
@pacrob Added the newsfragment. |
Signed-off-by: varun-r-mallya <[email protected]>
Signed-off-by: varun-r-mallya <[email protected]>
Signed-off-by: varun-r-mallya <[email protected]>
This reverts commit 400ee9b.
This reverts commit 2730db4.
|
I will create a new PR. Small mistake, I did rebase. |
There was a problem hiding this comment.
Thanks, @unniznd This fix addresses a legitimate type consistency issue and improves error handling throughout the codebase. The implementation is clean, focused, and maintains backward compatibility.
Can you please chore the error messages text and can you add some test to see that the None case is well handled ?
Verify
# basic_host.py
raise StreamFailure("failed to negotiate protocol")
# security_multistream.py
raise SecurityUpgradeFailure("failed to negotiate the secure protocol")
# muxer_multistream.py
raise MuxerUpgradeFailure("failed to negotiate the multiplexer protocol")
```| @@ -288,6 +288,9 @@ async def _swarm_stream_handler(self, net_stream: INetStream) -> None: | |||
| protocol, handler = await self.multiselect.negotiate( | |||
| MultiselectCommunicator(net_stream), self.negotiate_timeout | |||
| ) | |||
| if protocol is None: | |||
| await net_stream.reset() | |||
| raise StreamFailure("No protocol selected") | |||
There was a problem hiding this comment.
For error messages, can you be more verbose, something like:
raise StreamFailure("Failed to negotiate protocol: no protocol selected")
| @@ -114,5 +117,7 @@ async def select_transport( | |||
| else: | |||
| # Select protocol if non-initiator | |||
| protocol, _ = await self.multiselect.negotiate(communicator) | |||
| if protocol is None: | |||
| raise SecurityUpgradeFailure("No protocol selected") | |||
There was a problem hiding this comment.
Same.
raise SecurityUpgradeFailure("failed to negotiate the secure protocol")
| communicator = MultiselectCommunicator(conn) | ||
| if conn.is_initiator: | ||
| protocol = await self.multiselect_client.select_one_of( | ||
| tuple(self.transports.keys()), communicator | ||
| ) | ||
| else: | ||
| protocol, _ = await self.multiselect.negotiate(communicator) | ||
| if protocol is None: | ||
| raise MuxerUpgradeFailure("No protocol selected") |
There was a problem hiding this comment.
Same, somethink like:
aise MuxerUpgradeFailure("failed to negotiate the multiplexer protocol")
What was wrong?
The multiselect type was inconsistent
How was it fixed?
Set the return type to None and handled the places where the
negotiatefunction is used.Summary of approach.
To-Do
Cute Animal Picture