fix: pass plugin name to StarTools.get_data_dir() to avoid inspect.stack() crash#193
Open
Rat0323 wants to merge 2 commits into
Open
fix: pass plugin name to StarTools.get_data_dir() to avoid inspect.stack() crash#193Rat0323 wants to merge 2 commits into
Rat0323 wants to merge 2 commits into
Conversation
Contributor
Reviewer's guide (collapsed on small PRs)Reviewer's GuidePass the explicit plugin name "astrbot_plugin_qq_group_daily_analysis" into all StarTools.get_data_dir() usages and remove the previous try/except-based fallback paths, standardizing data directory resolution for reports and debug artifacts. File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
Contributor
There was a problem hiding this comment.
Hey - I've found 1 issue, and left some high level feedback:
- The plugin name string "astrbot_plugin_qq_group_daily_analysis" is now duplicated across multiple call sites; consider centralizing it in a single constant (e.g., in a config/module-level variable) to avoid typos and ease future renames.
- Previously there was a filesystem-based fallback when StarTools.get_data_dir() raised; now any failure will bubble up—if you still expect non-inspection-related failures (e.g., StarTools unavailable), it may be worth retaining a narrower fallback around those specific error cases to preserve robustness.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The plugin name string "astrbot_plugin_qq_group_daily_analysis" is now duplicated across multiple call sites; consider centralizing it in a single constant (e.g., in a config/module-level variable) to avoid typos and ease future renames.
- Previously there was a filesystem-based fallback when StarTools.get_data_dir() raised; now any failure will bubble up—if you still expect non-inspection-related failures (e.g., StarTools unavailable), it may be worth retaining a narrower fallback around those specific error cases to preserve robustness.
## Individual Comments
### Comment 1
<location path="src/infrastructure/analysis/llm_analyzer.py" line_range="467" />
<code_context>
from astrbot.api.star import StarTools
- debug_dir = StarTools.get_data_dir() / "debug_data"
+ debug_dir = StarTools.get_data_dir("astrbot_plugin_qq_group_daily_analysis") / "debug_data"
debug_dir.mkdir(parents=True, exist_ok=True)
</code_context>
<issue_to_address>
**suggestion:** The plugin identifier string is duplicated in multiple places; consider centralizing it to avoid drift.
This literal plugin name is now embedded in multiple modules (config, init, HTML output, debug paths). Centralizing it in a shared constant or helper (e.g. `get_plugin_data_dir()` wrapping `StarTools.get_data_dir(PLUGIN_NAME)`) would make future renames or variant support safer and prevent subtle path mismatches.
Suggested implementation:
```python
from astrbot.api.star import StarTools
PLUGIN_NAME = "astrbot_plugin_qq_group_daily_analysis"
```
```python
debug_dir = StarTools.get_data_dir(PLUGIN_NAME) / "debug_data"
```
To fully centralize the plugin identifier and avoid drift across the codebase, you should also:
1. Identify other modules where `"astrbot_plugin_qq_group_daily_analysis"` is hardcoded (e.g., config, `__init__`, HTML output, other debug paths).
2. Extract `PLUGIN_NAME` (or a helper like `get_plugin_data_dir()`) into a shared module (for example, `src/infrastructure/analysis/constants.py` or a plugin-level `config.py`).
3. Replace the literals in those modules to import and use the shared `PLUGIN_NAME` or `get_plugin_data_dir()` instead of repeating the string.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
4 tasks
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.
Problem
When
StarTools.get_data_dir()is called without arguments, the framework attempts to deduce the calling plugin name usinginspect.stack(). Under certain environments (such as dynamic reloading, testing shims, or direct scripts where__main__is the entrypoint), metadata extraction fails, throwing a fatalRuntimeError: 无法获取模块 __main__ 的元信息.Solution
Explicitly pass the plugin name
"astrbot_plugin_qq_group_daily_analysis"to all calls ofStarTools.get_data_dir()to avoid this dynamic inspection crash. This is safe, backwards-compatible, and robust across all execution environments.Verified and tested successfully under AstrBot official environment.
Summary by Sourcery
Ensure the plugin always uses a stable, explicit data directory for storage and HTML/debug outputs by passing the plugin name into StarTools.get_data_dir() instead of relying on runtime inspection and fallback paths.
Bug Fixes:
Enhancements:
💡 Ecosystem & Framework Context
This issue stems from a known limitation in the AstrBot core framework's path-resolving API. We have submitted a core robustness fix to the official AstrBot core repository: AstrBot Core PR #8588.
However, this plugin-level change (passing the plugin name explicitly) remains highly necessary to ensure backward compatibility. It prevents the plugin from crashing on older installations of AstrBot that have not upgraded to the latest core version.
This is a safe, non-breaking, and recommended change.