fix(zulip): resolve ReadTimeout by using call_endpoint with configurable timeout and retry

Root cause: zulip.Client.send_message() and upload_file() use a hardcoded
15s timeout (from do_api_query: timeout or 15.0). For file uploads and
slow networks, 15s is insufficient.

Fix:
- Replace send_message/upload_file with call_endpoint(url=..., timeout=...)
  which allows passing a custom timeout per request
- Add ZulipConfig.timeout field (default 60s, configurable via WebUI)
- Add _call_with_retry() with exponential backoff for transient errors
  (ReadTimeout, ConnectionError, OSError) - up to 3 retries
- Update all TestSend tests to use call_endpoint mock
- Add 3 new tests: timeout passthrough, ReadTimeout retry, retry exhaustion
- Total: 88 tests all passing

Author: WeDone

deeptutor/tutorbot/channels/zulip.py

@@ -48,6 +48,7 @@ class ZulipConfig(Base):
     api_key: str = Field(default="", repr=False)
     allow_from: list[str] = Field(default_factory=list)
     group_policy: Literal["mention", "open"] = "mention"
+    timeout: float = Field(default=60.0, description="HTTP request timeout in seconds")
 
 
 class ZulipChannel(BaseChannel):
@@ -456,6 +457,11 @@ async def _download_one(url: str, label: str) -> tuple[str, str]:
     async def send(self, msg: OutboundMessage) -> None:
         """Send a message through Zulip, including file attachments.
 
+        Uses ``call_endpoint`` with a configurable timeout instead of the
+        default 15s from ``send_message`` / ``upload_file``.  Transient
+        errors (``ReadTimeout``, ``ConnectionError``) are retried up to
+        3 times with exponential backoff.
+
         Synchronous ``zulip.Client`` calls are wrapped in
         ``run_in_executor`` to avoid blocking the async event loop.
         LaTeX formulas are sent as-is (Zulip renders KaTeX natively).
@@ -478,11 +484,18 @@ async def send(self, msg: OutboundMessage) -> None:
 
         content = msg.content or ""
         loop = asyncio.get_running_loop()
+        timeout = self.config.timeout
 
         for media_path in msg.media or []:
             try:
-                result = await loop.run_in_executor(
-                    None, functools.partial(self._client.upload_file, media_path)
+                result = await self._call_with_retry(
+                    loop,
+                    functools.partial(
+                        self._client.call_endpoint,
+                        url="user_uploads",
+                        files=[media_path],
+                        timeout=timeout,
+                    ),
                 )
                 if result.get("result") == "success":
                     uri = result.get("uri", "")
@@ -500,8 +513,14 @@ async def send(self, msg: OutboundMessage) -> None:
         for chunk in chunks:
             message_data["content"] = chunk
             try:
-                result = await loop.run_in_executor(
-                    None, functools.partial(self._client.send_message, message_data)
+                result = await self._call_with_retry(
+                    loop,
+                    functools.partial(
+                        self._client.call_endpoint,
+                        url="messages",
+                        request=message_data,
+                        timeout=timeout,
+                    ),
                 )
                 if result.get("result") != "success":
                     logger.error(
@@ -510,6 +529,39 @@ async def send(self, msg: OutboundMessage) -> None:
             except Exception as e:
                 logger.error("Zulip send error: {}", type(e).__name__)
 
+    async def _call_with_retry(
+        self,
+        loop: asyncio.AbstractEventLoop,
+        fn: functools.partial,
+        max_retries: int = 3,
+    ) -> Any:
+        """Call a sync zulip.Client method with retry on transient errors.
+
+        Retries on ``ReadTimeout`` and ``ConnectionError`` with
+        exponential backoff (1s, 2s, 4s).
+        """
+        from requests.exceptions import ConnectionError as RequestsConnectionError
+        from requests.exceptions import ReadTimeout
+
+        retryable = (ReadTimeout, RequestsConnectionError, OSError)
+        for attempt in range(max_retries):
+            try:
+                return await loop.run_in_executor(None, fn)
+            except retryable as e:
+                if attempt < max_retries - 1:
+                    wait = 2**attempt
+                    logger.warning(
+                        "Zulip API transient error ({}), retry {}/{} in {}s: {}",
+                        type(e).__name__,
+                        attempt + 1,
+                        max_retries,
+                        wait,
+                        str(e)[:120],
+                    )
+                    await asyncio.sleep(wait)
+                else:
+                    raise
+
     async def stop(self) -> None:
         """Stop the Zulip channel and clean up resources."""
         self._running = False

tests/test_zulip_channel.py

@@ -290,24 +290,27 @@ class TestSend:
 
     @pytest.mark.asyncio
     async def test_send_stream_message(self, channel: ZulipChannel):
+        channel._client.call_endpoint.return_value = {"result": "success"}
         msg = OutboundMessage(channel="zulip", chat_id="general:off-topic", content="Hello stream!")
         await channel.send(msg)
-        channel._client.send_message.assert_called_once()
-        sent_data = channel._client.send_message.call_args[0][0]
-        assert sent_data["type"] == "stream"
-        assert sent_data["to"] == "general"
-        assert sent_data["subject"] == "off-topic"
-        assert sent_data["content"] == "Hello stream!"
+        send_call = channel._client.call_endpoint.call_args_list[-1]
+        assert send_call.kwargs.get("url") == "messages" or send_call[1].get("url") == "messages"
+        request = send_call.kwargs.get("request") or send_call[1].get("request")
+        assert request["type"] == "stream"
+        assert request["to"] == "general"
+        assert request["subject"] == "off-topic"
+        assert request["content"] == "Hello stream!"
 
     @pytest.mark.asyncio
     async def test_send_private_message(self, channel: ZulipChannel):
+        channel._client.call_endpoint.return_value = {"result": "success"}
         msg = OutboundMessage(channel="zulip", chat_id="user@example.com", content="Hello private!")
         await channel.send(msg)
-        channel._client.send_message.assert_called_once()
-        sent_data = channel._client.send_message.call_args[0][0]
-        assert sent_data["type"] == "private"
-        assert sent_data["to"] == "user@example.com"
-        assert sent_data["content"] == "Hello private!"
+        send_call = channel._client.call_endpoint.call_args_list[-1]
+        request = send_call.kwargs.get("request") or send_call[1].get("request")
+        assert request["type"] == "private"
+        assert request["to"] == "user@example.com"
+        assert request["content"] == "Hello private!"
 
     @pytest.mark.asyncio
     async def test_send_no_client_logs_warning(self, bus: MessageBus):
@@ -319,74 +322,149 @@ async def test_send_no_client_logs_warning(self, bus: MessageBus):
 
     @pytest.mark.asyncio
     async def test_send_with_media_upload(self, channel: ZulipChannel):
-        channel._client.upload_file.return_value = {
-            "result": "success",
-            "uri": "/user_uploads/uploaded_image.png",
-        }
+        call_count = 0
+
+        def fake_call_endpoint(**kwargs):
+            nonlocal call_count
+            call_count += 1
+            if kwargs.get("url") == "user_uploads":
+                return {"result": "success", "uri": "/user_uploads/uploaded_image.png"}
+            return {"result": "success"}
+
+        channel._client.call_endpoint.side_effect = fake_call_endpoint
         msg = OutboundMessage(
             channel="zulip", chat_id="user@example.com", content="See this:", media=["/tmp/image.png"]
         )
         await channel.send(msg)
-        channel._client.upload_file.assert_called_once_with("/tmp/image.png")
-        sent_data = channel._client.send_message.call_args[0][0]
-        assert "[attachment](/user_uploads/uploaded_image.png)" in sent_data["content"]
+        assert call_count == 2
+        send_call = channel._client.call_endpoint.call_args_list[-1]
+        request = send_call.kwargs.get("request") or send_call[1].get("request")
+        assert "[attachment](/user_uploads/uploaded_image.png)" in request["content"]
 
     @pytest.mark.asyncio
     async def test_send_upload_failure_still_sends(self, channel: ZulipChannel):
-        channel._client.upload_file.return_value = {"result": "error", "msg": "too large"}
+        call_count = 0
+
+        def fake_call_endpoint(**kwargs):
+            nonlocal call_count
+            call_count += 1
+            if kwargs.get("url") == "user_uploads":
+                return {"result": "error", "msg": "too large"}
+            return {"result": "success"}
+
+        channel._client.call_endpoint.side_effect = fake_call_endpoint
         msg = OutboundMessage(
             channel="zulip", chat_id="user@example.com", content="Hello", media=["/tmp/big_file.zip"]
         )
         await channel.send(msg)
-        sent_data = channel._client.send_message.call_args[0][0]
-        assert sent_data["content"] == "Hello"
+        send_call = channel._client.call_endpoint.call_args_list[-1]
+        request = send_call.kwargs.get("request") or send_call[1].get("request")
+        assert request["content"] == "Hello"
 
     @pytest.mark.asyncio
     async def test_send_upload_exception_still_sends(self, channel: ZulipChannel):
-        channel._client.upload_file.side_effect = RuntimeError("network error")
+        call_count = 0
+
+        def fake_call_endpoint(**kwargs):
+            nonlocal call_count
+            call_count += 1
+            if kwargs.get("url") == "user_uploads":
+                raise RuntimeError("network error")
+            return {"result": "success"}
+
+        channel._client.call_endpoint.side_effect = fake_call_endpoint
         msg = OutboundMessage(
             channel="zulip", chat_id="user@example.com", content="Hello", media=["/tmp/file.zip"]
         )
         await channel.send(msg)
-        sent_data = channel._client.send_message.call_args[0][0]
-        assert sent_data["content"] == "Hello"
+        send_call = channel._client.call_endpoint.call_args_list[-1]
+        request = send_call.kwargs.get("request") or send_call[1].get("request")
+        assert request["content"] == "Hello"
 
     @pytest.mark.asyncio
     async def test_send_empty_content_with_media(self, channel: ZulipChannel):
-        channel._client.upload_file.return_value = {
-            "result": "success",
-            "uri": "/user_uploads/img.png",
-        }
+        call_count = 0
+
+        def fake_call_endpoint(**kwargs):
+            nonlocal call_count
+            call_count += 1
+            if kwargs.get("url") == "user_uploads":
+                return {"result": "success", "uri": "/user_uploads/img.png"}
+            return {"result": "success"}
+
+        channel._client.call_endpoint.side_effect = fake_call_endpoint
         msg = OutboundMessage(
             channel="zulip", chat_id="user@example.com", content="", media=["/tmp/img.png"]
         )
         await channel.send(msg)
-        sent_data = channel._client.send_message.call_args[0][0]
-        assert sent_data["content"] == "[attachment](/user_uploads/img.png)"
+        send_call = channel._client.call_endpoint.call_args_list[-1]
+        request = send_call.kwargs.get("request") or send_call[1].get("request")
+        assert request["content"] == "[attachment](/user_uploads/img.png)"
 
     @pytest.mark.asyncio
     async def test_send_sends_failure_logs_error(self, channel: ZulipChannel):
-        channel._client.send_message.return_value = {"result": "error", "msg": "not subscribed"}
+        channel._client.call_endpoint.return_value = {"result": "error", "msg": "not subscribed"}
         msg = OutboundMessage(channel="zulip", chat_id="stream:topic", content="test")
         await channel.send(msg)
 
     @pytest.mark.asyncio
     async def test_send_message_exception_handled(self, channel: ZulipChannel):
-        channel._client.send_message.side_effect = RuntimeError("connection lost")
+        channel._client.call_endpoint.side_effect = RuntimeError("connection lost")
         msg = OutboundMessage(channel="zulip", chat_id="stream:topic", content="test")
         await channel.send(msg)
 
     @pytest.mark.asyncio
     async def test_send_long_message_is_chunked(self, channel: ZulipChannel):
         from deeptutor.tutorbot.channels.zulip import MAX_MESSAGE_LEN
 
+        channel._client.call_endpoint.return_value = {"result": "success"}
         long_content = "A" * (MAX_MESSAGE_LEN + 500)
         msg = OutboundMessage(channel="zulip", chat_id="user@example.com", content=long_content)
         await channel.send(msg)
-        assert channel._client.send_message.call_count > 1
-        for call in channel._client.send_message.call_args_list:
-            sent_data = call[0][0]
-            assert len(sent_data["content"]) <= MAX_MESSAGE_LEN
+        message_calls = [
+            c for c in channel._client.call_endpoint.call_args_list
+            if (c.kwargs.get("url") or c[1].get("url")) == "messages"
+        ]
+        assert len(message_calls) > 1
+        for call in message_calls:
+            request = call.kwargs.get("request") or call[1].get("request")
+            assert len(request["content"]) <= MAX_MESSAGE_LEN
+
+    @pytest.mark.asyncio
+    async def test_send_uses_configured_timeout(self, channel: ZulipChannel):
+        channel._client.call_endpoint.return_value = {"result": "success"}
+        channel.config.timeout = 120.0
+        msg = OutboundMessage(channel="zulip", chat_id="user@example.com", content="hi")
+        await channel.send(msg)
+        send_call = channel._client.call_endpoint.call_args_list[-1]
+        timeout_val = send_call.kwargs.get("timeout") or send_call[1].get("timeout")
+        assert timeout_val == 120.0
+
+    @pytest.mark.asyncio
+    async def test_send_retries_on_read_timeout(self, channel: ZulipChannel):
+        from requests.exceptions import ReadTimeout
+
+        call_count = 0
+
+        def fake_call_endpoint(**kwargs):
+            nonlocal call_count
+            call_count += 1
+            if call_count == 1:
+                raise ReadTimeout("timed out")
+            return {"result": "success"}
+
+        channel._client.call_endpoint.side_effect = fake_call_endpoint
+        msg = OutboundMessage(channel="zulip", chat_id="user@example.com", content="hi")
+        await channel.send(msg)
+        assert call_count == 2
+
+    @pytest.mark.asyncio
+    async def test_send_retries_exhausted_raises(self, channel: ZulipChannel):
+        from requests.exceptions import ReadTimeout
+
+        channel._client.call_endpoint.side_effect = ReadTimeout("timed out")
+        msg = OutboundMessage(channel="zulip", chat_id="user@example.com", content="hi")
+        await channel.send(msg)
 
 
 class TestInit: