Repository navigation
V1.4.0 wss连接qq服务器 - #9
Merged
Merged
Conversation
Contributor
Reviewer's GuideIntroduce a new HermesWss module to connect to QQ via WebSocket (wss), including a typed WebSocket client, REST sender, chunked upload, QR-based onboarding, and keyboard/interaction models; wire in new runtime dependencies and mypy strict settings for this module. Sequence diagram for QQ WebSocket message handling and replysequenceDiagram
participant QQGateway
participant QQWebSocketClient
participant OnMessageHandler as on_message_callback
participant QQMessageSender
QQGateway->>QQWebSocketClient: WebSocket DISPATCH C2C_MESSAGE_CREATE
QQWebSocketClient->>QQWebSocketClient: _parse_payload
QQWebSocketClient->>QQWebSocketClient: _on_message(event_type, data)
QQWebSocketClient->>OnMessageHandler: on_message(QQMessage)
activate OnMessageHandler
OnMessageHandler->>QQMessageSender: send_text_to_c2c(openid, content, reply_to, keyboard)
activate QQMessageSender
QQMessageSender->>TokenManager: get_token()
TokenManager-->>QQMessageSender: access_token
QQMessageSender->>QQGateway: POST /v2/users/{openid}/messages
QQGateway-->>QQMessageSender: SendResult
deactivate QQMessageSender
deactivate OnMessageHandler
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
📝 WalkthroughWalkthroughAdds a complete ChangesHermesWss package contracts
Messaging and interaction support
WebSocket gateway client
Large-file transfer
Estimated code review effort: 5 (Critical) | ~120 minutes Sequence Diagram(s)sequenceDiagram
participant Application
participant QQWebSocketClient
participant QQGateway
participant OnMessageCallback
Application->>QQWebSocketClient: start()
QQWebSocketClient->>QQGateway: obtain gateway URL and connect
QQWebSocketClient->>QQGateway: Identify or Resume
QQGateway-->>QQWebSocketClient: Hello and Dispatch events
QQWebSocketClient->>QQWebSocketClient: send heartbeats and deduplicate messages
QQWebSocketClient->>OnMessageCallback: deliver QQMessage
QQWebSocketClient->>QQGateway: reconnect or resume after disconnect
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Hey - I've found 7 issues, and left some high level feedback:
- In
wss_client.pyyou re‑implement token fetching logic (_ensure_token) instead of reusing the sharedTokenManager, and also keep an unusedTokenManagerimport—consider consolidating all access token handling throughTokenManagerto avoid duplicated logic and drift. - In
chunked_upload.pythe file is fully read into memory and all parts are uploaded concurrently without respecting theconcurrent_partslimit (you computeconcurrentbut never use it); it would be safer to stream the file in chunks and enforce concurrency with a semaphore or bounded worker pool to avoid high memory usage and uncontrolled parallelism for large files. - There are now two separate HTTP stacks (aiohttp in
wss_clientand httpx insender/chunked_upload), each with its own timeout and header construction; if possible, consider unifying on a single client abstraction so gateway and REST calls share configuration and behavior (proxies, retries, logging) and you don't have to maintain two parallel implementations.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In `wss_client.py` you re‑implement token fetching logic (`_ensure_token`) instead of reusing the shared `TokenManager`, and also keep an unused `TokenManager` import—consider consolidating all access token handling through `TokenManager` to avoid duplicated logic and drift.
- In `chunked_upload.py` the file is fully read into memory and all parts are uploaded concurrently without respecting the `concurrent_parts` limit (you compute `concurrent` but never use it); it would be safer to stream the file in chunks and enforce concurrency with a semaphore or bounded worker pool to avoid high memory usage and uncontrolled parallelism for large files.
- There are now two separate HTTP stacks (aiohttp in `wss_client` and httpx in `sender`/`chunked_upload`), each with its own timeout and header construction; if possible, consider unifying on a single client abstraction so gateway and REST calls share configuration and behavior (proxies, retries, logging) and you don't have to maintain two parallel implementations.
## Individual Comments
### Comment 1
<location path="modules/HermesWss/chunked_upload.py" line_range="157-158" />
<code_context>
+ )
+
+ # Step 2: 上传每个分片
+ concurrent: int = min(concurrent_parts, _MAX_CONCURRENT_PARTS)
+ if concurrent > 1:
+ await self._upload_parts_concurrent(parts, upload_urls)
+ else:
</code_context>
<issue_to_address>
**issue (bug_risk):** The `concurrent_parts` setting is computed but never actually used to limit concurrency.
`upload_file` computes `concurrent` from `concurrent_parts` and `_MAX_CONCURRENT_PARTS`, but `_upload_parts_concurrent` ignores it and creates one task per part, so concurrency is effectively unbounded for large uploads. Please pass `concurrent` into `_upload_parts_concurrent` and enforce it there (e.g., via a semaphore or fixed-size worker pool) to actually cap the number of simultaneous PUT requests.
</issue_to_address>
### Comment 2
<location path="modules/HermesWss/wss_client.py" line_range="822-825" />
<code_context>
+ scene_map: dict[int, str] = {0: "guild", 1: "group", 2: "c2c"}
+ scene: str = scene_map.get(scene_code, "")
+
+ button_data_obj: object = resolved.get("button_data", "") if isinstance(resolved, dict) else ""
+ if isinstance(resolved, dict):
+ bd: object = resolved.get("button_data", "")
+ button_data_str: str = str(bd) if bd is not None else ""
+ else:
+ button_data_str = ""
</code_context>
<issue_to_address>
**suggestion:** Redundant and partially unused variables in `_on_interaction` make the button data parsing harder to follow and more error-prone.
In `_on_interaction`, `button_data_obj` and the first `resolved.get("button_data", ...)` are unused; the effective value is taken only from `bd` in the second branch. This duplication obscures the control flow and makes future changes riskier. Consider a single branch that sets `button_data_str` from `resolved` when it’s a dict, otherwise `""`, and apply the same simplification pattern to the `button_id` logic.
</issue_to_address>
### Comment 3
<location path="modules/HermesWss/models.py" line_range="401-403" />
<code_context>
+# ======================================================================
+
+# POST 消息的函数签名（适配器注入用）
+PostMessageFn = Callable[
+ [str, str, str | None, InlineKeyboard | None],
+ Coroutine["asyncio.AbstractEventLoop", object, dict[str, object]],
+]
+
</code_context>
<issue_to_address>
**suggestion:** The `PostMessageFn` Coroutine typing is likely incorrect and could confuse type checkers.
`Coroutine`’s type parameters are `(yield_type, send_type, return_type)`, not an event loop. Annotating it as `Coroutine[asyncio.AbstractEventLoop, object, dict[str, object]]` is misleading for an async function that just returns `dict[str, object]`. Consider changing this to:
```python
PostMessageFn = Callable[
[str, str, str | None, InlineKeyboard | None],
Coroutine[Any, Any, dict[str, object]],
]
```
or simply `Awaitable[dict[str, object]]` if you don’t need the full `Coroutine` signature. This will align better with type checkers like mypy.
Suggested implementation:
```python
from typing import Awaitable, Callable, Coroutine
```
```python
# POST 消息的函数签名（适配器注入用）
PostMessageFn = Callable[
[str, str, str | None, InlineKeyboard | None],
Awaitable[dict[str, object]],
]
```
</issue_to_address>
### Comment 4
<location path="modules/HermesWss/token_manager.py" line_range="70-73" />
<code_context>
+ 或任何其他导致 token 刷新失败的情况时抛出。
+ """
+ # 快速路径：缓存仍在有效期内（预留 60 秒缓冲期）
+ if time.time() < self._token_expires_at - 60:
+ assert self._access_token is not None
+ return self._access_token
+
</code_context>
<issue_to_address>
**suggestion (bug_risk):** Using `assert` for the cached-token path can raise `AssertionError` instead of a domain-specific error.
Here, if the cached-token invariants are ever violated (e.g., due to future refactors or concurrency), callers will get a bare `AssertionError` instead of `QQTokenError`. Consider replacing the assert with an explicit `if self._access_token is None:` branch that either raises `QQTokenError` or triggers a refresh, to keep error handling consistent with the rest of `get_token`.
```suggestion
# 快速路径：缓存仍在有效期内（预留 60 秒缓冲期）
if time.time() < self._token_expires_at - 60:
if self._access_token is None:
# 理论上在过期时间未到的情况下，_access_token 不应为 None
# 如果发生这种情况，说明内部状态被破坏，统一抛出 QQTokenError
raise QQTokenError("缓存的 access token 缺失，但过期时间仍然有效")
return self._access_token
```
</issue_to_address>
### Comment 5
<location path="modules/HermesWss/chunked_upload.py" line_range="405" />
<code_context>
+ Returns:
+ 十六进制 md5 字符串。
+ """
+ hasher: hashlib._Hash = hashlib.md5()
+ with file_path.open("rb") as f:
+ chunk: bytes = f.read(_MD5_10M_SIZE)
</code_context>
<issue_to_address>
**nitpick:** Using the private `hashlib._Hash` type in annotations is brittle.
Here `hasher` is annotated as `hashlib._Hash`, which is an internal type and can change across Python versions. Since this variable is only local, consider avoiding the private type by either dropping the annotation, using `hashlib.Hash` if supported, or falling back to a more generic annotation (e.g. `Any` or a union of public/private types).
</issue_to_address>
### Comment 6
<location path="modules/HermesWss/sender.py" line_range="527" />
<code_context>
+ last_error = str(exc)
+
+ # 永久性错误不重试
+ if any(k in error_str for k in ("invalid", "forbidden", "not found", "bad request")):
+ return SendResult(success=False, error=str(exc), retryable=False)
+
</code_context>
<issue_to_address>
**suggestion (bug_risk):** Classifying permanent vs retryable errors by substring search on the message text is fragile.
In `_send_with_retry`, retryability is decided by searching for substrings like "invalid"/"forbidden" in the error string. This is brittle: small changes in wording, localization, or transient errors that happen to contain those words can lead to misclassification. Prefer branching on structured signals (e.g., `QQApiError.status_code` ranges such as 4xx vs 5xx, or specific `QQTokenError` types), and only fall back to message heuristics when no structured data is available.
</issue_to_address>
### Comment 7
<location path="modules/HermesWss/README.md" line_range="12" />
<code_context>
+
+| 功能 | 说明 |
+|------|------|
+| **WebSocket 长连接** | 替代回调，无需公网暴露端口、配 SSL、验证回调 URL |
+| **自动重连** | 带退避策略，覆盖 QQ 所有关闭码（4004/4006/4008/4009/4914/4915） |
+| **心跳保活** | 根据 Hello 事件动态调整心跳间隔 |
</code_context>
<issue_to_address>
**suggestion (typo):** Consider changing “配 SSL” to the more standard written form “配置 SSL”.
Using the full phrase here avoids spoken shorthand and makes the table wording more formal and polished.
```suggestion
| **WebSocket 长连接** | 替代回调，无需公网暴露端口、配置 SSL、验证回调 URL |
```
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Comment on lines
+822
to
+825
| button_data_obj: object = resolved.get("button_data", "") if isinstance(resolved, dict) else "" | ||
| if isinstance(resolved, dict): | ||
| bd: object = resolved.get("button_data", "") | ||
| button_data_str: str = str(bd) if bd is not None else "" |
Contributor
There was a problem hiding this comment.
suggestion: Redundant and partially unused variables in _on_interaction make the button data parsing harder to follow and more error-prone.
In _on_interaction, button_data_obj and the first resolved.get("button_data", ...) are unused; the effective value is taken only from bd in the second branch. This duplication obscures the control flow and makes future changes riskier. Consider a single branch that sets button_data_str from resolved when it’s a dict, otherwise "", and apply the same simplification pattern to the button_id logic.
Comment on lines
+401
to
+403
| PostMessageFn = Callable[ | ||
| [str, str, str | None, InlineKeyboard | None], | ||
| Coroutine["asyncio.AbstractEventLoop", object, dict[str, object]], |
Contributor
There was a problem hiding this comment.
suggestion: The PostMessageFn Coroutine typing is likely incorrect and could confuse type checkers.
Coroutine’s type parameters are (yield_type, send_type, return_type), not an event loop. Annotating it as Coroutine[asyncio.AbstractEventLoop, object, dict[str, object]] is misleading for an async function that just returns dict[str, object]. Consider changing this to:
PostMessageFn = Callable[
[str, str, str | None, InlineKeyboard | None],
Coroutine[Any, Any, dict[str, object]],
]or simply Awaitable[dict[str, object]] if you don’t need the full Coroutine signature. This will align better with type checkers like mypy.
Suggested implementation:
from typing import Awaitable, Callable, Coroutine# POST 消息的函数签名（适配器注入用）
PostMessageFn = Callable[
[str, str, str | None, InlineKeyboard | None],
Awaitable[dict[str, object]],
]
Comment on lines
+70
to
+73
| # 快速路径：缓存仍在有效期内（预留 60 秒缓冲期） | ||
| if time.time() < self._token_expires_at - 60: | ||
| assert self._access_token is not None | ||
| return self._access_token |
Contributor
There was a problem hiding this comment.
suggestion (bug_risk): Using assert for the cached-token path can raise AssertionError instead of a domain-specific error.
Here, if the cached-token invariants are ever violated (e.g., due to future refactors or concurrency), callers will get a bare AssertionError instead of QQTokenError. Consider replacing the assert with an explicit if self._access_token is None: branch that either raises QQTokenError or triggers a refresh, to keep error handling consistent with the rest of get_token.
Suggested change
| # 快速路径：缓存仍在有效期内（预留 60 秒缓冲期） | |
| if time.time() < self._token_expires_at - 60: | |
| assert self._access_token is not None | |
| return self._access_token | |
| # 快速路径：缓存仍在有效期内（预留 60 秒缓冲期） | |
| if time.time() < self._token_expires_at - 60: | |
| if self._access_token is None: | |
| # 理论上在过期时间未到的情况下，_access_token 不应为 None | |
| # 如果发生这种情况，说明内部状态被破坏，统一抛出 QQTokenError | |
| raise QQTokenError("缓存的 access token 缺失，但过期时间仍然有效") | |
| return self._access_token |
| Returns: | ||
| 十六进制 md5 字符串。 | ||
| """ | ||
| hasher: hashlib._Hash = hashlib.md5() |
Contributor
There was a problem hiding this comment.
nitpick: Using the private hashlib._Hash type in annotations is brittle.
Here hasher is annotated as hashlib._Hash, which is an internal type and can change across Python versions. Since this variable is only local, consider avoiding the private type by either dropping the annotation, using hashlib.Hash if supported, or falling back to a more generic annotation (e.g. Any or a union of public/private types).
| last_error = str(exc) | ||
|
|
||
| # 永久性错误不重试 | ||
| if any(k in error_str for k in ("invalid", "forbidden", "not found", "bad request")): |
Contributor
There was a problem hiding this comment.
suggestion (bug_risk): Classifying permanent vs retryable errors by substring search on the message text is fragile.
In _send_with_retry, retryability is decided by searching for substrings like "invalid"/"forbidden" in the error string. This is brittle: small changes in wording, localization, or transient errors that happen to contain those words can lead to misclassification. Prefer branching on structured signals (e.g., QQApiError.status_code ranges such as 4xx vs 5xx, or specific QQTokenError types), and only fall back to message heuristics when no structured data is available.
|
|
||
| | 功能 | 说明 | | ||
| |------|------| | ||
| | **WebSocket 长连接** | 替代回调，无需公网暴露端口、配 SSL、验证回调 URL | |
Contributor
There was a problem hiding this comment.
suggestion (typo): Consider changing “配 SSL” to the more standard written form “配置 SSL”.
Using the full phrase here avoids spoken shorthand and makes the table wording more formal and polished.
Suggested change
| | **WebSocket 长连接** | 替代回调，无需公网暴露端口、配 SSL、验证回调 URL | | |
| | **WebSocket 长连接** | 替代回调，无需公网暴露端口、配置 SSL、验证回调 URL | |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 16
🧹 Nitpick comments (1)
modules/HermesWss/models.py (1)
400-404: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse
Awaitablefor these callback aliases.PostMessageFn,OnMessageCallback, andOnInteractionCallbackonly need to promise an awaitable result;Coroutine["asyncio.AbstractEventLoop", object, ...]over-specifies the contract and uses the wrong coroutine type parameters. Replace them withAwaitable[...]and drop theasyncioimport if it becomes unused.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@modules/HermesWss/models.py` around lines 400 - 404, Update the callback type aliases PostMessageFn, OnMessageCallback, and OnInteractionCallback to return Awaitable[...] instead of Coroutine["asyncio.AbstractEventLoop", object, ...], preserving each existing result type. Remove the asyncio import if it is no longer used.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@modules/HermesWss/chunked_upload.py`:
- Around line 156-161: Update the upload flow around _upload_parts_concurrent to
pass the normalized concurrent limit instead of discarding it. Change the
concurrent implementation to create an asyncio.Semaphore with that limit and
acquire it around each PUT operation, while preserving sequential behavior when
the limit is one and the maximum of ten.
- Around line 120-133: In the upload method surrounding _compute_md5_10m, define
or reuse the protocol’s authoritative maximum file-size constant and compare
file_size immediately after file_path_obj.stat().st_size is obtained. Raise
UploadFileTooLargeError for oversized files before computing the hash or reading
file contents, while preserving the existing not-found handling.
- Around line 230-249: Update the response parsing and UploadPrepareResponse
construction to reject incomplete successful API responses: require a non-empty
upload_id, a positive block_size, and valid non-empty file_info wherever that
field is parsed. Treat missing, zero, or invalid values as an error before
returning or starting chunk iteration, and catch malformed response JSON so it
is converted to the module’s established API/error type rather than escaping
from response.json().
- Around line 66-86: Update the chunked uploader class around __init__ to track
whether it created the httpx.AsyncClient internally, then add an async aclose()
method that closes only internally owned clients. Provide async context-manager
support via __aenter__ and __aexit__ that returns the uploader and invokes
aclose(), while leaving caller-provided clients open.
- Around line 216-228: Preserve UploadDailyLimitExceededError raised in the
response JSON handling within the non-200 branch by re-raising it before the
broad exception handler, or by narrowing the handler to JSON parsing failures
only. Keep generic UploadError behavior for malformed JSON and other non-limit
responses, while retaining the existing _BIZ_CODE_DAILY_LIMIT check.
- Around line 102-129: Validate chat_type at the start of upload_file, allowing
only "c2c" or "group" and raising UploadError for any other value before
checking the file or selecting an endpoint. Preserve the existing endpoint
behavior for valid chat types.
In `@modules/HermesWss/constants.py`:
- Line 113: Update the close-code classification in constants.py so both 4006
and the existing INVALID_SEQ value 4007 are treated as session-invalid and
trigger session clearing. Add the missing 4006 constant and include both codes
in the same invalid-session set or mapping used by the surrounding definitions,
preserving existing handling for other close codes.
In `@modules/HermesWss/keyboards.py`:
- Around line 286-335: Update modules/HermesWss/keyboards.py lines 286-335 to
centralize interaction parsing around raw["data"]["resolved"], extracting button
and resolver fields from that payload while preserving the documented fallbacks.
Update modules/HermesWss/wss_client.py lines 809-850 to remove the duplicate
field extraction and reuse the corrected parse_interaction_event(data) helper;
both sites must produce complete, consistent interaction events.
In `@modules/HermesWss/onboard.py`:
- Around line 120-130: Wrap the resp.json() call in the onboarding task-creation
flow with the existing QQBindError handling so JSON decoder failures become the
documented onboarding error instead of escaping directly. Apply equivalent
handling to _poll_bind_result, while preserving the existing response-shape and
task_id validation behavior for successfully parsed JSON.
- Around line 159-167: Update qr_register’s workflow to enforce its deadline
across all network requests and polling sleeps: use the current remaining
duration when configuring each httpx.Client/request, and sleep for
min(ONBOARD_POLL_INTERVAL, remaining). Apply the same remaining-time handling to
the polling and subsequent request blocks identified in the diff, while
preserving the existing EXPIRED return when the deadline is reached.
In `@modules/HermesWss/sender.py`:
- Around line 47-67: Update QQMessageSender.__init__ to track whether it owns
the internally created AsyncClient, add async aclose() and async context-manager
support that closes only owned clients, and update modules/HermesWss/README.md
lines 50-59 to create one sender outside the callback and close it during
application shutdown instead of instantiating one per message.
- Around line 512-536: Update _send_with_retry to avoid retrying ambiguous
transport failures, especially for POST requests where the server may have
accepted the message before the connection dropped. Preserve and reuse a stable
request/message identifier across attempts if the API supports idempotency;
otherwise return a non-retryable SendResult for converted httpx.RequestError
cases while retaining retries for safely retryable API errors.
- Around line 447-459: The approval flow in the surrounding sender method must
use a payload compatible with the selected send API and propagate its result.
Update the calls to send_text_to_c2c/send_text_to_group (or switch to the
Markdown-capable path) so build_approval_text content renders correctly, capture
the returned SendResult, and return its success value instead of always
returning True; preserve the existing logging and routing behavior.
In `@modules/HermesWss/wss_client.py`:
- Around line 739-747: The message deduplication logic in the `_dedup_lock`
block must evict entries by age rather than arbitrary set order. Replace
`_seen_msg_ids` with an insertion-ordered mapping of message IDs to timestamps,
record each new ID’s arrival time, and remove oldest entries whose age exceeds
`DEDUP_WINDOW_SECONDS`; preserve duplicate detection and locking behavior.
- Around line 301-317: Update the connection flow around ws_connect in the
WebSocket session initialization to apply CONNECT_TIMEOUT_SECONDS to the
handshake, using the aiohttp session ClientTimeout or an asyncio.timeout
wrapper. Do not rely on ClientWSTimeout(ws_close=...), which should remain
limited to close behavior.
- Around line 133-137: Update the aiohttp.ClientSession creation in the
_http_client initialization path to enable trust_env=True, ensuring token and
gateway HTTP requests honor HTTPS_PROXY and related environment proxy settings
while preserving the existing timeout configuration.
---
Nitpick comments:
In `@modules/HermesWss/models.py`:
- Around line 400-404: Update the callback type aliases PostMessageFn,
OnMessageCallback, and OnInteractionCallback to return Awaitable[...] instead of
Coroutine["asyncio.AbstractEventLoop", object, ...], preserving each existing
result type. Remove the asyncio import if it is no longer used.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 836a606e-5bf9-4990-85c9-282495b500f0
⛔ Files ignored due to path filters (1)
uv.lockis excluded by!**/*.lock
📒 Files selected for processing (63)
.github/workflows/black-format.yml.github/workflows/codeql.yml.gitignore.python-version.sourcery.yml.vscode/settings.jsonLICENSEQBot.code-workspaceREADME.mdconfig.yamlmodules/AutoAuth.pymodules/Config.pymodules/HermesWss/README.mdmodules/HermesWss/__init__.pymodules/HermesWss/chunked_upload.pymodules/HermesWss/constants.pymodules/HermesWss/crypto.pymodules/HermesWss/exceptions.pymodules/HermesWss/keyboards.pymodules/HermesWss/models.pymodules/HermesWss/onboard.pymodules/HermesWss/sender.pymodules/HermesWss/token_manager.pymodules/HermesWss/utils.pymodules/HermesWss/wss_client.pymodules/HttpSender.pymodules/Urls.pyplugin/GlmAi.pyplugin/GlmAi_d/.gitignoreplugin/GlmAi_d/Config.pyplugin/GlmAi_d/History.pyplugin/GlmAi_d/Paths.pyplugin/GlmAi_d/RequApi.pyplugin/GlmAi_d/Sql.pyplugin/GlmAi_d/Types.pyplugin/GlmAi_d/config.yamlplugin/GlmAi_d/data/personality/xy.mdplugin/GlmAi_d/sql/init.sqlplugin/GlmAi_d/sql/message/add.sqlplugin/GlmAi_d/sql/message/clear.sqlplugin/GlmAi_d/sql/message/get.sqlplugin/GlmAi_d/sql/user/add.sqlplugin/GlmAi_d/sql/user/check.sqlplugin/GlmAi_d/sql/user/del.sqlplugin/Holiday.pyplugin/Holiday_d/.gitignoreplugin/Holiday_d/RequApi.pyplugin/Holiday_d/SqlCache.pyplugin/Holiday_d/test.httpplugin/TestAutoReply.pyplugin/Weather.pyplugin/Weather_d/.gitignoreplugin/Weather_d/GetWeather.pyplugin/Weather_d/SqlCache.pyplugin/Weather_d/config.yamlplugin/Weather_d/test.httppyproject.tomlrun.shtest/ai.jsontest/aiChat.pytest/passCallback.pytest/recieveHttp.pytest/testSqlmodel.py
Comment on lines
+216
to
+228
| if response.status_code != 200: | ||
| err_body: str = response.text | ||
| # 检查日配额超限（biz_code 在响应体 JSON 中） | ||
| try: | ||
| err_json: dict[str, object] = response.json() | ||
| biz_code: object = err_json.get("biz_code", 0) | ||
| if biz_code == _BIZ_CODE_DAILY_LIMIT: | ||
| raise UploadDailyLimitExceededError(file_name, file_size) | ||
| except Exception: | ||
| pass | ||
| raise UploadError( | ||
| f"upload_prepare failed (status={response.status_code}): {err_body[:200]}" | ||
| ) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Do not swallow UploadDailyLimitExceededError.
The exception raised at Line 223 is immediately caught by except Exception and replaced with a generic UploadError, breaking the public error contract.
Proposed fix
try:
err_json: dict[str, object] = response.json()
- biz_code: object = err_json.get("biz_code", 0)
- if biz_code == _BIZ_CODE_DAILY_LIMIT:
- raise UploadDailyLimitExceededError(file_name, file_size)
- except Exception:
- pass
+ except ValueError:
+ err_json = {}
+ if err_json.get("biz_code") == _BIZ_CODE_DAILY_LIMIT:
+ raise UploadDailyLimitExceededError(file_name, file_size)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| if response.status_code != 200: | |
| err_body: str = response.text | |
| # 检查日配额超限（biz_code 在响应体 JSON 中） | |
| try: | |
| err_json: dict[str, object] = response.json() | |
| biz_code: object = err_json.get("biz_code", 0) | |
| if biz_code == _BIZ_CODE_DAILY_LIMIT: | |
| raise UploadDailyLimitExceededError(file_name, file_size) | |
| except Exception: | |
| pass | |
| raise UploadError( | |
| f"upload_prepare failed (status={response.status_code}): {err_body[:200]}" | |
| ) | |
| if response.status_code != 200: | |
| err_body: str = response.text | |
| # 检查日配额超限（biz_code 在响应体 JSON 中） | |
| try: | |
| err_json: dict[str, object] = response.json() | |
| except ValueError: | |
| err_json = {} | |
| if err_json.get("biz_code") == _BIZ_CODE_DAILY_LIMIT: | |
| raise UploadDailyLimitExceededError(file_name, file_size) | |
| raise UploadError( | |
| f"upload_prepare failed (status={response.status_code}): {err_body[:200]}" | |
| ) |
🧰 Tools
🪛 Ruff (0.15.21)
[warning] 218-218: Comment contains ambiguous （ (FULLWIDTH LEFT PARENTHESIS). Did you mean ( (LEFT PARENTHESIS)?
(RUF003)
[warning] 218-218: Comment contains ambiguous ） (FULLWIDTH RIGHT PARENTHESIS). Did you mean ) (RIGHT PARENTHESIS)?
(RUF003)
[error] 224-225: try-except-pass detected, consider logging the exception
(S110)
[warning] 224-224: Do not catch blind exception: Exception
(BLE001)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@modules/HermesWss/chunked_upload.py` around lines 216 - 228, Preserve
UploadDailyLimitExceededError raised in the response JSON handling within the
non-200 branch by re-raising it before the broad exception handler, or by
narrowing the handler to JSON parsing failures only. Keep generic UploadError
behavior for malformed JSON and other non-limit responses, while retaining the
existing _BIZ_CODE_DAILY_LIMIT check.
Source: Linters/SAST tools
Comment on lines
+447
to
+459
| text: str = build_approval_text(req) | ||
| keyboard: InlineKeyboard = build_approval_keyboard(req.session_key) | ||
|
|
||
| try: | ||
| if chat_type == "c2c": | ||
| await self.send_text_to_c2c(chat_id, text, reply_to, keyboard) | ||
| else: | ||
| await self.send_text_to_group(chat_id, text, reply_to, keyboard) | ||
| logger.info( | ||
| "Approval request sent to %s:%s (session=%.20s...)", | ||
| chat_type, chat_id, req.session_key, | ||
| ) | ||
| return True |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Map the target file and nearby symbols first
ast-grep outline modules/HermesWss/sender.py --view expanded || true
# Read the relevant slice with line numbers
sed -n '400,520p' modules/HermesWss/sender.py
# Find the helpers and send-result handling across the repository
rg -n "build_approval_text|build_approval_keyboard|SendResult|MSG_TYPE_TEXT|Markdown|reply_to|send_text_to_c2c|send_text_to_group" modules -SRepository: dudu7615/QBot
Length of output: 11260
🏁 Script executed:
#!/bin/bash
set -euo pipefail
sed -n '120,220p' modules/HermesWss/sender.py
printf '\n----\n'
sed -n '1,260p' modules/HermesWss/models.py
printf '\n----\n'
sed -n '210,250p' modules/HermesWss/keyboards.py
printf '\n----\n'
sed -n '73,120p' modules/HermesWss/sender.pyRepository: dudu7615/QBot
Length of output: 11306
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Find all markdown message usage and approval-related send paths
rg -n "MSG_TYPE_MARKDOWN|markdown_support|build_approval_text|send_approval_request|send_text_to_c2c|send_text_to_group|success=False|retryable=False" modules/HermesWss -S
printf '\n----\n'
# Inspect the message-type constants and the retry/error path
sed -n '1,120p' modules/HermesWss/constants.py
printf '\n----\n'
sed -n '492,545p' modules/HermesWss/sender.pyRepository: dudu7615/QBot
Length of output: 7379
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Look for any markdown-specific send path or payload shape
rg -n "markdown_support|MSG_TYPE_MARKDOWN|markdown" modules/HermesWss -S
printf '\n----\n'
# Inspect README/docs around sending and any mention of markdown payloads
sed -n '1,140p' modules/HermesWss/README.mdRepository: dudu7615/QBot
Length of output: 4404
Send approval with the right payload and surface failures. send_text_to_* sends plain text, so the Markdown from build_approval_text will not be rendered as intended, and this method still returns True even when the underlying SendResult reports failure. Render plain text here or use the Markdown send path, and return result.success.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@modules/HermesWss/sender.py` around lines 447 - 459, The approval flow in the
surrounding sender method must use a payload compatible with the selected send
API and propagate its result. Update the calls to
send_text_to_c2c/send_text_to_group (or switch to the Markdown-capable path) so
build_approval_text content renders correctly, capture the returned SendResult,
and return its success value instead of always returning True; preserve the
existing logging and routing behavior.
Comment on lines
+512
to
+536
| for attempt in range(max_retries): | ||
| try: | ||
| token: str = await self._token_manager.get_token() | ||
| data: dict[str, object] = await self._api_request(method, path, body, token) | ||
| msg_id: str = str(data.get("id", uuid.uuid4().hex[:12])) | ||
| return SendResult( | ||
| success=True, | ||
| message_id=msg_id, | ||
| raw_response=data, | ||
| ) | ||
| except (QQApiError, QQTokenError) as exc: | ||
| error_str: str = str(exc).lower() | ||
| last_error = str(exc) | ||
|
|
||
| # 永久性错误不重试 | ||
| if any(k in error_str for k in ("invalid", "forbidden", "not found", "bad request")): | ||
| return SendResult(success=False, error=str(exc), retryable=False) | ||
|
|
||
| if attempt < max_retries - 1: | ||
| delay: float = 1.0 * (2.0 ** attempt) | ||
| logger.warning( | ||
| "Send retry %d/%d after %.1fs: %s", | ||
| attempt + 1, max_retries, delay, exc, | ||
| ) | ||
| await asyncio.sleep(delay) |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
rg -n -C3 '\b(msg_seq|idempotency|nonce|event_id)\b' modules/HermesWssRepository: dudu7615/QBot
Length of output: 1226
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '--- sender.py outline ---'
ast-grep outline modules/HermesWss/sender.py --view expanded || true
echo
echo '--- sender.py around retry loop ---'
sed -n '440,580p' modules/HermesWss/sender.py
echo
echo '--- search for request ids / idempotency / transport errors in HermesWss ---'
rg -n -C3 '\b(msg_seq|idempotency|nonce|event_id|request_id|retryable|timeout|disconnect|transport)\b' modules/HermesWss
echo
echo '--- locate QQApiError / QQTokenError definitions and API request implementation ---'
rg -n -C3 'class QQApiError|class QQTokenError|def _api_request|async def _api_request|def _token_manager|get_token\(' modules/HermesWssRepository: dudu7615/QBot
Length of output: 17817
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '--- sender.py _api_request ---'
sed -n '73,126p' modules/HermesWss/sender.py
echo
echo '--- exceptions.py ---'
sed -n '1,120p' modules/HermesWss/exceptions.py
echo
echo '--- any mention of httpx errors in sender/token manager ---'
rg -n -C3 'httpx\.|TimeoutException|RequestError|TransportError|ConnectError|ReadTimeout|WriteTimeout|PoolTimeout|RemoteProtocolError|Timeout' modules/HermesWss/sender.py modules/HermesWss/token_manager.py modules/HermesWss/exceptions.pyRepository: dudu7615/QBot
Length of output: 7903
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '--- message body builders / send methods ---'
sed -n '126,440p' modules/HermesWss/sender.py
echo
echo '--- broader search for duplicate / idempotency guidance ---'
rg -n -C3 '\b(duplicate|idempot|dedup|request key|stable.*id|message.*seq|msg_seq|client.*token|nonce|trace_id)\b' .Repository: dudu7615/QBot
Length of output: 8856
Avoid retrying ambiguous POSTs (modules/HermesWss/sender.py:512-536)
_send_with_retry() retries every QQApiError, and httpx.RequestError is converted into QQApiError(0, ...). If the connection drops after QQ has already accepted the message, the next attempt can send a duplicate because the payload carries no stable idempotency key. Preserve one request/message identifier across attempts if the API supports it, or stop retrying ambiguous transport failures.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@modules/HermesWss/sender.py` around lines 512 - 536, Update _send_with_retry
to avoid retrying ambiguous transport failures, especially for POST requests
where the server may have accepted the message before the connection dropped.
Preserve and reuse a stable request/message identifier across attempts if the
API supports idempotency; otherwise return a non-retryable SendResult for
converted httpx.RequestError cases while retaining retries for safely retryable
API errors.
Comment on lines
+133
to
+137
| # 创建 HTTP 客户端 | ||
| if self._http_client is None: | ||
| self._http_client = aiohttp.ClientSession( | ||
| timeout=aiohttp.ClientTimeout(total=DEFAULT_API_TIMEOUT), | ||
| ) |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
ast-grep outline modules/HermesWss/wss_client.py --match QQWebSocketClient --view expanded
rg -n -C3 'ClientSession\(|trust_env' modules/HermesWss/wss_client.pyRepository: dudu7615/QBot
Length of output: 1593
🏁 Script executed:
sed -n '120,270p' modules/HermesWss/wss_client.pyRepository: dudu7615/QBot
Length of output: 5334
Enable proxy settings on the HTTP client used for token and gateway requests.
self._http_client is created without trust_env=True, so HTTPS_PROXY is ignored for the token and gateway calls even though the WebSocket session enables it. Proxy-only deployments will fail before the WSS URL is obtained.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@modules/HermesWss/wss_client.py` around lines 133 - 137, Update the
aiohttp.ClientSession creation in the _http_client initialization path to enable
trust_env=True, ensuring token and gateway HTTP requests honor HTTPS_PROXY and
related environment proxy settings while preserving the existing timeout
configuration.
Comment on lines
+301
to
+317
| # 创建新的 session 并连接 | ||
| self._ws_session = aiohttp.ClientSession(trust_env=True) | ||
| headers: dict[str, str] = { | ||
| "User-Agent": build_user_agent(), | ||
| } | ||
|
|
||
| if self._ws_session is None: | ||
| raise RuntimeError("WebSocket session not initialized") | ||
|
|
||
| try: | ||
| self._ws = await self._ws_session.ws_connect( | ||
| gateway_url, | ||
| headers=headers, | ||
| timeout=aiohttp.ClientWSTimeout( | ||
| ws_close=CONNECT_TIMEOUT_SECONDS, | ||
| ), | ||
| ) |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
ast-grep outline modules/HermesWss/wss_client.py --match _open_ws --view expanded
rg -n -C4 'CONNECT_TIMEOUT_SECONDS|ClientTimeout|ClientWSTimeout|ws_connect' modules/HermesWss/wss_client.pyRepository: dudu7615/QBot
Length of output: 2035
🌐 Web query:
aiohttp 3.11.1 ClientWSTimeout ws_close connection establishment ws_connect ClientTimeout
💡 Result:
In aiohttp 3.11.x, the timeout configuration for websocket connections underwent significant changes to clarify the separation between connection establishment (HTTP request phase) and websocket-specific timeouts (receive and close phase) [1][2]. 1. Key Changes in 3.11.x The deprecated float-based timeout and receive_timeout parameters in ClientSession.ws_connect have been replaced by the structured ClientWSTimeout class [1][3]. You should now pass a ClientWSTimeout instance to the timeout parameter of ws_connect [4][3]. 2. Understanding Timeouts in ws_connect There is a distinction between the timeout for the initial HTTP connection establishment and the timeout for the active websocket: - Connection Establishment: The initial HTTP handshake for the websocket connection is governed by the standard ClientTimeout configuration of the ClientSession [4][5]. If you need to constrain how long the initial connection takes, ensure your ClientSession is configured with the appropriate ClientTimeout (e.g., total, connect, sock_connect) [4][5]. - Websocket Behavior: Once the connection is established, the ClientWSTimeout object controls the websocket-specific behavior [6][4]: - ws_receive: Sets a timeout for receiving complete messages (default is None/unlimited) [4][5]. - ws_close: Sets a timeout for the websocket to close (default is 10.0 seconds) [6][4]. 3. Implementation Example Instead of using the deprecated float values, use the following pattern: from aiohttp import ClientSession, ClientWSTimeout # Define websocket-specific timeouts ws_timeout = ClientWSTimeout(ws_receive=30.0, ws_close=10.0) async with ClientSession as session: async with session.ws_connect( "http://example.org/ws", timeout=ws_timeout) as ws: # Websocket operations pass Note that the default receive timeout for websockets changed from None to 10.0 in some recent minor versions or is generally encouraged to be set explicitly [1]. If you encounter issues where the connection appears to time out unexpectedly during the handshake, verify that your ClientSession's ClientTimeout settings are sufficient for the network conditions, as those remain independent of the ClientWSTimeout settings [7][4].
Citations:
- 1: https://github.com/aio-libs/aiohttp/releases/tag/v3.11.0
- 2: Deprecate obsolete timeout in ClientSession.ws_connect() (#3946) aio-libs/aiohttp#8965
- 3: https://docs.aiohttp.org/en/latest/_modules/aiohttp/client.html
- 4: http://docs.aiohttp.org/en/stable/client_reference.html
- 5: https://docs.aiohttp.org/en/latest/client%5Freference.html
- 6: https://docs.aiohttp.org/en/latest/_modules/aiohttp/client_ws.html
- 7: Broken timeout system with ws_connect aio-libs/aiohttp#8444
Apply CONNECT_TIMEOUT_SECONDS to the WebSocket handshake. ClientWSTimeout(ws_close=...) only affects the close phase, so reconnects still use aiohttp’s default connection timeout unless the session gets a ClientTimeout or ws_connect is wrapped in asyncio.timeout.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@modules/HermesWss/wss_client.py` around lines 301 - 317, Update the
connection flow around ws_connect in the WebSocket session initialization to
apply CONNECT_TIMEOUT_SECONDS to the handshake, using the aiohttp session
ClientTimeout or an asyncio.timeout wrapper. Do not rely on
ClientWSTimeout(ws_close=...), which should remain limited to close behavior.
Comment on lines
+739
to
+747
| # 消息去重 | ||
| async with self._dedup_lock: | ||
| if msg_id in self._seen_msg_ids: | ||
| logger.debug("[QQBot] Duplicate message: %s", msg_id) | ||
| return | ||
| self._seen_msg_ids.add(msg_id) | ||
| # 限制去重集合大小 | ||
| if len(self._seen_msg_ids) > 1000: | ||
| self._seen_msg_ids = set(list(self._seen_msg_ids)[-500:]) |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Evict deduplication entries by age, not set iteration order.
Converting the set to a list retains an arbitrary 500 IDs, potentially discarding newly received IDs immediately. It also ignores DEDUP_WINDOW_SECONDS, allowing duplicate commands to be processed again. Use an insertion-ordered timestamp map and expire the oldest entries.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@modules/HermesWss/wss_client.py` around lines 739 - 747, The message
deduplication logic in the `_dedup_lock` block must evict entries by age rather
than arbitrary set order. Replace `_seen_msg_ids` with an insertion-ordered
mapping of message IDs to timestamps, record each new ID’s arrival time, and
remove oldest entries whose age exceeds `DEDUP_WINDOW_SECONDS`; preserve
duplicate detection and locking behavior.
dudu7615
added a commit
that referenced
this pull request
Jul 29, 2026
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.
Summary by Sourcery
Introduce a new HermesWss module to connect to QQ via WebSocket, with token management, messaging utilities, and onboarding support, and wire it into the project with dependencies and strict typing.
New Features:
Enhancements:
Documentation:
Summary by CodeRabbit