Repository navigation
fix(plugins): report hot-list fetch failures instead of an empty list - #2240
Closed
lihongyuan99 wants to merge 1 commit into
Closed
lihongyuan99 wants to merge 1 commit into
lihongyuan99 wants to merge 1 commit into
Conversation
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
force-pushed
the
fix/hot-topics-fetch-failure-reported
branch
from
October 20, 2026 23:09
b7fb0bd to
1efc0ab
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
When
get_hot_topicscannot reach the upstream, it tells the model the hot list is empty instead of thatthe 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.pyfolds the two outcomes into one payload:The exception object is dropped (the module imports no logger, so nothing reaches the operator either) and
textis"", which is the only part the model reads as a tool result. A failed fetch and a realzero-row answer are byte-identical from the model's point of view.
That is not the convention this repo uses —
silentmeans "succeeded with nothing to show", and everyother 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}", andsilentonly at:76wiki-summary/main.py:71→{"error": str(exc), …}+"维基查询失败:{exc}", andsilentonly at:73sports-scores/main.py:49→{"items": [], "error": str(exc)}+"赛况获取失败:{exc}"hot-topicsis inconsistent even with itself: the unknown-source branch right above (:129) does returnerrorplus 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 Chinesemessage, leaving
silentfor the genuinely-empty case. No renderer change is needed —hot-topics/ui/index.js:15already returns
nullwhend.erroris set, in the same condition that already coversd.silent, so thecard renders nothing in both cases and the difference is only what the model is told.
Target branch
develop(feature / fix — default)main(release/*orhotfix/*only)Type of change
Test plan
make allalso builds the dashboard, which needs anode_modulesinstall thatis not available in this environment
New regression test
tests/unit/test_hot_topics_plugin.py::test_fetch_failure_is_not_reported_as_an_empty_hot_listloads the plugin exactly as the existing tests do (
default_bundled_plugins_root()+spec_from_file_location),monkeypatches the module's
httpx.Clientto raise, and pins both sides of the contract — a failing fetchcarries
error+ message, an empty hot list keeps{"items": [], "silent": True}andtext == ""— so thefix cannot be "fixed" back into conflating them again.
Red before the fix (
develop@58327d2esource, the failing fetch leaves noerrorkey at all):Green after:
pytest -q tests/unit/test_hot_topics_plugin.py tests/unit/test_bundled_plugins_layout.py->7 passed in 1.71sruff check src tests->All checks passed!ruff format --check src tests->1241 files already formattedmypy src/octop->Success: no issues found in 597 source filespytest -q -m "not live" -n 3on this head ->5043 passed, 35 skipped, 0 failed in 420.27sdevelop@58327d2ein the same Python 3.12 venv:5042 passed, 0 failed, 35 skipped in 381.93s5043 = 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
except Exception:becameexcept Exception as exc:and the payload carrieserror+ text._payload(), the three fetchers,silentfor the empty case, the manifest andui/index.jsare untouchedCHANGELOG.md(if user-facing) — one bullet under## [Unreleased]→### 修复that a failure no longer reports itself as success
ui/index.js:15already returnsnullford.error, whichis what it did for
d.silentbeforebundled/hot-topics/(title scan of the first 200 openPRs plus the file lists of the 30 open PRs in this account)
2026-10-21 维护记录(07:00 赛道)
develop@4632d0f42. Upstream moved six commits since this branch's previous base and theirCHANGELOG.mdentries 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 forCHANGELOG.mdstays+1/-0(no upstream line dropped, nothing added anywhere else).1efc0ab1d. Gates re-run on that head in auv 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 files3 passed in 1.37sdevelop@4632d0f42with the equivalent ofmake 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).