libp2p / libp2p/py-libp2p

Some nursery task exceptions are not caught in unit tests

Open
#604 1 comment 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Dominant language
Python
Stars
624
Forks
256
Avg merge
1d 34m
Merged PRs (30d)
47

Description

Summary
  • Commit info:
  • Hash: e076a038bccd7506e28e08a1b317a99bd361792b
  • Date: Mon May 12 13:46:14 2025 -0600

In unit tests, under certain circumstances, the exceptions raised by trio.nursery tasks are not caught. Some of these exception are fatal and crash the process in mission mode.

Expected behavior

In unit tests, an uncaught trio.nursery task exception results in cancelling all sibling tasks (in the same nursery) and the exception bubbles up. If this happens inside a unit test (decorated with @pytest.mark.trio), the test should fail and it does in many scenarios in py-libp2p.

Actual behavior

We see that under some circumstances, fatal nursery exceptions are not caught and the test passes. See below to reproduce the problem.

To be exact, all handshaking exceptions eventually bubble up to the local handler() function in TCPListener.listen(). As of now, this function (run in a nursery) does not catch exceptions. #593 fixes this.

Relevant log output

This part should run without any issues:

git clone https://github.com/libp2p/py-libp2p.git py-libp2p_trio_nursery_task_exceptions
cd py-libp2p_trio_nursery_task_exceptions
git log -1
# commit e076a038bccd7506e28e08a1b317a99bd361792b (HEAD -> main, tag: v0.2.6, origin/main, origin/HEAD)
# Author: pacrob <5199899+pacrob@users.noreply.github.com>
# Date:   Mon May 12 13:46:14 2025 -0600

python3 -m venv venv
source venv/bin/activate
python3 -m pip install -e ".[dev]"
pytest tests/core/network/test_swarm.py
# ================================================================ test session starts ================================================================
# platform linux -- Python 3.12.3, pytest-8.3.5, pluggy-1.5.0 -- /home/rameli/work/py-libp2p_trio_nursery_task_exceptions/venv/bin/python3
# cachedir: .pytest_cache
# rootdir: /home/rameli/work/py-libp2p_trio_nursery_task_exceptions
# configfile: pyproject.toml
# plugins: Faker-37.3.0, anyio-1.4.0, trio-0.8.0, xdist-3.6.1
# collected 4 items                                                                                                                                   
# 
# tests/core/network/test_swarm.py::test_swarm_dial_peer PASSED                                                                                 [ 25%]
# tests/core/network/test_swarm.py::test_swarm_close_peer PASSED                                                                                [ 50%]
# tests/core/network/test_swarm.py::test_swarm_remove_conn PASSED                                                                               [ 75%]
# tests/core/network/test_swarm.py::test_swarm_multiaddr PASSED                                                                                 [100%]
# 
# =============================================================== slowest 50 durations ================================================================
# 1.43s call     tests/core/network/test_swarm.py::test_swarm_close_peer
# 0.70s call     tests/core/network/test_swarm.py::test_swarm_remove_conn
# 0.54s call     tests/core/network/test_swarm.py::test_swarm_multiaddr
# 0.51s call     tests/core/network/test_swarm.py::test_swarm_dial_peer
# 0.04s setup    tests/core/network/test_swarm.py::test_swarm_dial_peer
# 
# (7 durations < 0.005s hidden.  Use -vv to show these durations.)
# ================================================================= 4 passed in 3.26s =================================================================

All 4 tests should pass without any issues.
Now, we introduce a new test in test_swarm.py that should fail but doesn't.

This test (see below) should fail because the security_protocol is different between the two swarms resulting in a SecurityUpgradeFailure followed by a SwarmException in connection handler in Swarm.listen().

The connection handler runs in an independent task whose failure is not caught by the unit test.

In the following unit test, all exceptions from src_swarm.dial_peer() are intentionally suppressed to show that the nursery exceptions are missed.

cat <<EOF >> tests/core/network/test_swarm.py


from libp2p.security.noise.transport import PROTOCOL_ID as NOISE_PROTOCOL_ID
from libp2p.security.insecure.transport import PLAINTEXT_PROTOCOL_ID

@pytest.mark.trio
async def test_swarm_connection_handler_failure_uncaught():
    async with SwarmFactory.create_and_listen(security_protocol = NOISE_PROTOCOL_ID) as src_swarm:
        async with SwarmFactory.create_and_listen(security_protocol = PLAINTEXT_PROTOCOL_ID) as dest_swarm:
            dest_peer_id = dest_swarm.get_peer_id()
            dest_addrs = tuple(
                addr
                for transport in dest_swarm.listeners.values()
                for addr in transport.get_addrs()
            )
            
            try:
                src_swarm.peerstore.add_addrs(dest_peer_id, dest_addrs, 10000)
                await src_swarm.dial_peer(dest_peer_id)
            except:
                pass
EOF

Now, we run the test that should fail but doesn't:

pytest tests/core/network/test_swarm.py::test_swarm_connection_handler_failure_uncaught
#======================================================================================== test session starts ========================================================================================
#platform linux -- Python 3.12.3, pytest-8.3.5, pluggy-1.5.0 -- /home/rameli/work/py-libp2p_trio_nursery_task_exceptions/venv/bin/python3
#cachedir: .pytest_cache
#rootdir: /home/rameli/work/py-libp2p_trio_nursery_task_exceptions
#configfile: pyproject.toml
#plugins: Faker-37.3.0, anyio-1.4.0, trio-0.8.0, xdist-3.6.1
#collected 1 item                                                                                                                                                                                    
#
#tests/core/network/test_swarm.py::test_swarm_connection_handler_failure_uncaught PASSED                                                                                                       [100%]
#
#======================================================================================= slowest 50 durations ========================================================================================
#0.97s call     tests/core/network/test_swarm.py::test_swarm_connection_handler_failure_uncaught
#0.02s setup    tests/core/network/test_swarm.py::test_swarm_connection_handler_failure_uncaught
#
#(1 durations < 0.005s hidden.  Use -vv to show these durations.)
#========================================================================================= 1 passed in 1.00s =========================================================================================
Possible Solution

As shown below, in the case of an exception during security handshake, SecurityUpgradeFailure should be caught in conn_handler() defined in Swarm.listen() but that is not the case when running unit tests.

            async def conn_handler(
                read_write_closer: ReadWriteCloser, maddr: Multiaddr = maddr
            ) -> None:
                raw_conn = RawConnection(read_write_closer, False)

                # Per, https://discuss.libp2p.io/t/multistream-security/130, we first
                # secure the conn and then mux the conn
                try:
                    # FIXME: This dummy `ID(b"")` for the remote peer is useless.
                    secured_conn = await self.upgrader.upgrade_security(
                        raw_conn, ID(b""), False
                    )
                except SecurityUpgradeFailure as error:
                    logger.debug("failed to upgrade security for peer at %s", maddr)
                    await raw_conn.close()
                    raise SwarmException(
                        f"failed to upgrade security for peer at {maddr}"
                    ) from error

In the unit tests, if a breakpoint in set on except SecurityUpgradeFailure, it never hits but it does hit in the mission mode.

In the unit tests, if we use except BaseException instead, the exception is caught but again the parent exception handlers don't catch in the unit tests.

Note that, in the unit tests, BaseException works but Exception doesn't. It's as if the bubbled up exception do not derive from Exception even though they are explicitly deriving from it. Is something modifying the exception objects in the unit tests?

Environment
  • Distro: Linux Mint 22.1
  • Kernel: 6.8.0-57-generic
  • Commit hash: e076a038bccd7506e28e08a1b317a99bd361792b
  • Date: Mon May 12 13:46:14 2025 -0600
Would you like to work on fixing this bug ?

Preferably not at this point. TLS, QUIC and AutoTLS have higher priorities.

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Reproduce the issue with tests/core/network/test_swarm.py and the provided failing test, then inspect Swarm.listen() and its conn_handler(), along with TCPListener.listen(). Compare unit-test behavior with mission mode and review the context of #593. Done means nursery task exceptions reliably fail the pytest test without breaking the existing swarm tests.

Written by the indexing model from the issue text.

Assessment

Tech stack
python
Domain
networking, testing-qa
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
35/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.