From b57488734d8d063fd952910cc86f6d1b658bc193 Mon Sep 17 00:00:00 2001 From: yetuge <182123929+yetuge@users.noreply.github.com> Date: Thu, 22 Oct 2026 19:26:38 +0800 Subject: [PATCH] fix(channels): recover from undecodable runtime config and WeChat state (#7273) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `store.py` already treats a state file whose bytes are not UTF-8 as corruption: `_read_legacy_file` catches `UnicodeDecodeError` and the three call paths start fresh or skip the import (#7233, AGENTS.md: "Store JSON decode errors follow corrupt-file recovery"). Four readers in the same package read the same kind of file with an explicit `encoding="utf-8"` but catch only `json.JSONDecodeError`/`OSError`; `UnicodeDecodeError` is a `ValueError`, so it escapes and takes the caller down instead of following that policy: * `ChannelRuntimeConfigStore._load` — `ChannelService.start()` and the channel-connections router construct the store, so one undecodable `runtime-config.json` stops every IM channel from starting, including the non-fallback path where a peer's copy should be rewritten from this process's snapshot. * `_read_legacy_file` — the one-time `runtime-config.json` import aborts instead of keeping the file and reporting it as corrupt. * `WechatChannel._load_state` / `_load_auth_state` — `start()` loads both through `asyncio.to_thread`, so an undecodable cursor or auth file raises out of the channel's own start instead of warning and continuing unauthenticated. Each reader now names `UnicodeDecodeError` next to the existing handlers, so the outcome matches the sibling store: a warning, the in-memory fallback, and the file left in place. --- backend/app/channels/runtime_config_store.py | 5 +- backend/app/channels/wechat.py | 4 +- .../test_channel_runtime_config_db_store.py | 12 +++ .../tests/test_channel_undecodable_state.py | 78 +++++++++++++++++++ 4 files changed, 96 insertions(+), 3 deletions(-) create mode 100644 backend/tests/test_channel_undecodable_state.py diff --git a/backend/app/channels/runtime_config_store.py b/backend/app/channels/runtime_config_store.py index c12e24ea58..a5c38089fc 100644 --- a/backend/app/channels/runtime_config_store.py +++ b/backend/app/channels/runtime_config_store.py @@ -130,7 +130,7 @@ class ChannelRuntimeConfigStore: if self._path.exists(): try: raw = json.loads(self._path.read_text(encoding="utf-8")) - except (json.JSONDecodeError, OSError): + except (json.JSONDecodeError, OSError, UnicodeDecodeError): if fallback is not None: logger.warning("Corrupt channel runtime config store at %s, rewriting it from this process's copy", self._path) return {name: dict(config) for name, config in fallback.items()} @@ -369,6 +369,9 @@ def _read_legacy_file(path: Path) -> dict[str, Any] | None: raw = path.read_text(encoding="utf-8") except FileNotFoundError: return None + except UnicodeDecodeError: + logger.warning("Corrupt channel runtime config %s; skipping the import (file kept)", path) + return None except OSError: logger.warning("Cannot read channel runtime config %s; skipping the import (file kept)", path, exc_info=True) return None diff --git a/backend/app/channels/wechat.py b/backend/app/channels/wechat.py index e909ab0896..d604265a11 100644 --- a/backend/app/channels/wechat.py +++ b/backend/app/channels/wechat.py @@ -1695,7 +1695,7 @@ class WechatChannel(Channel): return try: data = json.loads(self._cursor_path.read_text(encoding="utf-8")) - except (OSError, json.JSONDecodeError): + except (OSError, json.JSONDecodeError, UnicodeDecodeError): logger.warning("[WeChat] failed to read cursor state from %s", self._cursor_path) return cursor = data.get("get_updates_buf") @@ -1719,7 +1719,7 @@ class WechatChannel(Channel): return try: data = json.loads(self._auth_path.read_text(encoding="utf-8")) - except (OSError, json.JSONDecodeError): + except (OSError, json.JSONDecodeError, UnicodeDecodeError): logger.warning("[WeChat] failed to read auth state from %s", self._auth_path) return if not isinstance(data, dict): diff --git a/backend/tests/test_channel_runtime_config_db_store.py b/backend/tests/test_channel_runtime_config_db_store.py index 074582839d..a7f8dc37f1 100644 --- a/backend/tests/test_channel_runtime_config_db_store.py +++ b/backend/tests/test_channel_runtime_config_db_store.py @@ -234,6 +234,18 @@ def test_a_corrupt_legacy_file_is_kept(session_factory, tmp_path): assert legacy.read_text(encoding="utf-8") == "{not json" +def test_an_undecodable_legacy_file_is_kept(session_factory, tmp_path, caplog): + """A file whose bytes are not UTF-8 is corrupt, not a crash: the import skips and keeps it.""" + legacy = tmp_path / "runtime-config.json" + legacy.write_bytes(b'{"slack": {"enabled": "\xff"}}') + + with caplog.at_level(logging.WARNING, logger="app.channels.runtime_config_store"): + assert _store(session_factory, cipher=_cipher(), legacy_path=legacy).import_legacy_json() == 0 + + assert legacy.read_bytes() == b'{"slack": {"enabled": "\xff"}}' + assert "Corrupt channel runtime config" in caplog.text + + def test_without_a_cipher_the_import_keeps_the_file(session_factory, tmp_path): legacy = tmp_path / "runtime-config.json" _write_legacy(legacy, {"slack": SLACK}) diff --git a/backend/tests/test_channel_undecodable_state.py b/backend/tests/test_channel_undecodable_state.py new file mode 100644 index 0000000000..532f354422 --- /dev/null +++ b/backend/tests/test_channel_undecodable_state.py @@ -0,0 +1,78 @@ +"""Undecodable channel state follows the corrupt-state recovery policy, not a crash. + +``app/channels/store.py`` recovers from a state file whose bytes are not UTF-8 +(``_read_legacy_file`` catches ``UnicodeDecodeError`` and starts fresh), and its +AGENTS.md states that rule for channel store JSON. The other state readers in the +same package read the same kind of file with an explicit ``encoding="utf-8"`` but +only catch ``json.JSONDecodeError`` and ``OSError``; ``UnicodeDecodeError`` is a +``ValueError``, not an ``OSError``, so it escapes and the channel cannot start. + +Each test writes undecodable bytes into the file that reader owns and asserts the +documented outcome: a warning, the in-memory fallback, and the file left in place. +""" + +from __future__ import annotations + +import json +import logging +from pathlib import Path + +import pytest + +from app.channels import runtime_config_store +from app.channels.message_bus import MessageBus +from app.channels.wechat import WechatChannel + +UNDECODABLE = b'{"enabled": "\xff"}' + + +def test_runtime_config_store_starts_fresh_on_undecodable_state(tmp_path: Path, caplog: pytest.LogCaptureFixture) -> None: + path = tmp_path / "channels" / "runtime-config.json" + path.parent.mkdir(parents=True) + path.write_bytes(UNDECODABLE) + + with caplog.at_level(logging.WARNING, logger="app.channels.runtime_config_store"): + store = runtime_config_store.ChannelRuntimeConfigStore(path) + + assert store.get_provider_config("buzz") is None + assert "Corrupt channel runtime config store" in caplog.text + assert path.read_bytes() == UNDECODABLE + + +def test_runtime_config_store_reports_the_peer_copy_on_undecodable_state(tmp_path: Path, caplog: pytest.LogCaptureFixture) -> None: + """The reload path under ``_mutation`` must fall back to this process's copy.""" + path = tmp_path / "channels" / "runtime-config.json" + store = runtime_config_store.ChannelRuntimeConfigStore(path) + store.set_provider_config("buzz", {"enabled": True}) + path.write_bytes(UNDECODABLE) + + with caplog.at_level(logging.WARNING, logger="app.channels.runtime_config_store"): + store.set_provider_config("slack", {"enabled": True}) + + assert "rewriting it from this process's copy" in caplog.text + assert store.get_provider_config("buzz") == {"enabled": True} + + +@pytest.mark.asyncio +async def test_wechat_starts_on_undecodable_state(tmp_path: Path, caplog: pytest.LogCaptureFixture) -> None: + (tmp_path / "wechat-getupdates.json").write_bytes(UNDECODABLE) + channel = WechatChannel(MessageBus(), config={"bot_token": "test-token", "state_dir": str(tmp_path)}) + + with caplog.at_level(logging.WARNING, logger="app.channels.wechat"): + channel._load_state() + + assert channel._get_updates_buf == "" + assert "failed to read cursor state" in caplog.text + + +@pytest.mark.asyncio +async def test_wechat_starts_on_undecodable_auth_state(tmp_path: Path, caplog: pytest.LogCaptureFixture) -> None: + (tmp_path / "wechat-auth.json").write_bytes(UNDECODABLE) + channel = WechatChannel(MessageBus(), config={"bot_token": "", "state_dir": str(tmp_path), "qrcode_login_enabled": True}) + + with caplog.at_level(logging.WARNING, logger="app.channels.wechat"): + channel._load_auth_state() + + assert channel._bot_token == "" + assert "failed to read auth state" in caplog.text + assert json.dumps({}) == "{}"