Skip to content

fix(plugins): report hot-list fetch failures instead of an empty list - #2240

Closed
lihongyuan99 wants to merge 1 commit into
TencentCloud:developfrom
lihongyuan99:fix/hot-topics-fetch-failure-reported
Closed

lihongyuan99 wants to merge 1 commit into
TencentCloud:developfrom
lihongyuan99:fix/hot-topics-fetch-failure-reported

Conversation

@lihongyuan99

@lihongyuan99 lihongyuan99 commented Oct 20, 2026 •

Copy link
Copy Markdown
Contributor

Summary

When get_hot_topics cannot reach the upstream, it tells the model the hot list is empty instead of that
the fetch failed — so the assistant reports "no trending items today" for a 403, a timeout or a DNS failure,
and may invent a list to fill the gap.

src/octop/infra/agents/plugins/bundled/hot-topics/main.py folds the two outcomes into one payload:

try:
    items = fetchers[src](n)
except Exception:
    return _payload({"source": src, "items": [], "silent": True}, "")
if not items:
    return _payload({"source": src, "items": [], "silent": True}, "")

The exception object is dropped (the module imports no logger, so nothing reaches the operator either) and
text is "", which is the only part the model reads as a tool result. A failed fetch and a real
zero-row answer are byte-identical from the model's point of view.

That is not the convention this repo uses — silent means "succeeded with nothing to show", and every
other bundled plugin that has a failure path separates them:

  • github-trending/main.py:55 → {"items": [], "error": str(exc), …} + "获取 GitHub 趋势失败:{exc}"
  • movie-search/main.py:74 → {"items": [], "error": str(exc)} + "搜索失败:{exc}", and silent only at :76
  • wiki-summary/main.py:71 → {"error": str(exc), …} + "维基查询失败:{exc}", and silent only at :73
  • sports-scores/main.py:49 → {"items": [], "error": str(exc)} + "赛况获取失败:{exc}"

hot-topics is inconsistent even with itself: the unknown-source branch right above (:129) does return
error plus a human-readable message. And this path is not hypothetical — _weibo() carries the comment
"Without Referer this endpoint returns 403 for server-side clients", i.e. the upstream is known to answer
4xx, and all three fetchers raise on any transport error, timeout or non-2xx status.

The fix mirrors the sibling pattern: except Exception as exc → data.error = str(exc) and a Chinese
message, leaving silent for the genuinely-empty case. No renderer change is needed — hot-topics/ui/index.js:15
already returns null when d.error is set, in the same condition that already covers d.silent, so the
card renders nothing in both cases and the difference is only what the model is told.

Target branch

  • Base is develop (feature / fix — default)
  • Base is main (release/* or hotfix/* only)

Type of change

  • Bug fix
  • New feature
  • Breaking change
  • Documentation
  • Refactor / chore

Test plan

  • Gates run by hand — make all also builds the dashboard, which needs a node_modules install that
    is not available in this environment
  • Added/updated tests

New regression test tests/unit/test_hot_topics_plugin.py::test_fetch_failure_is_not_reported_as_an_empty_hot_list
loads the plugin exactly as the existing tests do (default_bundled_plugins_root() + spec_from_file_location),
monkeypatches the module's httpx.Client to raise, and pins both sides of the contract — a failing fetch
carries error + message, an empty hot list keeps {"items": [], "silent": True} and text == "" — so the
fix cannot be "fixed" back into conflating them again.

Red before the fix (develop@58327d2e source, the failing fetch leaves no error key at all):

>       assert data["data"]["error"] == "http 403"
E       KeyError: 'error'
1 failed, 2 passed in 2.14s

Green after:

  • pytest -q tests/unit/test_hot_topics_plugin.py tests/unit/test_bundled_plugins_layout.py -> 7 passed in 1.71s
  • ruff check src tests -> All checks passed!
  • ruff format --check src tests -> 1241 files already formatted
  • mypy src/octop -> Success: no issues found in 597 source files
  • full pytest -q -m "not live" -n 3 on this head -> 5043 passed, 35 skipped, 0 failed in 420.27s
  • baseline on unmodified develop@58327d2e in the same Python 3.12 venv: 5042 passed, 0 failed, 35 skipped in 381.93s

5043 = 5042 + this PR's one new case and the skipped count did not move, so nothing else changed status.
No request is made in the test: the fetcher is replaced with a fake client that raises, so CI needs no
network, no locale and no clock, and the two pre-existing tests (test_weibo_sends_referer,
test_zhihu_uses_public_hot_list) keep their original expectations.

Checklist

  • One problem per PR: one payload branch in one plugin — except Exception: became
    except Exception as exc: and the payload carries error + text. _payload(), the three fetchers,
    silent for the empty case, the manifest and ui/index.js are untouched
  • Updated CHANGELOG.md (if user-facing) — one bullet under ## [Unreleased] → ### 修复
  • README / docs updated (if needed) — the plugin's documented behaviour is unchanged; what changes is
    that a failure no longer reports itself as success
  • No UI change, so no screenshot applies — ui/index.js:15 already returns null for d.error, which
    is what it did for d.silent before
  • No existing test was relaxed or rewritten
  • Overlap checked: no open pull request touches bundled/hot-topics/ (title scan of the first 200 open
    PRs plus the file lists of the 30 open PRs in this account)

2026-10-21 维护记录(07:00 赛道)

  • Rebased onto develop@4632d0f42. Upstream moved six commits since this branch's previous base and their CHANGELOG.md entries landed on the same line as this branch's bullet, so that file was the only conflict; the entry is re-applied inside the current ## [Unreleased] → ### 修复 list. Diff vs develop for CHANGELOG.md stays +1/-0 (no upstream line dropped, nothing added anywhere else).
  • The code patch is byte-identical to the previous head (the non-CHANGELOG files were hash-compared before and after the rebase), so this push changes no behaviour.
  • New head: 1efc0ab1d. Gates re-run on that head in a uv sync-ed Python 3.12 venv (a worktree venv left on the old lock produces false reds): RC ruff=0 format=0 mypy=0 targeted=0 — 0 means clean for ruff check / ruff format --check / mypy src/octop / this PR's own tests.
  • ruff format --check: 1248 files already formatted; mypy src/octop: Success: no issues found in 602 source files
  • This PR's tests on the new base: 3 passed in 1.37s
  • Head-vs-develop delta (only this PR's files): 1 0 CHANGELOG.md 3 3 src/octop/infra/agents/plugins/bundled/hot-topics/main.py 28 0 tests/unit/test_hot_topics_plugin.py
  • Full suite measured today on the exact base develop@4632d0f42 with the equivalent of make test (-m "not live" -n 3, NO_PROXY=127.0.0.1,localhost): 5078 passed, 35 skipped, 0 failed in 422.14s. Not re-run per branch: the rebased tree differs from that baseline only by the files listed above, which are covered by the targeted run above and by this head's own CI check-runs. Live tests stay skipped on forks (no secrets).

get_hot_topics() caught every fetcher exception and returned the same
payload it uses for a genuinely empty hot list: items=[], silent=True and
an empty text. The exception object was dropped and nothing is logged, so
the model received an empty tool result for a 403 or a timeout and could
only tell the user that today's hot list happens to be empty.

silent is this repo's convention for success with zero rows -- every other
bundled plugin with an error path (github-trending, movie-search,
wiki-summary, sports-scores) returns data.error plus a human-readable
message, and hot-topics itself already does that for an unknown source.
Return the same shape on failure; ui/index.js already returns null when
d.error is set, so the card rendering does not change.
@lihongyuan99
lihongyuan99 force-pushed the fix/hot-topics-fetch-failure-reported branch from b7fb0bd to 1efc0ab Compare October 20, 2026 23:09
@jubaoliang jubaoliang closed this Oct 22, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants