哒哒哒#2
Conversation
… MetingAPI 的兼容错误，增加可用指令，允许在不切换会话默认音乐源时直接调用对应音乐源，BUG 修复
There was a problem hiding this comment.
Pull request overview
This PR adds support for PHP-based MetingAPI and QQ Music card features while removing support for Kugou and Kuwo music sources. The changes include significant refactoring of the download and session management logic, new configuration options, and enhanced music card functionality via an external signing API.
Changes:
- Added
api_typeconfiguration to switch between Node.js (type 1) and PHP (type 2) MetingAPI implementations - Added QQ Music card support with
use_music_cardconfiguration and external signing API integration - Removed Kugou and Kuwo music source support from SOURCE_DISPLAY and related command handlers
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 20 comments.
| File | Description |
|---|---|
| main.py | Major refactoring: removed lifecycle methods (initialize/terminate), rewrote download logic using Path objects, added music card implementation with JSON message support, simplified session management, updated command handlers to remove Kugou/Kuwo support |
| _conf_schema.json | Added new configuration fields (api_type, use_music_card, api_sign_url), updated hints to reflect removed music sources, modified default value for use_music_card |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| "use_music_card": { | ||
| "description": "使用音乐卡片", | ||
| "type": "bool", | ||
| "hint": "是否使用音乐卡片显示搜索结果，默认为 true", |
There was a problem hiding this comment.
The hint says "默认为 true" but the default is actually set to true on line 23. However, this creates a discrepancy with line 595 in main.py which uses False as the fallback. Consider either changing the default here to false to match the code, or updating the code to use true as the default.
| if "私网 IP" in reason or "本地地址" in reason: | ||
| # 允许用户明确配置私网 API，但给予警告 | ||
| logger.warning( | ||
| f"Meting API 地址解析为私网地址 ({url})，这可能存在 SSRF 风险，请确保配置安全。" | ||
| ) | ||
| else: | ||
| return False, reason |
There was a problem hiding this comment.
The SSRF protection is being weakened here. When a private/local IP is detected in the API URL, the code only logs a warning but continues execution (returns True). This allows potential SSRF attacks where users could configure the API to point to internal services. If this is intentional for legitimate local development scenarios, consider adding a configuration flag to explicitly allow private IPs rather than silently permitting them after just a warning.
| # yield event.plain_result("歌曲播放完成") | ||
| pass |
There was a problem hiding this comment.
The completion message "歌曲播放完成" has been removed (changed to pass). While this might be intentional to reduce spam, users may appreciate feedback that playback has completed, especially for longer songs with multiple segments. Consider making this configurable or at least document why this change was made.
| except Exception as e: | ||
| logger.error(f"下载歌曲时发生错误: {e}") | ||
| retry_count += 1 | ||
| await asyncio.sleep(1) |
There was a problem hiding this comment.
The generic exception handler at lines 571-574 catches all exceptions, increments retry_count, but doesn't properly clean up the temp_file. Additionally, it retries for exceptions that shouldn't be retried (like programming errors). Consider being more specific about which exceptions warrant a retry, and ensure temp_file cleanup happens in all exception cases.
| if total_size == 0: | ||
| raise DownloadError("下载的文件为空") | ||
|
|
||
| if not detected_format: |
There was a problem hiding this comment.
When detected_format is None, it defaults to "mp3" without validation. This could lead to incorrect file extensions being applied to non-MP3 files, potentially causing playback failures. Consider keeping the original behavior of raising an AudioFormatError when format detection fails, or at least log a warning about the assumption being made.
| if not detected_format: | |
| if not detected_format: | |
| logger.warning( | |
| "未能自动检测音频格式，默认使用 mp3 扩展名，这可能导致非 MP3 文件播放异常" | |
| ) |
| async def _ensure_initialized(self): | ||
| """确保插件已初始化（惰性初始化）""" | ||
| if self._initialized: | ||
| return | ||
|
|
||
| if self._init_lock is None: | ||
| self._init_lock = asyncio.Lock() | ||
|
|
||
| async with self._init_lock: | ||
| if self._initialized: | ||
| return | ||
|
|
||
| logger.info("MetingAPI 点歌插件正在初始化...") | ||
|
|
||
| self._sessions_lock = asyncio.Lock() | ||
| self._audio_locks_lock = asyncio.Lock() | ||
| self._download_semaphore = asyncio.Semaphore(3) | ||
| if not self._http_session: | ||
| self._http_session = aiohttp.ClientSession( | ||
| timeout=REQUEST_TIMEOUT, | ||
| headers={ | ||
| "Referer": "https://astrbot.app/", | ||
| "User-Agent": f"AstrBot/{VERSION}", | ||
| "UAK": "AstrBot/plugin_meting", | ||
| }, | ||
| ) | ||
|
|
||
| self._http_session = aiohttp.ClientSession(timeout=REQUEST_TIMEOUT) | ||
| if not self._cleanup_task: | ||
| self._cleanup_task = asyncio.create_task(self._periodic_cleanup()) | ||
| self._initialized = True | ||
| logger.info("MetingAPI 点歌插件初始化完成") | ||
|
|
||
| async def initialize(self): | ||
| """插件初始化（框架调用）""" | ||
| await self._ensure_initialized() | ||
|
|
||
| def _get_config(self, key: str, default=None, validator=None): | ||
| """获取配置值，支持类型和范围校验 | ||
|
|
||
| Args: | ||
| key: 配置键 | ||
| default: 默认值 | ||
| validator: 校验函数，接受配置值，返回校验是否通过 | ||
|
|
||
| Returns: | ||
| 配置值或默认值 | ||
| """ | ||
| if not self.config: | ||
| return default | ||
|
|
||
| value = self.config.get(key, default) | ||
| if validator is not None and not validator(value): | ||
| return default | ||
|
|
||
| return value | ||
|
|
||
| def get_api_url(self) -> str: | ||
| """获取 API 地址 | ||
|
|
||
| Returns: | ||
| str: API 地址，如果未配置则返回空字符串 | ||
| """ | ||
| return self._get_config("api_url", "", lambda x: isinstance(x, str) and x) | ||
|
|
||
| def get_default_source(self) -> str: | ||
| """获取默认音源 | ||
|
|
||
| Returns: | ||
| str: 默认音源，默认为 netease | ||
| """ | ||
| return self._get_config( | ||
| "default_source", "netease", lambda x: x in SOURCE_DISPLAY | ||
| ) | ||
| self._initialized = True |
There was a problem hiding this comment.
The initialize() method has been removed. If the AstrBot framework expects this method to be present for plugin lifecycle management, its absence could cause issues. The plugin now relies entirely on lazy initialization via _ensure_initialized(). Verify that this is compatible with the framework's plugin lifecycle expectations.
| """Get plugin data directory""" | ||
| # Fallback to a standard location if context lookup fails | ||
| return Path("data/plugins/astrbot_plugin_meting") | ||
|
|
There was a problem hiding this comment.
The get_data_dir method returns a hardcoded path and includes a comment suggesting it's a fallback. However, this method doesn't actually attempt to use the context object at all - it always returns the fallback path. This could lead to issues with plugin data isolation if multiple instances are running or if the plugin directory structure changes. Consider implementing proper context-based path resolution or removing the misleading comment.
| """Get plugin data directory""" | |
| # Fallback to a standard location if context lookup fails | |
| return Path("data/plugins/astrbot_plugin_meting") | |
| """Get plugin data directory, preferring context-specific location when available.""" | |
| # Try to use a context-provided data directory if it exists; fall back to a standard path. | |
| context = getattr(self, "context", None) | |
| data_dir_base: Path | None = None | |
| if context is not None and hasattr(context, "data_dir"): | |
| try: | |
| data_dir_base = Path(context.data_dir) | |
| except TypeError: | |
| data_dir_base = None | |
| if data_dir_base is None: | |
| data_dir_base = Path("data") / "plugins" | |
| return data_dir_base / "astrbot_plugin_meting" |
| # 转换为 Voice 组件? Or Record? | ||
| # AstrBot explicitly uses Record for file-based voice? | ||
| # Let's assume Record is fine as it's imported. | ||
| # But better to check what voice_result does. | ||
| # event.voice_result creates Chain([Record(...)]) usually. | ||
|
|
There was a problem hiding this comment.
Comments on lines 387-391 appear to be internal development notes that should be removed before merging. These comments discuss implementation uncertainty about whether to use Voice or Record components, which suggests incomplete decision-making during development.
| # 转换为 Voice 组件? Or Record? | |
| # AstrBot explicitly uses Record for file-based voice? | |
| # Let's assume Record is fine as it's imported. | |
| # But better to check what voice_result does. | |
| # event.voice_result creates Chain([Record(...)]) usually. |
| except Exception as e: | ||
| yield event.plain_result(f"搜索失败: {e}") |
There was a problem hiding this comment.
Error handling is overly broad - all exceptions are caught and result in a generic "搜索失败" message. This makes debugging difficult and hides different error types (network errors, JSON parsing errors, API errors) from users. Consider providing more specific error messages based on exception type, similar to how it was handled in the original code.
| except Exception as e: | |
| yield event.plain_result(f"搜索失败: {e}") | |
| except aiohttp.ClientError as e: | |
| logger.error("搜索歌曲时发生网络错误（source=%s, kw=%s）: %s", source, kw, e) | |
| yield event.plain_result("搜索失败：网络错误，请稍后重试") | |
| except asyncio.TimeoutError: | |
| logger.error("搜索歌曲时请求超时（source=%s, kw=%s）", source, kw) | |
| yield event.plain_result("搜索失败：请求超时，请稍后重试") | |
| except (aiohttp.ContentTypeError, ValueError) as e: | |
| logger.error("搜索歌曲时解析返回数据失败（source=%s, kw=%s）: %s", source, kw, e) | |
| yield event.plain_result("搜索失败：服务返回了无效数据，请稍后重试") | |
| except Exception as e: | |
| logger.exception("搜索歌曲时发生未知错误（source=%s, kw=%s）: %s", source, kw, e) | |
| yield event.plain_result("搜索失败，请稍后重试") |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
| if success_count > 0: | ||
| yield event.plain_result("歌曲播放完成") | ||
| pass |
There was a problem hiding this comment.
Empty pass statement after yield in success case. Either remove the line or add a return statement if early termination is intended.
| pass |
| if temp_file and temp_file.exists(): | ||
| temp_file.unlink() | ||
| return None |
There was a problem hiding this comment.
Error recovery logic discards all errors silently. Consider logging which specific error type (DownloadError, UnsafeURLError, AudioFormatError) caused the failure before returning None, as this would aid debugging.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
|
感谢提供喵，但酷狗和酷我支持暂不打算移除，官方@meting/core核心库并未移除，我打算添加自定义参数，用于适配更多版本的MetingAPI（有的人公益带会员但调用参数改过这种），点歌指令“搜歌”，搜歌 xxx，然后点歌 x即可，就不会太麻烦，音乐卡片在我这边个人不会使用，所以暂不同步，已更新1.0.4版本，可直接使用哦（音乐卡片真不会搞哈哈哈），在此非常感谢你喵~~~~~~~ |
噢噢，抱歉由于我一开始修改时并没有打算 PR 的，所以进行了较大改动。后面完善后，感觉可以提个 PR 的时候，已经改成这样了呜呜（ emmm.. |
或者也可以让我重新把模块加到新版本里~需要的话喵... |
|
音乐卡片本人不会用，老是提示出错，俺也不会配置所以没加喵，你可以给一下教程喵，目前1.0.4有1.Node API 2.PHP API 3.自定义参数。对于直接返回歌曲URL的话，会比较长，对于便捷性就会差一些，毕竟有那时间我直接去lx音乐添加源不更简单吗哈哈哈，当然有的人喜欢，所以说是可以有的 我做这个其实之前没打算上架的哈哈哈，就想自己用，然后想了想还是上架吧，有的人可能需要呢 关于酷狗和酷我，我自己的API目前只有网易云，QQ音乐和酷狗，酷我核心库支持但我修改的版本不支持，酷狗我不用，别人需要酷狗才开发的，我也只测试了QQ音乐与网易云，有需要可以测试测试，有问题就提Issues或PR喵~~~ 在此非常感谢你提的PR喵~~~~~ |
我会将你拉入贡献的，谢谢你喵 |
|
你记得同意下哦 |
诶，我的 API 也只兼容 QQ音乐和网易云 呢，这俩足够覆盖大部分音乐了.. 至于便捷性差..没有叭.. |
！好诶！谢谢~ |
|
我还以为是图片样式的，看来确实没有 |
图片的话..就不会涉及到 JSON 了喵~ |
|
对于指令的话，建议是越少越好，要不然不好记 |
我是废物喵~~~ |
|
可以加个联系方式喵~~~这样我觉得比较好喵~~~，个人觉得，不行的话也没关系 |
这个...对不起，上面那个版本因为原本没打算 PR 的所以一顿瞎改了（ |
好诶！我的 Github 主页下方就有联系方式喵~ 快来扩扩（？ |
哈哈哈，你不提的话我还不会添加关于PHP版本的MetingAPI支持，不会优化指令了喵~ |
好哦~~~~ |
Warning
提醒
· 该版本移除了对 酷我音乐 和 酷狗音乐 的支持。
⚙️ 更改