✨ feat: 添加lastfm支持 - #631
Conversation
lrst6963
commented
Dec 13, 2025
lrst6963
commented
Summary of ChangesHello @lrst6963, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! 此拉取请求引入了 Last.fm 集成功能，使用户能够将其音乐播放活动同步到 Last.fm 平台。这包括记录听歌历史（Scrobble）和实时更新当前正在播放的歌曲。通过在设置中配置 Last.fm API 凭据并授权连接账户，用户可以个性化其音乐分享体验。 Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://cdn.statically.io/img/avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
| <n-input | ||
| v-model:value="settingStore.lastfm.apiSecret" | ||
| placeholder="请输入 API Secret" | ||
| class="set" | ||
| type="password" | ||
| show-password-on="click" | ||
| /> |
There was a problem hiding this comment.
将 apiSecret 等敏感信息存储在 localStorage 中存在严重的安全风险。localStorage 中的数据可以被任何运行在同源的 JavaScript 代码访问，这使得它容易受到 XSS (跨站脚本) 攻击。攻击者可以窃取用户的 apiSecret，从而冒充用户执行操作。
对于 Electron 应用，建议将敏感数据存储在主进程中，并通过安全的 IPC 通道进行通信。可以使用 electron-store 在主进程中管理配置，或者使用 electron-keytar 将密钥安全地存储在操作系统的钥匙串中。
对于 Web 应用，不应该在客户端存储 apiSecret。签名过程应该在后端服务器上完成。
| const checkAuth = setInterval(async () => { | ||
| try { | ||
| // 尝试获取会话 | ||
| const sessionResponse = await getSession(token); | ||
|
|
||
| if (sessionResponse.session) { | ||
| clearInterval(checkAuth); | ||
| authWindow?.close(); | ||
|
|
||
| // 保存会话信息 | ||
| settingStore.lastfm.sessionKey = sessionResponse.session.key; | ||
| settingStore.lastfm.username = sessionResponse.session.name; | ||
|
|
||
| window.$message.success(`已成功连接到 Last.fm 账号: ${sessionResponse.session.name}`); | ||
| lastfmAuthLoading.value = false; | ||
| } | ||
| } catch (error) { | ||
| // 用户还未授权，继续等待 | ||
| } | ||
| }, 2000); |
There was a problem hiding this comment.
在 connectLastfm 函数中，setInterval 内部的 catch 块会“吞掉”所有错误。这意味着如果 getSession 因为网络问题或其他非“用户未授权”的原因失败，轮询会静默地继续，直到超时。这会导致用户体验不佳和难以调试的问题。
建议在 catch 块中检查具体的错误类型。Last.fm API 会返回错误码，例如错误码 14 (This token has not been authorized) 表示用户尚未授权，可以继续轮询。其他错误（如网络错误、API 密钥无效等）应该被捕获、停止轮询，并向用户显示相应的错误信息。
| public stop() { | ||
| this.clearScrobbleTimer(); | ||
| this.currentTrack = null; | ||
| this.hasScrobbled = false; | ||
| } |
There was a problem hiding this comment.
当前的 stop 方法逻辑不完整。它只清除了定时器和状态，但没有处理在歌曲结束或跳过时，如果满足 Scrobble 条件但定时器还未触发的情况。这会导致以下问题：
- 自然播放结束的歌曲如果播放时长不足以触发定时器，将不会被记录。
- 用户在满足 Scrobble 条件后、定时器触发前跳过歌曲，该次播放不会被记录。
建议修改 stop 方法，在清除状态前，检查是否满足 Scrobble 条件，如果满足则立即执行 scrobble()。
this.clearScrobbleTimer();
// 如果歌曲播放时间足够但尚未 scrobble，则立即执行
if (this.currentTrack && !this.hasScrobbled) {
const settingStore = useSettingStore();
if (settingStore.lastfm.scrobbleEnabled) {
const playedTime = (Date.now() - this.playStartTime) / 1000;
const duration = this.currentTrack.duration || 0;
// 歌曲必须长于30秒才能 scrobble
if (duration > 30) {
const scrobblePoint = Math.min(duration / 2, 240);
if (playedTime >= scrobblePoint) {
this.scrobble();
}
}
}
}
this.currentTrack = null;
this.hasScrobbled = false;| // Last.fm Scrobbler - 仅在新歌曲开始播放时触发 | ||
| if (isNewSong) { | ||
| const album = | ||
| typeof playSongData?.album === "string" | ||
| ? playSongData?.album | ||
| : playSongData?.album?.name; | ||
| const duration = playSongData?.duration ? Math.floor(playSongData.duration / 1000) : undefined; | ||
| lastfmScrobbler.startPlaying(name || "", artist || "", album, duration); | ||
| } else { | ||
| // 恢复播放 | ||
| lastfmScrobbler.resume(); | ||
| } |
There was a problem hiding this comment.
当切换歌曲时（isNewSong 为 true），当前的逻辑只为新歌曲调用了 startPlaying，但没有为刚刚被切掉的上一首歌曲调用 stop()。这导致用户跳过歌曲时，上一首歌的播放记录无法被正确终结和提交（Scrobble）。
为了修复这个问题，在调用 startPlaying 之前，应该先调用 lastfmScrobbler.stop() 来处理上一首歌曲的 Scrobble 逻辑。
// Last.fm Scrobbler
if (isNewSong) {
// 终结上一首歌曲的 scrobble
lastfmScrobbler.stop();
const album =
typeof playSongData?.album === "string"
? playSongData?.album
: playSongData?.album?.name;
const duration = playSongData?.duration ? Math.floor(playSongData.duration / 1000) : undefined;
lastfmScrobbler.startPlaying(name || "", artist || "", album, duration);
} else {
// 恢复播放
lastfmScrobbler.resume();
}| const lastfmPostRequest = async (method: string, params: Record<string, string | number> = {}) => { | ||
| const { apiKey } = getApiConfig(); | ||
| const requestParams: Record<string, string | number> = { | ||
| method, | ||
| api_key: apiKey, | ||
| format: "json", | ||
| ...params, | ||
| }; |
There was a problem hiding this comment.
lastfmPostRequest 和 lastfmRequest 函数中创建 requestParams 的代码几乎完全相同。这造成了代码重复。建议将这部分逻辑提取到一个独立的辅助函数中，以提高代码的可维护性和复用性。
例如，你可以创建一个 _prepareRequestParams 函数：
const _prepareRequestParams = (method: string, params: Record<string, string | number>) => {
const { apiKey } = getApiConfig();
return {
method,
api_key: apiKey,
format: "json",
...params,
};
};然后在 lastfmRequest 和 lastfmPostRequest 中调用它。
| const isLastfmConfigured = computed(() => { | ||
| const { apiKey, apiSecret } = settingStore.lastfm; | ||
| return Boolean(apiKey && apiSecret); | ||
| }); |
There was a problem hiding this comment.
此 isLastfmConfigured 计算属性的逻辑与 src/api/lastfm.ts 中已有的 isLastfmConfigured 函数重复。为了遵循 DRY (Don't Repeat Yourself) 原则，建议导入并复用该函数。
你可以在 <script setup> 中导入 isLastfmConfigured 并修改此计算属性：
import { isLastfmConfigured as isApiConfigured } from '@/api/lastfm';
const isLastfmConfigured = computed(() => isApiConfigured());这需要你同时更新 from '@/api/lastfm' 的导入语句。
There was a problem hiding this comment.
Pull request overview
This PR adds Last.fm integration support to the music player, enabling users to scrobble (record) their listening history to Last.fm and update their "now playing" status.
Key Changes
- Integrated Last.fm scrobbler with the player's playback lifecycle (play, pause, resume, stop)
- Implemented Last.fm API client with authentication flow and scrobbling capabilities
- Added settings UI for Last.fm configuration and account connection
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 14 comments.
Show a summary per file
| File | Description |
|---|---|
src/utils/player.ts |
Integrated Last.fm scrobbler hooks into playback events (play, pause, stop) |
src/utils/lastfmScrobbler.ts |
New scrobbler implementation managing track state and timing for Last.fm submissions |
src/api/lastfm.ts |
New Last.fm API client with authentication, scrobbling, and track management functions |
src/stores/setting.ts |
Added Last.fm configuration state (credentials, session, feature toggles) |
src/components/Setting/OtherSetting.vue |
Added UI for Last.fm setup, authentication flow, and feature controls |
README.md |
Updated feature list to include Last.fm Scrobble support |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| ? playSongData?.album | ||
| : playSongData?.album?.name; |
There was a problem hiding this comment.
The album extraction logic only handles cases where album is a string or an object with a name property. If playSongData.album is null, undefined, or has a different structure, the album variable could contain unexpected values. Consider adding explicit null/undefined checks or using optional chaining more defensively.
| ? playSongData?.album | |
| : playSongData?.album?.name; | |
| ? playSongData.album | |
| : (typeof playSongData?.album?.name === "string" ? playSongData.album.name : ""); |
| const connectLastfm = async () => { | ||
| try { | ||
| lastfmAuthLoading.value = true; | ||
|
|
||
| // 获取认证令牌 | ||
| const tokenResponse = await getAuthToken(); | ||
| if (!tokenResponse.token) { | ||
| throw new Error("无法获取认证令牌"); | ||
| } | ||
|
|
||
| const token = tokenResponse.token; | ||
|
|
||
| // 打开授权页面 | ||
| const authUrl = getAuthUrl(token); | ||
| if (typeof window !== "undefined") { | ||
| const authWindow = window.open(authUrl, "_blank", "width=800,height=600"); | ||
|
|
||
| // 轮询等待用户授权 | ||
| const checkAuth = setInterval(async () => { | ||
| try { | ||
| // 尝试获取会话 | ||
| const sessionResponse = await getSession(token); | ||
|
|
||
| if (sessionResponse.session) { | ||
| clearInterval(checkAuth); | ||
| authWindow?.close(); | ||
|
|
||
| // 保存会话信息 | ||
| settingStore.lastfm.sessionKey = sessionResponse.session.key; | ||
| settingStore.lastfm.username = sessionResponse.session.name; | ||
|
|
||
| window.$message.success(`已成功连接到 Last.fm 账号: ${sessionResponse.session.name}`); | ||
| lastfmAuthLoading.value = false; | ||
| } | ||
| } catch (error) { | ||
| // 用户还未授权，继续等待 | ||
| } | ||
| }, 2000); | ||
|
|
||
| // 30秒超时 | ||
| setTimeout(() => { | ||
| clearInterval(checkAuth); | ||
| if (lastfmAuthLoading.value) { | ||
| lastfmAuthLoading.value = false; | ||
| window.$message.warning("授权超时，请重试"); | ||
| } | ||
| }, 30000); | ||
| } | ||
| } catch (error: any) { | ||
| console.error("Last.fm 连接失败:", error); | ||
| window.$message.error(`连接失败: ${error.message || "未知错误"}`); | ||
| lastfmAuthLoading.value = false; | ||
| } | ||
| }; |
There was a problem hiding this comment.
There's a race condition if the user clicks the "连接账号" button multiple times rapidly. Each click will start a new authorization flow with its own polling interval and timeout, but only the loading state from the last click will be tracked. This can lead to multiple authorization windows opening and intervals running simultaneously. Add a guard to prevent multiple concurrent authorization attempts.
| public startPlaying(name: string, artist: string, album?: string, duration?: number) { | ||
| const settingStore = useSettingStore(); | ||
|
|
||
| // 检查 Last.fm 是否启用 | ||
| if (!this.isEnabled()) return; | ||
|
|
||
| // 清除之前的定时器 | ||
| this.clearScrobbleTimer(); | ||
|
|
||
| // 记录新歌���信息 | ||
| this.currentTrack = { | ||
| name, | ||
| artist, | ||
| album, | ||
| duration, | ||
| timestamp: Math.floor(Date.now() / 1000), | ||
| }; | ||
| this.playStartTime = Date.now(); | ||
| this.hasScrobbled = false; | ||
|
|
||
| console.log("Last.fm: 开始播放", this.currentTrack); | ||
|
|
||
| // 更新正在播放状态 | ||
| if (settingStore.lastfm.nowPlayingEnabled) { | ||
| this.updateNowPlaying(); | ||
| } | ||
|
|
||
| // 设置 scrobble 定时器 | ||
| if (settingStore.lastfm.scrobbleEnabled) { | ||
| this.scheduleScrobble(); | ||
| } | ||
| } |
There was a problem hiding this comment.
If startPlaying is called rapidly in succession (e.g., when quickly skipping through tracks), there's a potential race condition where the updateNowPlaying and scheduleScrobble operations from a previous track may still be in progress when a new track starts. This could lead to incorrect scrobbles being recorded. Consider adding a mechanism to cancel pending operations when starting a new track.
| const playedTime = (Date.now() - this.playStartTime) / 1000; | ||
| const remainingTime = Math.max(0, scrobbleTime - playedTime); |
There was a problem hiding this comment.
When resuming after a pause, the played time calculation only considers the time from the initial play start, not accounting for the pause duration. This means if a user pauses for an extended period, the scrobble will happen earlier than intended relative to actual listening time. The playStartTime should be adjusted when pausing/resuming to track only actual listening time.
| lastfm: { | ||
| enabled: boolean; | ||
| apiKey: string; | ||
| apiSecret: string; | ||
| sessionKey: string; | ||
| username: string; | ||
| scrobbleEnabled: boolean; | ||
| nowPlayingEnabled: boolean; | ||
| }; |
There was a problem hiding this comment.
Sensitive credentials (apiKey, apiSecret, sessionKey) are stored in the settings store which may be persisted to localStorage or other storage mechanisms. These credentials should be stored securely, especially apiSecret and sessionKey, as they provide full access to the user's Last.fm account. Consider using encrypted storage or a more secure credential management approach.
| console.error("Last.fm API POST 错误:", error); | ||
| throw error; |
There was a problem hiding this comment.
The error handling in this catch block is identical to the one in lastfmRequest (line 71-72), resulting in duplicated error handling logic. Consider extracting this into a shared error handling function to maintain DRY principles.
| placeholder="请输入 API Secret" | ||
| class="set" | ||
| type="password" | ||
| show-password-on="click" |
There was a problem hiding this comment.
The API Secret input field uses type="password" which is appropriate, but consider adding autocomplete="off" to prevent browsers from auto-filling this sensitive field with saved passwords, which could lead to credential confusion.
| show-password-on="click" | |
| show-password-on="click" | |
| autocomplete="off" |
| const checkAuth = setInterval(async () => { | ||
| try { | ||
| // 尝试获取会话 | ||
| const sessionResponse = await getSession(token); | ||
|
|
||
| if (sessionResponse.session) { | ||
| clearInterval(checkAuth); | ||
| authWindow?.close(); | ||
|
|
||
| // 保存会话信息 | ||
| settingStore.lastfm.sessionKey = sessionResponse.session.key; | ||
| settingStore.lastfm.username = sessionResponse.session.name; | ||
|
|
||
| window.$message.success(`已成功连接到 Last.fm 账号: ${sessionResponse.session.name}`); | ||
| lastfmAuthLoading.value = false; | ||
| } | ||
| } catch (error) { | ||
| // 用户还未授权，继续等待 | ||
| } | ||
| }, 2000); |
There was a problem hiding this comment.
The polling mechanism for checking authorization status lacks proper cleanup. If the user closes the authorization window manually or navigates away from the settings page, the interval will continue running indefinitely, potentially causing memory leaks and unnecessary API calls.
Consider adding:
- A listener to detect when the auth window is closed
- Cleanup of the interval when the component is unmounted
- Storing the interval ID in a component ref so it can be cleared on unmount
| public stop() { | ||
| this.clearScrobbleTimer(); | ||
| this.currentTrack = null; | ||
| this.hasScrobbled = false; | ||
| } |
There was a problem hiding this comment.
The stop method clears the scrobble timer but doesn't attempt to scrobble if the track has been playing long enough. According to Last.fm scrobbling guidelines, if a user has listened to enough of a track before stopping, it should still be scrobbled. Consider checking if the track qualifies for scrobbling before clearing state.
| console.error("Last.fm API 错误:", error); | ||
| throw error; |
There was a problem hiding this comment.
The error handling simply logs and re-throws the error without providing user-friendly error information. Last.fm API returns specific error codes and messages that should be parsed and presented to users in a meaningful way (e.g., invalid API key, rate limit exceeded, service unavailable).
|
我去，好吓人 |
|
/gemini review |
| const checkAuth = setInterval(async () => { | ||
| try { | ||
| // 尝试获取会话 | ||
| const sessionResponse = await getSession(token); | ||
|
|
||
| if (sessionResponse.session) { | ||
| clearInterval(checkAuth); | ||
| authWindow?.close(); | ||
|
|
||
| // 保存会话信息 | ||
| settingStore.lastfm.sessionKey = sessionResponse.session.key; | ||
| settingStore.lastfm.username = sessionResponse.session.name; | ||
|
|
||
| window.$message.success(`已成功连接到 Last.fm 账号: ${sessionResponse.session.name}`); | ||
| lastfmAuthLoading.value = false; | ||
| } | ||
| } catch (error) { | ||
| // 用户还未授权，继续等待 | ||
| } | ||
| }, 2000); |
There was a problem hiding this comment.
当前的轮询逻辑没有处理用户手动关闭授权窗口的情况。当用户关闭弹窗后，setInterval 仍会继续执行，直到30秒超时，这会产生不必要的API请求并可能导致不佳的用户体验。建议在轮询中增加对 authWindow.closed 的检查，以便在用户关闭窗口时立即停止轮询。
const checkAuth = setInterval(async () => {
if (authWindow?.closed) {
clearInterval(checkAuth);
if (lastfmAuthLoading.value) {
lastfmAuthLoading.value = false;
window.$message.warning("授权已取消");
}
return;
}
try {
// 尝试获取会话
const sessionResponse = await getSession(token);
if (sessionResponse.session) {
clearInterval(checkAuth);
authWindow?.close();
// 保存会话信息
settingStore.lastfm.sessionKey = sessionResponse.session.key;
settingStore.lastfm.username = sessionResponse.session.name;
window.$message.success(`已成功连接到 Last.fm 账号: ${sessionResponse.session.name}`);
lastfmAuthLoading.value = false;
}
} catch (error) {
// 用户还未授权，继续等待
}
}, 2000);
| lastfmScrobbler.startPlaying( | ||
| name || "", | ||
| artist || "", | ||
| album, | ||
| Math.floor(song.duration / 1000) ?? undefined, | ||
| ); |
There was a problem hiding this comment.
此处将歌曲时长传递给 lastfmScrobbler 的方式存在风险。如果 song.duration 因某些原因不是一个有效的数字（例如 undefined），Math.floor(song.duration / 1000) 会返回 NaN。?? undefined 操作符对 NaN 无效，因此会将 NaN 传递给 startPlaying 方法，这会导致 scrobbler 逻辑异常（例如立即上报）。建议在传递前进行更严格的检查，确保只传递有效的秒数或 undefined。
const durationInSeconds = song.duration > 0 ? Math.floor(song.duration / 1000) : undefined;
lastfmScrobbler.startPlaying(
name || "",
artist || "",
album,
durationInSeconds,
);| private scheduleScrobble() { | ||
| if (!this.currentTrack) return; | ||
|
|
||
| const duration = this.currentTrack.duration || 240; // 默认 4 分钟 |
There was a problem hiding this comment.
作为深度防御，scheduleScrobble 方法可以增强对 duration 的校验。当前 this.currentTrack.duration || 240 的写法在 this.currentTrack.duration 为 NaN 时，duration 也会是 NaN，这会导致 setTimeout 立即执行，从而错误地立即上报歌曲。建议修改此行以更稳健地处理无效的 duration 值。
| const duration = this.currentTrack.duration || 240; // 默认 4 分钟 | |
| const duration = this.currentTrack.duration > 0 ? this.currentTrack.duration : 240; // 默认 4 分钟 |
| const lastfmRequest = async ( | ||
| method: string, | ||
| params: Record<string, string | number> = {}, | ||
| needAuth: boolean = false, | ||
| ) => { | ||
| const { apiKey } = getApiConfig(); | ||
| const requestParams: Record<string, string | number> = { | ||
| method, | ||
| api_key: apiKey, | ||
| format: "json", | ||
| ...params, | ||
| }; | ||
|
|
||
| if (needAuth) { | ||
| requestParams.api_sig = generateSignature(requestParams); | ||
| } | ||
|
|
||
| try { | ||
| const response = await lastfmClient.get("", { params: requestParams }); | ||
| return response.data; | ||
| } catch (error) { | ||
| console.error("Last.fm API 错误:", error); | ||
| throw error; | ||
| } | ||
| }; | ||
|
|
||
| /** | ||
| * Last.fm API POST 请求（用于需要签名的写操作） | ||
| * @param method API 方法名 | ||
| * @param params 参数 | ||
| */ | ||
| const lastfmPostRequest = async (method: string, params: Record<string, string | number> = {}) => { | ||
| const { apiKey } = getApiConfig(); | ||
| const requestParams: Record<string, string | number> = { | ||
| method, | ||
| api_key: apiKey, | ||
| format: "json", | ||
| ...params, | ||
| }; | ||
|
|
||
| requestParams.api_sig = generateSignature(requestParams); | ||
|
|
||
| try { | ||
| const formData = new URLSearchParams(); | ||
| Object.entries(requestParams).forEach(([key, value]) => { | ||
| formData.append(key, String(value)); | ||
| }); | ||
|
|
||
| const response = await lastfmClient.post("", formData, { | ||
| headers: { | ||
| "Content-Type": "application/x-www-form-urlencoded", | ||
| }, | ||
| }); | ||
| return response.data; | ||
| } catch (error) { | ||
| console.error("Last.fm API POST 错误:", error); | ||
| throw error; | ||
| } | ||
| }; |
There was a problem hiding this comment.
lastfmRequest 和 lastfmPostRequest 方法中存在重复的代码逻辑，它们都包含了构建 requestParams 的过程。为了提高代码的可维护性并减少冗余，建议将这部分逻辑提取到一个独立的辅助函数中。
例如，你可以创建一个 prepareRequestParams 函数：
const prepareRequestParams = (method: string, params: Record<string, string | number> = {}) => {
const { apiKey } = getApiConfig();
const requestParams: Record<string, string | number> = {
method,
api_key: apiKey,
format: "json",
...params,
};
return requestParams;
};然后使用这个函数来重构 lastfmRequest 和 lastfmPostRequest。
| const settingStore = useSettingStore(); | ||
| if (settingStore.lastfm.scrobbleEnabled) { | ||
| const playedTime = (Date.now() - this.playStartTime) / 1000; | ||
| const duration = this.currentTrack.duration || 0; |
There was a problem hiding this comment.
与 scheduleScrobble 方法类似，此处对 duration 的处理也可以更健壮。当 this.currentTrack.duration 为 NaN 时，duration 也会是 NaN。虽然后续的 duration > 30 判断可以避免错误，但为了代码的一致性和健壮性，建议进行同样的修改。
| const duration = this.currentTrack.duration || 0; | |
| const duration = this.currentTrack.duration > 0 ? this.currentTrack.duration : 0; |
