fix: surface underlying cause when MiOT device refresh fails

[mvanhorn]

Summary

/api/miot/refresh_miot_devices and /api/miot/device_list failed with the opaque, doubled message Failed to refresh MiOT devices: Failed to refresh MiOT devices and no underlying cause in the logs, so the real failure (timeout, auth scope, unsupported device type) was invisible even at ERROR level. The refresh paths now attach the actual underlying exception as the cause.

Why this matters

The reporter in #341 pointed out that the exception was raised bare, with no wrapped cause: the proxy layer caught the real exception, logged a single line, and returned None, so the service layer only ever saw a falsy result and raised a generic MiotServiceException with the original error already discarded.

MiotProxy.refresh_devices() in backend/miloco/src/miloco/miot/client.py caught the exception from get_devices_async(), logged it, and returned None. MiotService.refresh_miot_devices() then hit if not result: raise MiotServiceException("Failed to refresh MiOT devices") inside its own try, which the surrounding except Exception as e: raise MiotServiceException(f"...: {str(e)}") from e re-wrapped into the doubled message and chained __cause__ to that generic inner exception instead of the real fault. get_miot_device_list() had the same shape.

Changes

• refresh_devices() re-raises the caught exception instead of returning None, so the existing ... from e handlers in refresh_miot_devices() and get_miot_device_list() attach the real cause. The genuinely-empty-but-successful refresh now raises a distinct No MiOT devices found message rather than masking a fault as a refresh failure.
• The MIPS connect callback was registered directly to refresh_devices, which the SDK schedules via asyncio.ensure_future without reading its result. A small _refresh_devices_on_reconnect shim now swallows and logs reconnect-time failures, so a transient reconnect does not surface as an unretrieved task exception while awaited API callers still receive the raised cause.
• get_device_spec() and list_cameras_with_state() call the lazy get_devices() without a guard. They now wrap it in MiotServiceException(...) from e, matching the handler idiom already used by get_miot_device_list() and get_device_status(), so a lazy-refresh failure returns a structured error instead of a raw 500.

Testing

Added backend/miloco/tests/test_miot_refresh_errors.py. The error paths assert __cause__ is the original exception and the message carries the underlying text (and is not the doubled string); the happy path still returns True; the genuinely-empty refresh reports No MiOT devices found after refresh with no cause; refresh_miot_info() still records the per-label error in its errors list when refresh_devices() raises; and get_device_spec() surfaces the wrapped cause.

Running pytest over the touched MiOT suites: 97 passed.

Fixes #341

[CLAassistant]

All committers have signed the CLA.

[github-actions]

👋 感谢提交 PR @mvanhorn！维护者会尽快 review。

提交前请确认：

• CI 全绿（test / lint / build）
• 改动聚焦单一主题，便于审阅
• 若改动了依赖（lockfile / pyproject.toml / package.json），需维护者评论 /allow-dependencies-change <当前 head SHA> 放行（之后再 push 需重新放行）

[github-actions]

PR #369: fix: surface underlying cause when MiOT device refresh fails

作者: mvanhorn
范围: fix/341-fix-surface-underlying-cause-when-miot-d → main

本轮基于 PR 最新 head（2eb1835，CI merge commit 504573c）审查。相对 origin/main 的 diff 干净（无 phantom deletion）。上一轮 ci-bot 与 hlccd 提出的全部 🟡 均已在本 head 修复（见下方基线对账），本轮复核确认修复扎实，无新增 🔴/🟡。

修改方案

要解决的问题：/api/miot/refresh_miot_devices、/api/miot/device_list 失败时只抛出含糊且重复的 Failed to refresh MiOT devices: Failed to refresh MiOT devices，真实原因（超时 / 鉴权 scope / 不支持的设备类型）连 ERROR 日志都看不到（#341）。根因是代理层把真异常吞掉、只记一行日志后 return None，service 层只看到 falsy 结果，于是抛一个泛化 MiotServiceException，str(e) 又正好是上一句、from e 也链到这个通用异常而非真故障。

整体方案：

• 把代理层设备刷新失败语义从「记日志后返回 None」改成「记日志后重新抛出原始异常」，这样 service 层既有的 except Exception as e: ... from e 就能把真异常挂到 __cause__ 上、并把原文拼进消息（refresh_devices）。
• 把「刷新失败」与「刷新成功但账号下确实没有设备」分开：空结果改报一句独立的 No MiOT devices found after refresh，不再伪装成刷新失败；service 层为此在泛化 except 之前先加 except MiotServiceException: raise，让这句自抛消息原样透出、不被二次包装成 doubled message（refresh_miot_devices）。
• 因为设备刷新现在会抛异常，而 MIPS 重连回调是 SDK 用 asyncio.ensure_future 调度、从不读返回值——直接挂它会让一次瞬时重连失败变成「task 异常没人取」的告警。于是加一层垫片函数，在重连路径上吞掉并记日志、等下次 connect 自动重试；被 await 的 API 调用方仍能拿到抛出的真 cause（_refresh_devices_on_reconnect）。另两个把设备刷新当回调的监听器（bind / meta debounce）内部本就有 try/except 兜底并记 error（mips_listeners.py#L198、#L290），不受影响。
• 给几个此前裸调 lazy get_devices()、没有 try 守护的方法补上异常包装，让 lazy-refresh 失败返回结构化错误而非裸 500——覆盖 get_device_spec（L920）、list_cameras_with_state（L1250），以及前两轮 review 后新补的 _assert_did_in_allowed_home（控制路径前置校验，冷缓存 + MiOT 故障时不再把干净 4xx 降级成 5xx，且设备刷新失败时仍能走 camera 兜底校验）（_assert_did_in_allowed_home）。
• 前两轮 review 后新补的 家庭信息聚合刷新（refresh=True）：把无保护 gather 改成 return_exceptions=True 并逐项判失败，设备刷新瞬时失败时不再整体 500，而是读 _device_info_dict 内存缓存降级返回，scenes / cameras / persons 等仍能返回；且刻意不再走 lazy get_devices()（否则会二次触发同一个失败刷新）（get_home_info）。

关键设计原则：

1. 垫片分层：同一个 refresh_devices 方法，被 await 的 API 路径看真 cause、SDK 回调路径吞异常记日志——用一层 shim 函数隔离两种语义，而非在核心方法里加分支判断。为什么这么设计：核心方法保持单一语义（re-raise），调用方按需选 shim 或直接 await，避免"回调里要不要吞"的 flag 污染。
2. 缓存降级而非硬失败：设备刷新失败时读内存缓存（_device_info_dict）而非整体 500。为什么：家庭信息接口是前端主屏依赖，部分数据（scenes/cameras 正常 + 设备列表可能过期）远比白屏好；且刻意跳过 lazy get_devices() 防止二次触发同一个失败。
3. devices 失败 → camera 兜底校验：_assert_did_in_allowed_home 中设备查询失败时回退到 camera 缓存查询，两条数据源任一能定位到设备即放行家庭校验。为什么：camera 与 device 在 proxy 层是两套独立缓存，设备刷新失败不代表 camera 缓存也失效。

基线对账（Step 7.0）：

上轮提出	严重度	本轮状态
ci-bot：get_home_info gather 缺 return_exceptions → 设备刷新失败整体 500	🟡	✅ 已修（return_exceptions=True + 逐项判失败 + 缓存降级）
hlccd：_assert_did_in_allowed_home 未接住 lazy 刷新 → 控制路径 4xx 变 5xx	🟡	✅ 已修（try/except 包裹 + camera 兜底）
hlccd：两处新暴露路径缺回归测试	🔵	✅ 已补（见下方测试表末两行）
ci-bot：refresh_miot_cameras / scenes / user_info 同款 doubled message	🔵	⏸ 未改（pre-existing，PR 范围锁定 devices）
ci-bot：PR 描述漏列两处改动	🔵	⏸ 未改

测试覆盖：

关注点	test_miot_refresh_errors.py::用例	摘要
刷新失败链 cause	test_refresh_miot_devices_chains_underlying_refresh_error	__cause__ 为原 TimeoutError、消息含原文且非 doubled
device_list 失败链 cause	test_get_miot_device_list_chains_underlying_refresh_error	同上，走 get_devices 路径
happy path	test_refresh_miot_devices_returns_true_when_devices_refreshed	有设备时返回 True
空但成功	test_refresh_miot_devices_empty_refresh_reports_no_devices	报 No MiOT devices found after refresh 且 __cause__ 为 None
spec lazy 失败包装	test_get_device_spec_wraps_lazy_refresh_error	包成 MiotServiceException 带 cause
聚合刷新记错	test_refresh_miot_info_records_refresh_devices_error	refresh_miot_info 把 devices 标 False 并记进 errors 列表
校验路径 lazy 失败	test_assert_did_in_allowed_home_wraps_lazy_refresh_error	冷刷新失败时包成 MiotServiceException 带 cause（新增）
home_info 部分降级	test_get_home_info_refresh_returns_partial_data_when_devices_fail	设备刷新失败仍返回 scenes、走缓存不再调 get_devices（新增）

问题

🔵 建议（可选优化）

• backend/miloco/src/miloco/miot/service.py:407 — refresh_miot_cameras / refresh_miot_scenes / refresh_miot_user_info 三条 service 方法仍复刻 Failed to refresh MiOT devices — generic exception with no underlying cause logged #341 同款的 doubled message + 丢 cause

  • 背景: 本 PR 只把代理层 refresh_devices() 改成 re-raise，另外三个代理刷新方法 refresh_cameras() / refresh_scenes() / refresh_user_info() 仍在 except 里 return None（client.py#L768、#L1153、#L1192），吞掉真异常。

  • 问题: 于是这三条 service 方法会复刻 Failed to refresh MiOT devices — generic exception with no underlying cause logged #341 一模一样的 doubled message + 丢 cause。以 cameras 为例，云端读超时时：

    步骤	代码行为
    代理层 refresh_cameras() 抛超时	except Exception 捕获 → 只记日志 → return None，真 cause 丢失
    service refresh_miot_cameras 收到 falsy	if not result: raise MiotServiceException("Failed to refresh MiOT cameras")
    同函数外层 except Exception as e	raise MiotServiceException(f"...: {str(e)}") from e，str(e) 恰为上一句
    调用方 / 日志看到	Failed to refresh MiOT cameras: Failed to refresh MiOT cameras，__cause__ 链到通用异常而非超时——与 Failed to refresh MiOT devices — generic exception with no underlying cause logged #341 现象一致

    这是 pre-existing、且在 PR 声明范围之外，故列 🔵；但既然 PR 已经把 devices 这一份改对，顺带把同构的三份对齐能一次性消掉这个「多份要同步」的隐性 invariant。

  • 改进: 把三个代理刷新方法一并改成 re-raise（与 refresh_devices 同款），并在对应 service 方法的泛化 except 之前加 except MiotServiceException: raise 挡层。示例（refresh_cameras）：

    # client.py — 与 refresh_devices 对齐
    async def refresh_cameras(self) -> dict[str, MIoTCameraInfo] | None:
        async with self._refresh_cameras_lock:
            try:
                cameras = await self._miot_client.get_cameras_async()
                ...
                return cameras
            except Exception as e:
                logger.error("Failed to refresh cameras: %s", e)
                raise

    # service.py — 让自抛消息原样透出，不被二次包装
    async def refresh_miot_cameras(self):
        try:
            result = await self._miot_proxy.refresh_cameras()
            if not result:
                raise MiotServiceException("No MiOT cameras found after refresh")
            return True
        except MiotServiceException:
            raise
        except Exception as e:
            logger.error("Failed to refresh MiOT cameras: %s", e)
            raise MiotServiceException(
                f"Failed to refresh MiOT cameras: {str(e)}"
            ) from e

    若本 PR 想严格锁定在 Failed to refresh MiOT devices — generic exception with no underlying cause logged #341/devices，也可仅在 PR 描述里注明「cameras / scenes / user_info 的同款修复留待后续」，避免读者误以为已全量修复。

• PR 描述 — ## Changes 段漏列两处本 head 实际做出的重要改动

  • 背景: PR body 的 ## Changes 三条只讲了 ① refresh_devices re-raise、② MIPS 垫片、③ get_device_spec + list_cameras_with_state 包裹。

  • 问题: 但最新 head 的 diff 里另有两处同等重要的改动完全没在描述中出现：

    1. _assert_did_in_allowed_home 补了 lazy get_devices() 的 try/except（且带 camera 兜底逻辑）——与第 ③ 条同类，却未一并列出；
    2. get_home_info(refresh=True) 的 gather 改 return_exceptions=True + 缓存降级——这是一处独立的行为变更（从「任一刷新失败整体 500」变成「部分降级返回」），描述里只字未提。

    这两处恰是前两轮 review 后新增的核心修复，读者只看描述会以为 PR 范围仅限最初三条，review / 回溯时容易漏看。因 PR body 随时可编辑，成本极低，故列 🔵。

  • 改进: 在 ## Changes 补两条，例如：

    - `_assert_did_in_allowed_home()` 同样以 `MiotServiceException(...) from e`
      包裹 lazy `get_devices()`，并在 devices 刷新失败时回退到 camera 校验，
      避免控制路径把干净的 4xx 降级成原始 5xx。
    - `get_home_info(refresh=True)` 的并发刷新改用 `return_exceptions=True`
      并逐项判失败：device 刷新瞬时失败时读 `_device_info_dict` 缓存降级返回
      （scenes / cameras / persons 仍可用），不再整体 500。

结论

LGTM — 修复方向正确、错误可见性钉得扎实，前两轮 review 的全部 🟡 与可回归 🔵 均已消化，测试覆盖到位（含两处新暴露路径），全部 refresh_devices 运行时回调路径（bind / meta debounce、两处 gather、重连垫片、refresh_miot_info）均已对 re-raise 做了保护。仅剩两条 🔵（同构 cameras/scenes/user_info 刷新链的同款修复可顺带对齐；PR 描述补齐两处未列改动），均非阻塞，可合并后择机处理。

---

_{由 review-pr skill v1.6 生成}

[hlccd]

修复方向是对的。原来那句的 Failed to refresh MiOT devices: Failed to refresh MiOT devices 是两处叠加：refresh_devices 出错时 return None 先把真异常吞了；refresh_miot_devices 又在自己的 try 里 raise MiotServiceException(...)、紧接着被同一个 except Exception 重包一层，str(e) 正好是上一句、from e 也链到了这个通用异常而非真故障，所以超时 / 授权 scope / 设备类型这些真原因连 ERROR 日志都看不到（即 #341 现象）。

本 PR 的修法：refresh_devices 改为 re-raise（不再 return None）+ 在服务层通用 except 前加 except MiotServiceException: raise 挡层（避免已有意义的异常被二次包裹翻倍），让真异常经 ... from e 挂上真 cause；并给 MIPS 重连回调单独包 _refresh_devices_on_reconnect（SDK 用 ensure_future 触发、不读返回值，直接抛会冒 "Task exception was never retrieved" 告警）。这几步都对。

但目前看起来只完成了一半——底层复用路径没跟着处理，把原来的优雅降级悄悄改成了硬失败。以下需要补：

问题

🟡 重要（应当修复）

• backend/miloco/src/miloco/miot/service.py:114 — _assert_did_in_allowed_home 没接住 get_devices() 的懒刷新，冷缓存下会把控制路径的干净 4xx 变成原始异常 5xx

  • 背景: get_devices() 是 if not self._device_info_dict: await self.refresh_devices()（client.py），冷缓存（从未成功刷过）下调 refresh_devices() 现在会抛。_assert_did_in_allowed_home 是 control_device 等控制路径的前置校验。把 get_devices() 的 7 个调用方过了一遍——get_miot_device_list / get_device_spec / get_device_status / list_homes / list_cameras_with_state / get_home_info 都在 try 内，唯独此处裸调。
  • 问题: 冷启动 + MiOT 故障（token 过期 / 网络断）时，用户对某设备发控制指令，本该拿到"设备不在允许家庭 / 未找到"的干净 ResourceNotFoundException / ValidationException（4xx），实际会拿到 get_devices() 冒上来的原始 MiOT 异常、被上层兜成 500。
  • 改进: 给这处 get_devices() 补与其它方法一致的包裹；或更彻底地在 get_devices() 内部对懒刷新失败降级、把冒泡边界统一收到显式 refresh 入口。局部包裹版：

    try:
        devices = await self._miot_proxy.get_devices()
    except Exception as e:
        logger.error("Failed to load devices while checking home scope for %s: %s", did, e)
        raise MiotServiceException(f"Failed to load devices: {str(e)}") from e

• backend/miloco/src/miloco/miot/service.py:668 — get_home_info(refresh=True) 的 gather 漏了 return_exceptions=True，一个刷新失败拖垮整个接口

  • 背景: refresh=True 时并发刷新 devices / scenes / cameras；refresh_devices() 现在会抛。（行号以分支为准，位于 get_home_info 的 if refresh: 块内。）
  • 问题: device 刷新一抛，整个 gather 直接炸、被外层 except 兜成 MiotServiceException，接口整体失败——哪怕 scenes、cameras 都正常。原来 refresh_devices 返 None 时 gather 照过、能返回部分家庭数据，现在退化成硬失败。同文件 _background_refresh 那处 gather 正好带了 return_exceptions=True，两种写法并存。
  • 改进: 与 _background_refresh 对齐，逐项判失败：

    results = await asyncio.gather(
        self._miot_proxy.refresh_devices(),
        self._miot_proxy.refresh_scenes(),
        self._miot_proxy.refresh_cameras(),
        return_exceptions=True,
    )
    for r in results:
        if isinstance(r, Exception):
            logger.warning("get_home_info refresh partial failure: %s", r)

🔵 建议（可选优化）

• backend/miloco/tests/test_miot_refresh_errors.py — 新增测试没覆盖上面两处新暴露的路径
  • 背景: 现有用例挺全——refresh / device_list / 空结果 / get_device_spec 懒刷新链 / refresh_miot_info 都测了。
  • 问题: 正好缺 _assert_did_in_allowed_home 冷刷新失败、和 get_home_info(refresh=True) 的 gather 行为这两个恰恰是本 PR 改动新引入的分支，没有回归保护。
  • 改进: 补两个用例——① _assert_did_in_allowed_home 在懒刷新抛错时的行为符合修复后的预期（干净的域异常而非原始 5xx）；② get_home_info(refresh=True) 在 device 刷新失败、scenes/cameras 正常时仍能返回部分数据。

结论

需要修改 — 错误可见性修得对，但 get_devices() 懒刷新链的一个调用方 + get_home_info 的 gather 把原来的优雅降级改成了硬失败，建议连同测试一并补齐再合。

[mvanhorn]

Restored the graceful degradation in both places you flagged: get_home_info(refresh=True) now returns partial data when device refresh fails but scenes/cameras succeed, and the lazy-refresh chain raises the clean domain exception instead of hard-failing callers that previously tolerated it. Added the two regression tests you outlined in test_miot_refresh_errors.py (lazy-refresh failure surfaces the domain exception, and partial home info on device-refresh failure). ruff passes; CI will run the full pytest.

[mvanhorn]

Thanks @hlccd - agreed with your analysis. The branch already surfaces the underlying cause: refresh_devices re-raises instead of returning None, so the real exception propagates instead of the doubled message. Happy to adjust the error wording too if you'd like.

[hlccd]

麻烦老师空闲时修正一下

[mvanhorn]

Done - signed the CLA and rebased onto main to clear the conflict. The reconstructed list_cameras_with_state keeps main's three-state/channel logic wrapped in the underlying-cause error handling from this PR. Tests pass (the miot refresh-error suite plus the camera/service tests). Ready for review whenever you have time - thanks!