diff --git a/repeater/handler_helpers/room_server.py b/repeater/handler_helpers/room_server.py index 1d6b988..8139a33 100644 --- a/repeater/handler_helpers/room_server.py +++ b/repeater/handler_helpers/room_server.py @@ -136,7 +136,10 @@ class RoomServer: ) # Send via packet injector - await packet_injector(packet, wait_for_ack=False) + sent = await packet_injector(packet, wait_for_ack=False) + if not sent: + logger.error(f"Room '{room_name}': Failed to send advert") + return False logger.info( f"Room '{room_name}': Sent flood advert '{node_name}' at ({latitude:.6f}, {longitude:.6f})" diff --git a/repeater/main.py b/repeater/main.py index 9388989..f573ef8 100644 --- a/repeater/main.py +++ b/repeater/main.py @@ -1271,10 +1271,17 @@ class RepeaterDaemon: scope_label="advert", ) - # Send via dispatcher - await self.dispatcher.send_packet(packet, wait_for_ack=False) + injector = getattr(getattr(self, "router", None), "inject_packet", None) + if callable(injector): + sent = await injector(packet, wait_for_ack=False) + else: + sent = await self.dispatcher.send_packet(packet, wait_for_ack=False) - if self.repeater_handler: + if not sent: + logger.error("Failed to send advert: packet transmission was rejected") + return False + + if not callable(injector) and self.repeater_handler: self.repeater_handler.mark_seen(packet) pkt_hash = packet.calculate_packet_hash().hex()[:16] self.dispatcher.packet_filter.track_packet(pkt_hash) diff --git a/repeater/web/api_endpoints.py b/repeater/web/api_endpoints.py index 640b649..5663c9d 100644 --- a/repeater/web/api_endpoints.py +++ b/repeater/web/api_endpoints.py @@ -5977,11 +5977,18 @@ class APIEndpoints: scope_label="room server advert", ) - # Send via dispatcher - await self.daemon_instance.dispatcher.send_packet(packet, wait_for_ack=False) + injector = getattr(getattr(self.daemon_instance, "router", None), "inject_packet", None) + if callable(injector): + sent = await injector(packet, wait_for_ack=False) + else: + sent = await self.daemon_instance.dispatcher.send_packet(packet, wait_for_ack=False) - # Mark as seen to prevent re-forwarding - if self.daemon_instance.repeater_handler: + if not sent: + logger.error("Failed to send room server advert: packet transmission was rejected") + return False + + # Mark as seen only when bypassing the engine and sending directly. + if not callable(injector) and self.daemon_instance.repeater_handler: self.daemon_instance.repeater_handler.mark_seen(packet) logger.debug(f"Marked room server advert '{node_name}' as seen in duplicate cache") diff --git a/tests/test_api_endpoints_core_coverage.py b/tests/test_api_endpoints_core_coverage.py index b2a3b6f..e884fca 100644 --- a/tests/test_api_endpoints_core_coverage.py +++ b/tests/test_api_endpoints_core_coverage.py @@ -1546,6 +1546,39 @@ def test_room_server_advert_applies_default_region_scope(): dispatcher.send_packet.assert_awaited_once_with(packet, wait_for_ack=False) +def test_room_server_advert_returns_false_when_send_rejected(): + from openhop_core.protocol.constants import ROUTE_TYPE_FLOOD + + api = _make_api({"mesh": {"default_region": "alpha"}, "repeater": {}}) + dispatcher = SimpleNamespace(send_packet=AsyncMock(return_value=False)) + api.daemon_instance = SimpleNamespace(dispatcher=dispatcher, repeater_handler=None) + + packet = SimpleNamespace( + header=ROUTE_TYPE_FLOOD, + transport_codes=[0, 0], + get_payload_type=lambda: 3, + get_payload=lambda: b"room_server_advert_payload", + ) + + with ( + patch("openhop_core.protocol.PacketBuilder.create_advert", return_value=packet), + patch("openhop_core.protocol.transport_keys.get_auto_key_for", return_value=b"\x01" * 16), + patch("openhop_core.protocol.transport_keys.calc_transport_code", return_value=0xCAFE), + ): + result = asyncio.run( + api._send_room_server_advert_async( + identity=SimpleNamespace(), + node_name="RoomAlpha", + latitude=1.0, + longitude=2.0, + disable_fwd=False, + ) + ) + + assert result is False + dispatcher.send_packet.assert_awaited_once_with(packet, wait_for_ack=False) + + def test_set_mode_and_set_duty_cycle_paths(cherrypy_ctx): request, _ = cherrypy_ctx api = _make_api({"repeater": {}, "duty_cycle": {}}) diff --git a/tests/test_handler_helpers_room_server.py b/tests/test_handler_helpers_room_server.py index 8125034..683daca 100644 --- a/tests/test_handler_helpers_room_server.py +++ b/tests/test_handler_helpers_room_server.py @@ -109,6 +109,40 @@ async def test_room_server_add_post_returns_false_on_db_insert_failure(): assert ok is False +@pytest.mark.asyncio +async def test_room_server_send_advert_callback_returns_false_on_inject_failure(): + config = { + "identities": { + "room_servers": [ + { + "name": "room-alpha", + "settings": {"node_name": "Room Alpha", "latitude": 1.0, "longitude": 2.0}, + } + ] + } + } + + injector = AsyncMock(return_value=False) + rs = RoomServer( + room_hash=0x34, + room_name="room-alpha", + local_identity=_FakeIdentity(b"R" * 32), + sqlite_handler=_FakeDB(), + packet_injector=injector, + acl=_FakeACL(), + config_path="/tmp/room.yaml", + config=config, + config_manager=SimpleNamespace(), + ) + + packet = SimpleNamespace() + with patch("openhop_core.protocol.PacketBuilder.create_advert", return_value=packet): + ok = await rs.cli.send_advert_callback() + + assert ok is False + injector.assert_awaited_once() + + def test_room_server_init_caps_max_posts_to_hard_limit(): rs = _make_room_server(max_posts=MAX_UNSYNCED_POSTS + 50) assert rs.max_posts == MAX_UNSYNCED_POSTS diff --git a/tests/test_main_py_coverage.py b/tests/test_main_py_coverage.py index faf3606..cc661c5 100644 --- a/tests/test_main_py_coverage.py +++ b/tests/test_main_py_coverage.py @@ -264,6 +264,30 @@ async def test_send_advert_branches_and_success_path(): daemon.dispatcher.packet_filter.track_packet.assert_called_once() +@pytest.mark.asyncio +async def test_send_advert_returns_false_when_dispatch_rejects(): + daemon = RepeaterDaemon(_base_config(), radio=object()) + daemon.dispatcher = SimpleNamespace( + send_packet=AsyncMock(return_value=False), + packet_filter=SimpleNamespace(track_packet=MagicMock()), + ) + daemon.local_identity = _FakeIdentity(b"\x21" + b"x" * 31) + daemon.config["repeater"]["mode"] = "forward" + daemon.repeater_handler = SimpleNamespace(mark_seen=MagicMock()) + daemon.gps_service = SimpleNamespace( + get_repeater_location=lambda: {"latitude": 9.1, "longitude": 8.2, "source": "gps"} + ) + + packet = SimpleNamespace(calculate_packet_hash=lambda: b"\xab" * 16) + with patch("openhop_core.protocol.PacketBuilder.create_advert", return_value=packet): + ok = await daemon.send_advert() + + assert ok is False + daemon.dispatcher.send_packet.assert_awaited_once_with(packet, wait_for_ack=False) + daemon.repeater_handler.mark_seen.assert_not_called() + daemon.dispatcher.packet_filter.track_packet.assert_not_called() + + @pytest.mark.asyncio async def test_send_advert_applies_transport_scope_when_default_region_set(): from openhop_core.protocol.constants import ROUTE_TYPE_FLOOD, ROUTE_TYPE_TRANSPORT_FLOOD