Replace return True/False with try/except (fixes #194)#1184
Open
parth-soni07 wants to merge 7 commits intolibp2p:mainfrom
Open
Replace return True/False with try/except (fixes #194)#1184parth-soni07 wants to merge 7 commits intolibp2p:mainfrom
parth-soni07 wants to merge 7 commits intolibp2p:mainfrom
Conversation
Contributor
Author
Contributor
|
@parth-soni07 : Looks great. CCing @acul71 and @sumanjeet0012 for their feedback. We can merge it before the upcoming release on Monday. Please add a newsfragment. |
Contributor
Author
hello @seetadev, I have already pushed a newsfragment in this PR let me know if it needs any changes! Thank You |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
What was wrong?
Issue #194
How was it fixed?
listen()– RaiseOpenConnectionErroron missing/invalid port or failed start instead of returning False; addedtcp_port_strisNonecheck before int() for type safety.listen()return type changed from ->boolto ->None; docstring updated to describe raising on failure.listen()now returns None and raises on failure; removed return True.set_deadline(),set_read_deadline(),set_write_deadline()raiseValueErrorfor negative ttl instead of returning False; ABCset_deadline()updated to ->None.set_stream_handler()– Validateinput: raise HostExceptionfor empty/whitespaceprotocol_id or Nonehandler.set_deadline()and swarmregister_notifee()docstrings updated so they no longer claim aFalsereturn.pytest.raises(ValueError)for invalid ttl; host: test empty protocol_id and None handler; QUIC/websocket: dropsuccess = await listener.listen() / assert success.example_mplex_timeouts.pyusestry/exceptValueError for invalid ttl instead of checking a return value.tcp.pyfixed by shortening the :raises docstring line; test handler made async to satisfy THandler.Refactors selected APIs to raise exceptions on failure instead of returning
True/False, so callers use try/except instead of checking return values.Summary
None).try/except.Changes
1. Listener
listen()(IListener and all implementations)async def listen(...) -> boolreturnedTrueon success,Falseon failure.async def listen(...) -> None; raises (e.g.OpenConnectionError,QUICListenError) on failure.Updated: TCP, QUIC, WebSocket, CircuitV2 listeners + ABC
IListenerinlibp2p/abc.py.2. Mplex stream deadline methods
set_deadline(),set_read_deadline(),set_write_deadline()returnedFalsefor invalidttl(e.g. negative).None; raiseValueErrorfor invalidttl.Updated:
libp2p/stream_muxer/mplex/mplex_stream.py+ ABC inlibp2p/abc.py.3. BasicHost
set_stream_handler()protocol_idorNonehandler; docstring said "return true if successful".HostExceptionfor empty/whitespaceprotocol_idorNonehandler. No boolean return.Updated:
libp2p/host/basic_host.py.4. TCP transport
listen()Falseon missing/invalid port or failed start.OpenConnectionErrorwith a clear message. Docstring shortened for line length (E501).Updated:
libp2p/transport/tcp/tcp.py.5. Docstrings only
set_deadline()and swarmregister_notifee(): docstrings updated so they no longer claim aFalsereturn.is_closed: docstring clarified.Tests
test_tcp_listener_raises_on_missing_port— assertsOpenConnectionErrorwhen port is missing.pytest.raises(ValueError)for invalidttl; valid calls expect no exception.test_set_stream_handler_raises_on_empty_protocol_id,test_set_stream_handler_raises_on_null_handler.success = await listener.listen(...); assert success.News fragment
See
newsfragments/194.bugfix.rst.