From 892d9f795982068b6535a081e2c4a61d3537677b Mon Sep 17 00:00:00 2001 From: saopig1 <4x7sw862st@gmail.com> Date: Wed, 6 May 2026 15:46:31 +0800 Subject: [PATCH] =?UTF-8?q?fix:=20corner-case=20audit=20=E2=80=94=20distin?= =?UTF-8?q?guish=20failures,=20recover=20from=20blips?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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) --- src/music/qq.ts | 23 +++++++++++++++++------ src/web/api/player.ts | 12 +++++++----- web/src/stores/player.ts | 10 +++++++++- web/src/views/Settings.vue | 27 ++++++++++++++++++++++++--- 4 files changed, 57 insertions(+), 15 deletions(-) diff --git a/src/music/qq.ts b/src/music/qq.ts index 78f31a2..47a3b95 100644 --- a/src/music/qq.ts +++ b/src/music/qq.ts @@ -124,24 +124,35 @@ export class QQMusicProvider implements MusicProvider { * The wrapper's /getMusicPlay accepts a comma-separated songmid list * and resolves all of them in a single upstream call (~2-3s for 100+ * 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> { + async getPlayableSongIds(songIds: string[]): Promise | null> { if (songIds.length === 0) return new Set(); try { const res = await this.api.get("/getMusicPlay", { params: { songmid: songIds.join(","), quality: this.quality, ...this.cookieParams }, }); - const playUrlMap: Record = - res.data?.data?.playUrl ?? {}; + const playUrlMap: Record | undefined = + 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(); for (const [mid, info] of Object.entries(playUrlMap)) { if (info?.url) playable.add(mid); } return playable; } catch { - // On any error, fall through to per-song retry path — return empty - // and let the caller try songs sequentially via getSongUrl. - return new Set(); + return null; } } diff --git a/src/web/api/player.ts b/src/web/api/player.ts index 63b9267..dfaf30e 100644 --- a/src/web/api/player.ts +++ b/src/web/api/player.ts @@ -255,15 +255,17 @@ export function createPlayerRouter( // ones, otherwise the playback retry loop wastes time guessing. let queueable: { id: string }[] = songs; const totalCount = songs.length; - const qqLike = provider as { getPlayableSongIds?: (ids: string[]) => Promise> }; + const qqLike = provider as { getPlayableSongIds?: (ids: string[]) => Promise | null> }; if (typeof qqLike.getPlayableSongIds === "function") { 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)); } - // If batch returned nothing, leave queueable as-is and let the - // sequential retry path try anyway (handles the case where the - // batch endpoint failed entirely). + // If null, the batch endpoint itself errored — fall through to + // the sequential retry path, which still has a chance. } if (queueable.length === 0) { res.json({ message: `Loaded ${totalCount} songs but none were playable (likely copyright/region restrictions).` }); diff --git a/web/src/stores/player.ts b/web/src/stores/player.ts index 25d5cf0..bf24a07 100644 --- a/web/src/stores/player.ts +++ b/web/src/stores/player.ts @@ -426,7 +426,15 @@ export const usePlayerStore = defineStore('player', { this.bilibiliPopular = bili.value.data.songs ?? []; } - this.lastFetchTime = Date.now(); + // 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(); + } }, }, }); diff --git a/web/src/views/Settings.vue b/web/src/views/Settings.vue index 5431687..f8ce798 100755 --- a/web/src/views/Settings.vue +++ b/web/src/views/Settings.vue @@ -414,7 +414,11 @@ {{ bot.name }}
-
加载中...
+
+ {{ profileLoadError[bot.id] }} + +
+
加载中...