Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 23 additions & 2 deletions gateway/platforms/telegram.py
Original file line number Diff line number Diff line change
Expand Up @@ -1942,7 +1942,20 @@ async def send_document(
)
return SendResult(success=True, message_id=str(msg.message_id))
except Exception as e:
print(f"[{self.name}] Failed to send document: {e}")
# Route to the standard logger so operators see the
# diagnostic (exception type + stack) in gateway logs.
# The previous ``print`` went to stdout and was invisible
# in systemd/Docker-captured log streams — operators
# reporting "Hermes says success, Telegram received
# nothing" had no way to see the underlying error. See
# #13356. Matches the established pattern in
# ``send_voice`` / ``send_image_file`` below/above.
logger.error(
"[%s] Failed to send Telegram document, falling back to base adapter: %s",
self.name,
e,
exc_info=True,
)
return await super().send_document(chat_id, file_path, caption, file_name, reply_to)

async def send_video(
Expand Down Expand Up @@ -1973,7 +1986,15 @@ async def send_video(
)
return SendResult(success=True, message_id=str(msg.message_id))
except Exception as e:
print(f"[{self.name}] Failed to send video: {e}")
# Route to the standard logger so operators see the
# diagnostic (exception type + stack) in gateway logs —
# same rationale as ``send_document`` above. See #13356.
logger.error(
"[%s] Failed to send Telegram video, falling back to base adapter: %s",
self.name,
e,
exc_info=True,
)
return await super().send_video(chat_id, video_path, caption, reply_to)

async def send_image(
Expand Down
167 changes: 167 additions & 0 deletions tests/gateway/test_telegram_documents.py
Original file line number Diff line number Diff line change
Expand Up @@ -606,6 +606,110 @@ async def test_send_document_api_error_falls_back(self, connected_adapter, tmp_p
assert result.success is True
assert result.message_id == "fallback"

# --- Regression coverage for #13356 -----------------------------------
# Before the fix, ``send_document`` wrote the exception to stdout via
# ``print(...)`` instead of ``logger.error``. On systemd/Docker gateway
# deployments the diagnostic was invisible, so operators seeing
# "Hermes says success, Telegram received nothing" had no way to
# see the underlying error. These tests pin that every failure
# routes through the standard logger with a full stack trace (matches
# the pattern established by ``send_voice`` and ``send_image_file``).

Comment on lines +609 to +617

Copilot AI Apr 21, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The PR description says this file adds “4 new cases (2 per method)”, but this diff adds 5 new tests (3 under TestSendDocument plus 2 under TestSendVideo). Please update the PR description to match the actual test changes (or drop the extra test if the intent really was 4).

Copilot uses AI. Check for mistakes.
@pytest.mark.asyncio
async def test_send_document_api_error_is_logged_with_exc_info(
self, connected_adapter, tmp_path, caplog
):
"""Native send_document failures must be logged via logger.error
with ``exc_info=True`` so operators can diagnose from gateway logs
(not stdout via ``print``). See #13356."""
import logging
test_file = tmp_path / "file.pdf"
test_file.write_bytes(b"data")

connected_adapter._bot.send_document = AsyncMock(
side_effect=RuntimeError("Telegram upload rejected")
)
connected_adapter.send = AsyncMock(
return_value=SendResult(success=True, message_id="fallback")
)

with caplog.at_level(logging.ERROR, logger="gateway.platforms.telegram"):
await connected_adapter.send_document(
chat_id="12345", file_path=str(test_file),
)

matching = [
r for r in caplog.records
if r.name == "gateway.platforms.telegram" and r.levelno == logging.ERROR
]
assert matching, (
"Expected an ERROR-level log from gateway.platforms.telegram on "
"send_document failure; caplog records: "
f"{[(r.name, r.levelname, r.getMessage()) for r in caplog.records]}"
)
log = matching[0]
# Full diagnostic must be attached (exc_info=True path).
assert log.exc_info is not None, (
"logger.error must be called with exc_info=True so the stack "
"trace lands in the gateway log — this is the whole point of "
"the #13356 fix."
)
# The underlying exception text must surface in the log message.
assert "Telegram upload rejected" in log.getMessage()
# And the log line must name the adapter so operators see which
# platform raised (matches ``send_voice`` / ``send_image_file``).
assert connected_adapter.name in log.getMessage()

@pytest.mark.asyncio
async def test_send_document_logs_do_not_hit_stdout(
self, connected_adapter, tmp_path, capsys
):
"""Structural pin: the old ``print(...)`` path is gone. Any
stdout output during a send_document failure would be a
regression (invisible to systemd / Docker log capture)."""
test_file = tmp_path / "file.pdf"
test_file.write_bytes(b"data")

connected_adapter._bot.send_document = AsyncMock(
side_effect=RuntimeError("boom")
)
connected_adapter.send = AsyncMock(
return_value=SendResult(success=True, message_id="fallback")
)

await connected_adapter.send_document(
chat_id="12345", file_path=str(test_file),
)
captured = capsys.readouterr()
# Nothing on stdout — all diagnostic must flow via ``logger``.
assert f"[{connected_adapter.name}] Failed to send document" not in captured.out

@pytest.mark.asyncio
async def test_send_document_still_invokes_base_fallback_on_error(
self, connected_adapter, tmp_path
):
"""Preserved-behaviour canary: even after the logging fix, the
native-failure path still falls through to the base adapter's
text fallback — matches sibling ``send_voice`` / ``send_image_file``
semantics. This PR only changes observability, not routing."""
test_file = tmp_path / "file.pdf"
test_file.write_bytes(b"data")

connected_adapter._bot.send_document = AsyncMock(
side_effect=RuntimeError("boom")
)
connected_adapter.send = AsyncMock(
return_value=SendResult(success=True, message_id="fallback-msg-id")
)

result = await connected_adapter.send_document(
chat_id="12345", file_path=str(test_file),
)
# Base fallback ran (routed through self.send)
connected_adapter.send.assert_awaited_once()
assert result.success is True
assert result.message_id == "fallback-msg-id"

@pytest.mark.asyncio
async def test_send_document_reply_to(self, connected_adapter, tmp_path):
"""reply_to parameter is forwarded as reply_to_message_id."""
Expand Down Expand Up @@ -771,3 +875,66 @@ async def test_send_video_thread_id(self, connected_adapter, tmp_path):

call_kwargs = connected_adapter._bot.send_video.call_args[1]
assert call_kwargs["message_thread_id"] == 789

# --- Regression coverage for #13356 (send_video path) ----------------
# Identical observability fix as ``send_document`` — the ``print(...)``
# path was the twin of the one flagged in the issue.

@pytest.mark.asyncio
async def test_send_video_api_error_is_logged_with_exc_info(
self, connected_adapter, tmp_path, caplog
):
"""Native send_video failures must route through logger.error with
``exc_info=True`` so gateway log capture sees them."""
import logging
test_file = tmp_path / "clip.mp4"
test_file.write_bytes(b"\x00\x00\x00\x1c" + b"ftyp" + b"\x00" * 100)

connected_adapter._bot.send_video = AsyncMock(
side_effect=RuntimeError("Telegram video rejected")
)
connected_adapter.send = AsyncMock(
return_value=SendResult(success=True, message_id="fallback")
)

with caplog.at_level(logging.ERROR, logger="gateway.platforms.telegram"):
await connected_adapter.send_video(
chat_id="12345", video_path=str(test_file),
)

matching = [
r for r in caplog.records
if r.name == "gateway.platforms.telegram" and r.levelno == logging.ERROR
]
assert matching, (
"Expected an ERROR-level log on send_video failure; caplog records: "
f"{[(r.name, r.levelname, r.getMessage()) for r in caplog.records]}"
)
log = matching[0]
assert log.exc_info is not None, (
"logger.error must be called with exc_info=True — the stack "
"trace is the whole diagnostic."
)
assert "Telegram video rejected" in log.getMessage()
assert connected_adapter.name in log.getMessage()

@pytest.mark.asyncio
async def test_send_video_logs_do_not_hit_stdout(
self, connected_adapter, tmp_path, capsys
):
"""Structural pin: no ``print(...)`` leak on send_video errors."""
test_file = tmp_path / "clip.mp4"
test_file.write_bytes(b"\x00\x00\x00\x1c" + b"ftyp" + b"\x00" * 100)

connected_adapter._bot.send_video = AsyncMock(
side_effect=RuntimeError("boom")
)
connected_adapter.send = AsyncMock(
return_value=SendResult(success=True, message_id="fallback")
)

await connected_adapter.send_video(
chat_id="12345", video_path=str(test_file),
)
captured = capsys.readouterr()
assert f"[{connected_adapter.name}] Failed to send video" not in captured.out
Loading