fix(avatar): fail closed on BOXIM connection errors
This commit is contained in:
@@ -2,7 +2,7 @@ from fastapi import APIRouter, Body, Depends, Header, HTTPException
|
||||
from sqlalchemy.orm import Session
|
||||
|
||||
from database import get_db
|
||||
from models import Authorization
|
||||
from models import Authorization, TakeoverCursor, TakeoverReplyTask
|
||||
from responses import fail, ok
|
||||
from routers.avatars import _require_owned_avatar
|
||||
|
||||
@@ -151,10 +151,32 @@ def update_permission_settings(
|
||||
except ValueError as exc:
|
||||
return fail(str(exc), 400)
|
||||
|
||||
previous_permissions = _stored_avatar_permissions(avatar)
|
||||
avatar.config = {
|
||||
**(avatar.config or {}),
|
||||
AVATAR_PERMISSION_KEY: permissions,
|
||||
}
|
||||
cursor = db.query(TakeoverCursor).filter(TakeoverCursor.avatar_id == avatar.id).first()
|
||||
if cursor and "takeover" in permissions and "takeover" not in previous_permissions:
|
||||
cursor.initialized = False
|
||||
cursor.last_message_id = "0"
|
||||
cursor.last_error = ""
|
||||
elif cursor and "takeover" not in permissions:
|
||||
cursor.last_error = ""
|
||||
|
||||
if "takeover" not in permissions:
|
||||
tasks = (
|
||||
db.query(TakeoverReplyTask)
|
||||
.filter(
|
||||
TakeoverReplyTask.avatar_id == avatar.id,
|
||||
TakeoverReplyTask.status.in_(("pending", "generating", "ready", "sending")),
|
||||
)
|
||||
.all()
|
||||
)
|
||||
for task in tasks:
|
||||
task.status = "cancelled"
|
||||
task.cancel_reason = "takeover_disabled"
|
||||
task.locked_at = None
|
||||
db.commit()
|
||||
db.refresh(avatar)
|
||||
return ok(_permission_settings_payload(avatar), "授权设置已保存")
|
||||
|
||||
@@ -34,12 +34,12 @@ def get_takeover_status(
|
||||
.count()
|
||||
)
|
||||
|
||||
if not enabled:
|
||||
if cursor and cursor.last_error:
|
||||
status, message = "error", cursor.last_error
|
||||
elif not enabled:
|
||||
status, message = "disabled", "主动接管未开启"
|
||||
elif not user or not user.huihui_token:
|
||||
status, message = "needs_login", "请重新登录会会生产账号以连接 BOXIM"
|
||||
elif cursor and cursor.last_error:
|
||||
status, message = "error", cursor.last_error
|
||||
elif (
|
||||
cursor
|
||||
and cursor.initialized
|
||||
|
||||
@@ -97,7 +97,18 @@ class BoxIMClient:
|
||||
payload.get("message") or "BOXIM 授权失败",
|
||||
code=code or response.status_code,
|
||||
auth_error=response.status_code in (400, 401, 403)
|
||||
or code in (400, 401, 40100, 403, "400", "401", "40100", "403"),
|
||||
or code in (
|
||||
400,
|
||||
401,
|
||||
40100,
|
||||
40101,
|
||||
403,
|
||||
"400",
|
||||
"401",
|
||||
"40100",
|
||||
"40101",
|
||||
"403",
|
||||
),
|
||||
)
|
||||
if not data.get("accessToken"):
|
||||
raise BoxIMError("会会未返回 BOXIM 访问凭证", auth_error=True)
|
||||
@@ -128,7 +139,7 @@ class BoxIMClient:
|
||||
payload.get("message") or "BOXIM 请求失败",
|
||||
code=code or response.status_code,
|
||||
auth_error=response.status_code in (400, 401, 403)
|
||||
or code in (400, 401, 403, "400", "401", "403"),
|
||||
or code in (400, 401, 40100, 40101, 403, "400", "401", "40100", "40101", "403"),
|
||||
)
|
||||
return payload.get("data")
|
||||
|
||||
|
||||
@@ -191,6 +191,37 @@ class TakeoverService:
|
||||
def _forget_boxim_session(self, user_id: str):
|
||||
self._sessions.pop(user_id, None)
|
||||
|
||||
def _disable_after_connection_failure(
|
||||
self,
|
||||
db: Session,
|
||||
avatar: Avatar,
|
||||
cursor: TakeoverCursor,
|
||||
message: str,
|
||||
):
|
||||
permissions = (avatar.config or {}).get("authorizationPermissions", [])
|
||||
avatar.config = {
|
||||
**(avatar.config or {}),
|
||||
"authorizationPermissions": [
|
||||
permission
|
||||
for permission in permissions
|
||||
if permission != TAKEOVER_PERMISSION
|
||||
],
|
||||
}
|
||||
cursor.last_error = message
|
||||
cursor.last_polled_at = self.now()
|
||||
tasks = (
|
||||
db.query(TakeoverReplyTask)
|
||||
.filter(
|
||||
TakeoverReplyTask.avatar_id == avatar.id,
|
||||
TakeoverReplyTask.status.in_(ACTIVE_TASK_STATUSES),
|
||||
)
|
||||
.all()
|
||||
)
|
||||
for task in tasks:
|
||||
task.status = "cancelled"
|
||||
task.cancel_reason = "connection_failed"
|
||||
task.locked_at = None
|
||||
|
||||
async def _sync_avatar(self, avatar_id: str) -> bool:
|
||||
db = self.session_factory()
|
||||
try:
|
||||
@@ -204,8 +235,12 @@ class TakeoverService:
|
||||
db.add(cursor)
|
||||
db.flush()
|
||||
if not user or not user.huihui_token:
|
||||
cursor.last_error = "请重新登录会会生产账号后再开启主动接管"
|
||||
cursor.last_polled_at = self.now()
|
||||
self._disable_after_connection_failure(
|
||||
db,
|
||||
avatar,
|
||||
cursor,
|
||||
"请重新登录会会生产账号后再开启主动接管",
|
||||
)
|
||||
db.commit()
|
||||
return False
|
||||
|
||||
@@ -222,10 +257,10 @@ class TakeoverService:
|
||||
except Exception as exc:
|
||||
if isinstance(exc, BoxIMError) and exc.auth_error:
|
||||
self._forget_boxim_session(user.id)
|
||||
cursor.last_error = "BOXIM 授权已失效,请重新登录会会生产账号"
|
||||
message = "BOXIM 授权已失效,请重新登录会会生产账号"
|
||||
else:
|
||||
cursor.last_error = f"BOXIM 暂时连接失败:{str(exc)[:160]}"
|
||||
cursor.last_polled_at = self.now()
|
||||
message = f"BOXIM 暂时连接失败:{str(exc)[:160]}"
|
||||
self._disable_after_connection_failure(db, avatar, cursor, message)
|
||||
db.commit()
|
||||
logger.warning("BOXIM sync failed for avatar %s: %s", avatar.id, exc)
|
||||
return False
|
||||
|
||||
@@ -6,7 +6,7 @@ from fastapi.testclient import TestClient
|
||||
|
||||
from database import SessionLocal
|
||||
from main import app
|
||||
from models import Authorization, TakeoverCursor, TakeoverReplyTask, User
|
||||
from models import Authorization, Avatar, TakeoverCursor, TakeoverReplyTask, User
|
||||
|
||||
|
||||
client = TestClient(app)
|
||||
@@ -203,3 +203,29 @@ def test_takeover_status_reports_ready_pending_count_and_errors(authorization_co
|
||||
failed = client.get(endpoint, headers=context["owner_headers"]).json()["data"]
|
||||
assert failed["status"] == "error"
|
||||
assert failed["message"] == "BOXIM 暂时不可用"
|
||||
|
||||
db = SessionLocal()
|
||||
try:
|
||||
avatar = db.query(Avatar).filter(Avatar.id == avatar_id).one()
|
||||
avatar.config = {"authorizationPermissions": ["chat"]}
|
||||
db.commit()
|
||||
finally:
|
||||
db.close()
|
||||
|
||||
auto_disabled = client.get(endpoint, headers=context["owner_headers"]).json()["data"]
|
||||
assert auto_disabled["enabled"] is False
|
||||
assert auto_disabled["status"] == "error"
|
||||
|
||||
client.put(
|
||||
f"/api/avatar/{avatar_id}/permission-settings",
|
||||
headers=context["owner_headers"],
|
||||
json={"permissions": ["chat", "takeover"]},
|
||||
)
|
||||
db = SessionLocal()
|
||||
try:
|
||||
cursor = db.query(TakeoverCursor).filter(TakeoverCursor.avatar_id == avatar_id).one()
|
||||
assert cursor.initialized is False
|
||||
assert cursor.last_message_id == "0"
|
||||
assert cursor.last_error == ""
|
||||
finally:
|
||||
db.close()
|
||||
|
||||
@@ -1,7 +1,7 @@
|
||||
"""End-to-end service tests for BOXIM takeover timing and human priority."""
|
||||
|
||||
from datetime import datetime, timedelta, timezone
|
||||
from unittest.mock import patch
|
||||
from unittest.mock import AsyncMock, patch
|
||||
|
||||
import pytest
|
||||
from sqlalchemy import create_engine
|
||||
@@ -10,6 +10,7 @@ from sqlalchemy.pool import StaticPool
|
||||
|
||||
from database import Base
|
||||
from models import Avatar, TakeoverCursor, TakeoverMessage, TakeoverReplyTask, User
|
||||
from services.boxim_client import BoxIMError
|
||||
from services.takeover_service import TakeoverService, _plain_text_reply
|
||||
|
||||
|
||||
@@ -192,5 +193,28 @@ async def test_quick_successive_messages_are_coalesced_into_one_reply(service_co
|
||||
db.close()
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_connection_failure_disables_takeover_and_stops_retrying(service_context):
|
||||
session_factory, service, boxim, _ = service_context
|
||||
boxim.exchange_access_token = AsyncMock(
|
||||
side_effect=BoxIMError("无效的访问令牌", code=40101, auth_error=True)
|
||||
)
|
||||
|
||||
await service.poll_and_process_messages()
|
||||
await service.poll_and_process_messages()
|
||||
|
||||
db = session_factory()
|
||||
try:
|
||||
avatar = db.query(Avatar).one()
|
||||
cursor = db.query(TakeoverCursor).one()
|
||||
assert "takeover" not in avatar.config["authorizationPermissions"]
|
||||
assert cursor.initialized is False
|
||||
assert "重新登录" in cursor.last_error
|
||||
assert db.query(TakeoverReplyTask).count() == 0
|
||||
finally:
|
||||
db.close()
|
||||
boxim.exchange_access_token.assert_awaited_once_with("prod-huihui-token")
|
||||
|
||||
|
||||
def test_plain_text_reply_removes_markdown_and_empty_lines():
|
||||
assert _plain_text_reply("## 建议\n\n**不能自行用药**\n`必要时就医`") == "建议\n不能自行用药\n必要时就医"
|
||||
|
||||
Reference in New Issue
Block a user