diff --git a/cron/scheduler.py b/cron/scheduler.py index 520b0f4630d..4c764bd13a4 100644 --- a/cron/scheduler.py +++ b/cron/scheduler.py @@ -1696,12 +1696,30 @@ def _deliver_result(job: dict, content: str, adapters=None, loop=None) -> Option # prevent "coroutine was never awaited" RuntimeWarning, then retry in a # fresh thread that has no running loop. coro.close() - with concurrent.futures.ThreadPoolExecutor(max_workers=1) as pool: - future = pool.submit(asyncio.run, _send_to_platform(platform, pconfig, chat_id, cleaned_delivery_content, thread_id=thread_id, media_files=media_files)) - result = future.result(timeout=30) + # The thread-pool fallback can itself raise (SMTP ConnectionError, + # future.result timeout, etc.). An exception raised inside this + # `except RuntimeError` block is NOT caught by the sibling + # `except Exception` below — it would escape _deliver_result() + # and crash the whole delivery loop, silently skipping every + # remaining target (#47163). Wrap the fallback in its own + # try/except so a per-target failure is logged and the loop + # continues to the next target. + try: + pool = concurrent.futures.ThreadPoolExecutor(max_workers=1) + try: + future = pool.submit(asyncio.run, _send_to_platform(platform, pconfig, chat_id, cleaned_delivery_content, thread_id=thread_id, media_files=media_files)) + result = future.result(timeout=30) + finally: + pool.shutdown(wait=False) + except Exception as e: + msg = f"delivery to {platform_name}:{chat_id} failed: {e}" + logger.error("Job '%s': %s", job["id"], msg, exc_info=True) + target_errors.extend([msg]) + delivery_errors.extend(target_errors) + continue except Exception as e: msg = f"delivery to {platform_name}:{chat_id} failed: {e}" - logger.error("Job '%s': %s", job["id"], msg) + logger.error("Job '%s': %s", job["id"], msg, exc_info=True) target_errors.extend([msg]) delivery_errors.extend(target_errors) continue diff --git a/scripts/release.py b/scripts/release.py index d58bdd2d383..e6432419240 100755 --- a/scripts/release.py +++ b/scripts/release.py @@ -45,6 +45,7 @@ ACP_REGISTRY_MANIFEST = REPO_ROOT / "acp_registry" / "agent.json" # Auto-extracted from noreply emails + manual overrides AUTHOR_MAP = { + "swissly@users.noreply.github.com": "swissly", # PR #47167 salvage (wrap cron delivery thread-pool fallback in its own try/except so a per-target failure can't escape the except-RuntimeError block and crash the multi-target delivery loop; #47163) "30854794+YLChen-007@users.noreply.github.com": "YLChen-007", # PR #27289 salvage (case-insensitive streaming reasoning-tag filter in cli.py _stream_delta + gateway stream_consumer so mixed-case variants like / are suppressed, not just the hardcoded case literals) "27672904+kangsoo-bit@users.noreply.github.com": "kangsoo-bit", # PR #47508 salvage (keep Telegram gateway alive on transient bootstrap network errors: best-effort deleteWebhook + resilient start_polling degrade to background recovery instead of failing startup) "259353979+testingbuddies24@users.noreply.github.com": "testingbuddies24", # PR #43192 salvage (strip orphan think-tag close tags in progressive gateway stream so a bare whose open was dropped upstream can't leak to the user) diff --git a/tests/cron/test_scheduler.py b/tests/cron/test_scheduler.py index 01bae7ed2d4..54445e7054c 100644 --- a/tests/cron/test_scheduler.py +++ b/tests/cron/test_scheduler.py @@ -4391,3 +4391,79 @@ class TestCronContinuableSurfaceInChannel: store.get_or_create_session.assert_not_called() mirror_mock.assert_not_called() + +class TestMultiTargetDeliveryContinuesOnFailure: + """When delivery to one target fails inside the standalone thread-pool + fallback, the loop must continue to the remaining targets (#47163). + + The fallback runs inside the `except RuntimeError` block of + `_deliver_result`. Before the fix, an exception raised there (SMTP + ConnectionError, future.result timeout) escaped the function entirely — + it is NOT caught by the sibling `except Exception` — crashing the loop + and silently dropping every subsequent target. + """ + + def _email_cfg(self): + from gateway.config import Platform + + pconfig = MagicMock() + pconfig.enabled = True + mock_cfg = MagicMock() + mock_cfg.platforms = {Platform.EMAIL: pconfig} + return mock_cfg + + def test_first_target_failure_does_not_crash_loop(self): + """First email target fails in the fallback; the second is still attempted.""" + job = { + "id": "multi-email-job", + "deliver": "email:a@example.com,email:b@example.com", + } + + with patch("gateway.config.load_gateway_config", return_value=self._email_cfg()), \ + patch("cron.scheduler.load_config", return_value={"cron": {"wrap_response": False}}), \ + patch("asyncio.run", side_effect=RuntimeError("no running loop")), \ + patch("concurrent.futures.ThreadPoolExecutor") as mock_pool_cls: + mock_pool = MagicMock() + mock_pool_cls.return_value = mock_pool + + fail_future = MagicMock() + fail_future.result.side_effect = ConnectionError("SMTP connection refused") + ok_future = MagicMock() + ok_future.result.return_value = {"success": True} + mock_pool.submit.side_effect = [fail_future, ok_future] + + result = _deliver_result(job, "Report content") + + # Both targets attempted — the loop did not crash after the first failure. + assert mock_pool.submit.call_count == 2, ( + f"expected 2 delivery attempts, got {mock_pool.submit.call_count}" + ) + # First target's failure is surfaced in the returned error string. + assert result is not None + assert "a@example.com" in result + assert "SMTP connection refused" in result + + def test_all_targets_fail_returns_combined_errors(self): + """When every target fails, the result reports all of them.""" + job = { + "id": "all-fail-job", + "deliver": "email:a@example.com,email:b@example.com", + } + + with patch("gateway.config.load_gateway_config", return_value=self._email_cfg()), \ + patch("cron.scheduler.load_config", return_value={"cron": {"wrap_response": False}}), \ + patch("asyncio.run", side_effect=RuntimeError("no running loop")), \ + patch("concurrent.futures.ThreadPoolExecutor") as mock_pool_cls: + mock_pool = MagicMock() + mock_pool_cls.return_value = mock_pool + + fail_future = MagicMock() + fail_future.result.side_effect = ConnectionError("connection refused") + mock_pool.submit.return_value = fail_future + + result = _deliver_result(job, "Report content") + + assert result is not None + assert "a@example.com" in result + assert "b@example.com" in result + assert mock_pool.submit.call_count == 2