fix: corner-case audit — distinguish failures, recover from blips

Three small but real correctness fixes from auditing recent commits:

1. getPlayableSongIds returned an empty Set for both "endpoint failed"
   and "succeeded but all unplayable" — the caller couldn't tell which.
   Return Set | null now: null = error (fall through to sequential
   retry), empty Set = authoritative "all unplayable" (short-circuit
   to a clear message instead of wasting 20+ retries).

2. Web store: fetchHomeData unconditionally wrote lastFetchTime even
   when every fetch rejected (network blip, server down). That cached
   the failure for 5 minutes — user had to hard-reload to recover.
   Now only commit lastFetchTime if at least one auth-status call
   succeeded.

3. Settings profile section: if GET /profile failed, profileConfigs
   stayed undefined and the row showed "加载中..." forever. Track a
   per-bot error state and render an inline "加载失败 / 重试" link
   so the user can recover without page reload.

Out of scope but documented:
- ein=29 hardcoded in fetchCollectedPlaylists (no pagination yet —
  users with 30+ collected QQ playlists get truncated).
- /play-song single-failure UX (returns "Cannot play" message but
  frontend ignores; needs a global toast/notification primitive).
- NetEase has no analogous batch precheck (could surface same
  "click and wait silent" issue if user has region-restricted NetEase
  playlists).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
saopig1andClaude Opus 4.7 committed 2026-05-06 15:46:31 +08:00
1 parent 3d0aca52d0
commit 892d9f7959
4 files changed
+56 -14

No files matched your search

+17 -6
View File
@@ -124,24 +124,35 @@ export class QQMusicProvider implements MusicProvider {
* The wrapper's /getMusicPlay accepts a comma-separated songmid list * The wrapper's /getMusicPlay accepts a comma-separated songmid list
* and resolves all of them in a single upstream call (~2-3s for 100+ * and resolves all of them in a single upstream call (~2-3s for 100+
* songs), so this is much cheaper than per-song probing. * songs), so this is much cheaper than per-song probing.
*
* Returns:
* - non-null Set: authoritative result. Empty Set means all songs are
* unplayable; non-empty means filter to those mids.
* - null: the batch endpoint failed (timeout/exception). Caller
* should fall back to sequential retry rather than treating as
* "all unplayable", since we don't actually know.
*
* TODO: songIds with 1000+ entries may exceed URL length; chunk if
* we ever support that scale.
*/ */
async getPlayableSongIds(songIds: string[]): Promise<Set<string>> { async getPlayableSongIds(songIds: string[]): Promise<Set<string> | null> {
if (songIds.length === 0) return new Set(); if (songIds.length === 0) return new Set();
try { try {
const res = await this.api.get("/getMusicPlay", { const res = await this.api.get("/getMusicPlay", {
params: { songmid: songIds.join(","), quality: this.quality, ...this.cookieParams }, params: { songmid: songIds.join(","), quality: this.quality, ...this.cookieParams },
}); });
const playUrlMap: Record<string, { url?: string }> = const playUrlMap: Record<string, { url?: string }> | undefined =
res.data?.data?.playUrl ?? {}; res.data?.data?.playUrl;
// Distinguish "endpoint returned no playUrl object at all" (treat
// as failure → null) from "returned an empty/all-unplayable map".
if (!playUrlMap) return null;
const playable = new Set<string>(); const playable = new Set<string>();
for (const [mid, info] of Object.entries(playUrlMap)) { for (const [mid, info] of Object.entries(playUrlMap)) {
if (info?.url) playable.add(mid); if (info?.url) playable.add(mid);
} }
return playable; return playable;
} catch { } catch {
// On any error, fall through to per-song retry path — return empty return null;
// and let the caller try songs sequentially via getSongUrl.
return new Set();
} }
} }
+7 -5
View File
@@ -255,15 +255,17 @@ export function createPlayerRouter(
// ones, otherwise the playback retry loop wastes time guessing. // ones, otherwise the playback retry loop wastes time guessing.
let queueable: { id: string }[] = songs; let queueable: { id: string }[] = songs;
const totalCount = songs.length; const totalCount = songs.length;
const qqLike = provider as { getPlayableSongIds?: (ids: string[]) => Promise<Set<string>> }; const qqLike = provider as { getPlayableSongIds?: (ids: string[]) => Promise<Set<string> | null> };
if (typeof qqLike.getPlayableSongIds === "function") { if (typeof qqLike.getPlayableSongIds === "function") {
const playable = await qqLike.getPlayableSongIds(songs.map((s: { id: string }) => s.id)); const playable = await qqLike.getPlayableSongIds(songs.map((s: { id: string }) => s.id));
if (playable.size > 0) { if (playable !== null) {
// Authoritative answer from upstream — even an empty set means
// "we know none are playable", short-circuit immediately rather
// than wasting 20+ retries.
queueable = songs.filter((s: { id: string }) => playable.has(s.id)); queueable = songs.filter((s: { id: string }) => playable.has(s.id));
} }
// If batch returned nothing, leave queueable as-is and let the // If null, the batch endpoint itself errored — fall through to
// sequential retry path try anyway (handles the case where the // the sequential retry path, which still has a chance.
// batch endpoint failed entirely).
} }
if (queueable.length === 0) { if (queueable.length === 0) {
res.json({ message: `Loaded ${totalCount} songs but none were playable (likely copyright/region restrictions).` }); res.json({ message: `Loaded ${totalCount} songs but none were playable (likely copyright/region restrictions).` });
+8
View File
@@ -426,7 +426,15 @@ export const usePlayerStore = defineStore('player', {
this.bilibiliPopular = bili.value.data.songs ?? []; this.bilibiliPopular = bili.value.data.songs ?? [];
} }
// Only mark as fetched if at least the auth-status calls succeeded —
// a fully failed fetch (network blip / server down) should NOT be
// cached for 5 minutes, otherwise the user has to hard-reload to
// recover when connectivity returns.
const authOk =
neAuthRes.status === 'fulfilled' || qqAuthRes.status === 'fulfilled';
if (authOk) {
this.lastFetchTime = Date.now(); this.lastFetchTime = Date.now();
}
}, },
}, },
}); });
+24 -3
View File
@@ -414,7 +414,11 @@
<span class="profile-bot-name">{{ bot.name }}</span> <span class="profile-bot-name">{{ bot.name }}</span>
</button> </button>
<div v-if="profileExpanded[bot.id]" class="profile-toggles"> <div v-if="profileExpanded[bot.id]" class="profile-toggles">
<div v-if="!profileConfigs[bot.id]" class="profile-loading">加载中...</div> <div v-if="profileLoadError[bot.id]" class="profile-loading profile-error">
{{ profileLoadError[bot.id] }}
<button class="btn-link" @click="loadProfileConfig(bot.id)">重试</button>
</div>
<div v-else-if="!profileConfigs[bot.id]" class="profile-loading">加载中...</div>
<label <label
v-else v-else
v-for="t in PROFILE_TOGGLES" v-for="t in PROFILE_TOGGLES"
@@ -771,14 +775,18 @@ const PROFILE_TOGGLES: ReadonlyArray<{
const profileConfigs = reactive<Record<string, ProfileConfig>>({}); const profileConfigs = reactive<Record<string, ProfileConfig>>({});
const profileExpanded = reactive<Record<string, boolean>>({}); const profileExpanded = reactive<Record<string, boolean>>({});
const profileLoadError = reactive<Record<string, string | null>>({});
async function loadProfileConfig(botId: string) { async function loadProfileConfig(botId: string) {
if (profileConfigs[botId]) return; if (profileConfigs[botId]) return;
profileLoadError[botId] = null;
try { try {
const res = await axios.get(`/api/player/${botId}/profile`); const res = await axios.get(`/api/player/${botId}/profile`);
profileConfigs[botId] = res.data; profileConfigs[botId] = res.data;
} catch { } catch (err: any) {
// bot not loaded or no profile manager yet profileLoadError[botId] = err?.response?.status === 404
? '机器人未加载'
: '加载失败,请重试';
} }
} }
@@ -1299,6 +1307,19 @@ onUnmounted(() => {
text-align: center; text-align: center;
} }
.profile-error {
color: var(--brand-netease); // re-uses red brand color for error state
.btn-link {
margin-left: 8px;
font-size: 13px;
color: var(--color-primary);
text-decoration: underline;
background: transparent;
cursor: pointer;
}
}
.profile-toggle { .profile-toggle {
display: flex; display: flex;
align-items: center; align-items: center;