-
Notifications
You must be signed in to change notification settings - Fork 9.9k
增强域名重试机制 (#472) #474
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
增强域名重试机制 (#472) #474
Conversation
|
Warning Rate limit exceeded@hect0x7 has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 0 minutes and 31 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (4)
WalkthroughAdds AdvancedRetryPlugin with per-domain failure tracking and domain-ordered retries; introduces JSON-extraction utilities (JmcomicText.try_parse_json_object) and a sanitized JmApiResp.json(); refines a local client type annotation; truncates non-JSON error snippets; updates options, CI release workflow/config, and bumps package version. Changes
Sequence Diagram(s)sequenceDiagram
participant Client as JmcomicClient
participant Plugin as AdvancedRetryPlugin
participant Server as RemoteServer
Note over Plugin,Client: Plugin injected via new_jm_client\n(domain_retry_strategy -> request_with_retry)
Client->>Plugin: request_with_retry(request, url, is_image, **kwargs)
Plugin->>Plugin: resolve domain list (get_sorted_domain)
opt iterate domains & rounds
Plugin->>Client: resolve domain-specific URL (if path request)
Plugin->>Server: perform HTTP request
Server-->>Plugin: response / exception
alt response.retryable or exception
Plugin->>Plugin: update_failed_count(domain)
Plugin->>Plugin: select next domain / next round
else success
Plugin-->>Client: return response
end
end
alt all domains exhausted
Plugin->>Client: client.fallback(request, url, 0, 0, is_image, **kwargs)
Client-->>Plugin: fallback response
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Pre-merge checks (2 passed, 1 warning)❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing Touches
🧪 Generate unit tests
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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 3
🧹 Nitpick comments (4)
src/jmcomic/jm_option.py (1)
422-426: Fix type reference forJmcomicClientto avoid F405 and potential runtime evaluation.
- Using
JmcomicClienthere relies on star imports; Ruff flags it (F405). Quote the annotation (or gate a proper import under TYPE_CHECKING) to avoid both the linter warning and any evaluation concerns.Apply this minimal diff:
- client: JmcomicClient = clazz( + client: 'JmcomicClient' = clazz(Additionally (outside the selected lines), consider adding guarded imports at the top to satisfy type checkers:
from typing import TYPE_CHECKING if TYPE_CHECKING: from .jm_client_interface import JmcomicClientsrc/jmcomic/jm_plugin.py (3)
1237-1242: Preferdefaultdict(int)and keep per-client counters thread-safe.Minor improvement: simplify reads/writes to the failure counter and keep the local import of
Lock.Apply this diff:
- client: JmcomicClient = new_jm_client(*args, **kwargs) + client: JmcomicClient = new_jm_client(*args, **kwargs) client.domain_retry_strategy = self.request_with_retry - client.domain_req_failed_counter = {} + from collections import defaultdict + client.domain_req_failed_counter = defaultdict(int) from threading import Lock client.domain_counter_lock = Lock()
1247-1253: Avoid mutating caller-providedkwargsin-place across retries.Copy once at entry to prevent header/cookie pollution between calls.
Apply this diff:
def request_with_retry(self, client: AbstractJmClient, request: Callable, url: str, is_image: bool, **kwargs, ): + # Make a per-call copy to avoid side-effects across retries/domains + kwargs = dict(kwargs)
1253-1259: Docstring punctuation nit.If you run Ruff, replace fullwidth punctuation with ASCII to silence RUF002.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/jmcomic/jm_option.py(1 hunks)src/jmcomic/jm_plugin.py(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
src/jmcomic/jm_option.py (1)
src/jmcomic/jm_client_interface.py (1)
JmcomicClient(456-609)
src/jmcomic/jm_plugin.py (3)
src/jmcomic/jm_option.py (1)
new_jm_client(365-432)src/jmcomic/jm_client_interface.py (4)
JmcomicClient(456-609)url(37-38)of_api_url(484-485)get_domain_list(466-470)src/jmcomic/jm_client_impl.py (11)
request_with_retry(50-121)AbstractJmClient(7-229)of_api_url(44-45)update_request_with_specify_domain(134-138)update_request_with_specify_domain(464-471)update_request_with_specify_domain(916-919)raise_if_resp_should_retry(124-132)raise_if_resp_should_retry(966-1002)get_domain_list(202-203)get(38-39)fallback(209-212)
🪛 Ruff (0.12.2)
src/jmcomic/jm_option.py
422-422: JmcomicClient may be undefined, or defined from star imports
(F405)
src/jmcomic/jm_plugin.py
1224-1224: JmOption may be undefined, or defined from star imports
(F405)
1230-1230: Unused method argument: kwargs
(ARG002)
1234-1234: Callable may be undefined, or defined from star imports
(F405)
1237-1237: JmcomicClient may be undefined, or defined from star imports
(F405)
1247-1247: AbstractJmClient may be undefined, or defined from star imports
(F405)
1248-1248: Callable may be undefined, or defined from star imports
(F405)
1254-1254: Docstring contains ambiguous : (FULLWIDTH COLON). Did you mean : (COLON)?
(RUF002)
1255-1255: Docstring contains ambiguous , (FULLWIDTH COMMA). Did you mean , (COMMA)?
(RUF002)
1255-1255: Docstring contains ambiguous : (FULLWIDTH COLON). Did you mean : (COLON)?
(RUF002)
1256-1256: Docstring contains ambiguous , (FULLWIDTH COMMA). Did you mean , (COMMA)?
(RUF002)
1256-1256: Docstring contains ambiguous : (FULLWIDTH COLON). Did you mean : (COLON)?
(RUF002)
1257-1257: Docstring contains ambiguous , (FULLWIDTH COMMA). Did you mean , (COMMA)?
(RUF002)
1257-1257: Docstring contains ambiguous , (FULLWIDTH COMMA). Did you mean , (COMMA)?
(RUF002)
1266-1266: jm_log may be undefined, or defined from star imports
(F405)
1277-1277: Loop control variable rindex not used within loop body
Rename unused rindex to _rindex
(B007)
1279-1279: Loop control variable i not used within loop body
Rename unused i to _i
(B007)
1285-1285: Do not catch blind exception: Exception
(BLE001)
1288-1288: jm_log may be undefined, or defined from star imports
(F405)
1291-1291: JmcomicClient may be undefined, or defined from star imports
(F405)
1299-1299: AbstractJmClient may be undefined, or defined from star imports
(F405)
1304-1304: JmcomicClient may be undefined, or defined from star imports
(F405)
🔇 Additional comments (4)
src/jmcomic/jm_plugin.py (4)
1224-1227: Init looks fine.Constructor is clear; storing
retry_configon the instance is appropriate.
1291-1297: LGTM.Sorting by failure count and filtering by
timesis straightforward and stable.
1299-1302: Thread-safe counter update looks good.Locking per client avoids races on concurrent requests.
1304-1307: Static helper is fine.Clear and side-effect free.
| self.require_param(isinstance(retry_config, dict), '必须配置retry_config为dict') | ||
| self.retry_config = retry_config | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Validate required keys and normalize types in retry_config.
Fail fast if keys are missing; coerce to int to avoid surprises later.
Apply this diff:
- self.require_param(isinstance(retry_config, dict), '必须配置retry_config为dict')
- self.retry_config = retry_config
+ self.require_param(isinstance(retry_config, dict), '必须配置retry_config为dict')
+ required = ('retry_rounds', 'retry_domain_max_times')
+ missing = [k for k in required if k not in retry_config]
+ self.require_param(not missing, f'缺少配置项: {missing}')
+ retry_config['retry_rounds'] = int(retry_config['retry_rounds'])
+ retry_config['retry_domain_max_times'] = int(retry_config['retry_domain_max_times'])
+ self.retry_config = retry_config📝 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.
| self.require_param(isinstance(retry_config, dict), '必须配置retry_config为dict') | |
| self.retry_config = retry_config | |
| self.require_param(isinstance(retry_config, dict), '必须配置retry_config为dict') | |
| required = ('retry_rounds', 'retry_domain_max_times') | |
| missing = [k for k in required if k not in retry_config] | |
| self.require_param(not missing, f'缺少配置项: {missing}') | |
| retry_config['retry_rounds'] = int(retry_config['retry_rounds']) | |
| retry_config['retry_domain_max_times'] = int(retry_config['retry_domain_max_times']) | |
| self.retry_config = retry_config |
🤖 Prompt for AI Agents
In src/jmcomic/jm_plugin.py around lines 1231-1233, after verifying retry_config
is a dict, validate that all required keys used elsewhere (define a local
required_keys list matching the keys the retry logic expects) are present and
non-empty by calling self.require_param for each missing key to fail fast; then
normalize numeric fields by coercing values to int (wrap int(...) in try/except
to raise a clear require_param error on invalid types) and replace
self.retry_config with the normalized dict so later code won’t encounter
unexpected types.
| def do_request(domain): | ||
| url_to_use = url | ||
| if url_to_use.startswith('/'): | ||
| # path → url | ||
| url_to_use = client.of_api_url(url, domain) | ||
| client.update_request_with_specify_domain(kwargs, domain, is_image) | ||
| jm_log(client.log_topic(), client.decode(url_to_use)) | ||
| elif is_image: | ||
| # 图片url | ||
| client.update_request_with_specify_domain(kwargs, None, is_image) | ||
|
|
||
| resp = request(url_to_use, **kwargs) | ||
| resp = client.raise_if_resp_should_retry(resp, is_image) | ||
| return resp | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Use per-attempt kwargs and reset failure count on success.
- Each attempt should use its own kwargs copy; domain-specific header mutations shouldn’t leak across attempts.
- Reset the domain’s failure counter on success to allow recovery.
Apply this diff:
- def do_request(domain):
- url_to_use = url
+ def do_request(domain):
+ # Use a fresh kwargs for each attempt
+ local_kwargs = dict(kwargs)
+ url_to_use = url
if url_to_use.startswith('/'):
# path → url
url_to_use = client.of_api_url(url, domain)
- client.update_request_with_specify_domain(kwargs, domain, is_image)
+ client.update_request_with_specify_domain(local_kwargs, domain, is_image)
jm_log(client.log_topic(), client.decode(url_to_use))
elif is_image:
# 图片url
- client.update_request_with_specify_domain(kwargs, None, is_image)
+ client.update_request_with_specify_domain(local_kwargs, None, is_image)
- resp = request(url_to_use, **kwargs)
+ resp = request(url_to_use, **local_kwargs)
resp = client.raise_if_resp_should_retry(resp, is_image)
+ # Success → clear failed counter for this domain
+ with client.domain_counter_lock:
+ client.domain_req_failed_counter[domain] = 0
return resp📝 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.
| def do_request(domain): | |
| url_to_use = url | |
| if url_to_use.startswith('/'): | |
| # path → url | |
| url_to_use = client.of_api_url(url, domain) | |
| client.update_request_with_specify_domain(kwargs, domain, is_image) | |
| jm_log(client.log_topic(), client.decode(url_to_use)) | |
| elif is_image: | |
| # 图片url | |
| client.update_request_with_specify_domain(kwargs, None, is_image) | |
| resp = request(url_to_use, **kwargs) | |
| resp = client.raise_if_resp_should_retry(resp, is_image) | |
| return resp | |
| def do_request(domain): | |
| # Use a fresh kwargs for each attempt | |
| local_kwargs = dict(kwargs) | |
| url_to_use = url | |
| if url_to_use.startswith('/'): | |
| # path → url | |
| url_to_use = client.of_api_url(url, domain) | |
| client.update_request_with_specify_domain(local_kwargs, domain, is_image) | |
| jm_log(client.log_topic(), client.decode(url_to_use)) | |
| elif is_image: | |
| # 图片url | |
| client.update_request_with_specify_domain(local_kwargs, None, is_image) | |
| resp = request(url_to_use, **local_kwargs) | |
| resp = client.raise_if_resp_should_retry(resp, is_image) | |
| # Success → clear failed counter for this domain | |
| with client.domain_counter_lock: | |
| client.domain_req_failed_counter[domain] = 0 | |
| return resp |
🧰 Tools
🪛 Ruff (0.12.2)
1266-1266: jm_log may be undefined, or defined from star imports
(F405)
🤖 Prompt for AI Agents
In src/jmcomic/jm_plugin.py around lines 1260 to 1274, the request loop mutates
and reuses the same kwargs across retry attempts and never clears a domain
failure count on success; change it so each attempt creates a fresh copy of
kwargs (e.g., per_attempt_kwargs = dict(kwargs)) and use that copy for
client.update_request_with_specify_domain and request so domain-specific header
changes do not leak between attempts, and after a successful response reset the
domain's failure counter (either via the existing client reset helper if
available or by setting the client's failure counter for that domain to zero) so
the domain can recover for future requests.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
♻️ Duplicate comments (2)
src/jmcomic/jm_plugin.py (2)
1231-1233: Validate required keys and coerce types in retry_config.Fail fast and normalize to int to avoid runtime surprises.
- self.require_param(isinstance(retry_config, dict), '必须配置retry_config为dict') - self.retry_config = retry_config + self.require_param(isinstance(retry_config, dict), '必须配置retry_config为dict') + required = ('retry_rounds', 'retry_domain_max_times') + missing = [k for k in required if k not in retry_config] + self.require_param(not missing, f'缺少配置项: {missing}') + try: + retry_config['retry_rounds'] = int(retry_config['retry_rounds']) + retry_config['retry_domain_max_times'] = int(retry_config['retry_domain_max_times']) + except (TypeError, ValueError) as e: + raise PluginValidationException(self, f'retry_config参数类型错误: {e}') + self.retry_config = retry_config
1260-1274: Isolate per-attempt kwargs; reset domain failure count on success.Prevents header/cookie leakage across attempts and lets domains recover.
- def do_request(domain): - url_to_use = url + def do_request(domain): + # fresh kwargs per attempt + local_kwargs = dict(kwargs) + url_to_use = url if url_to_use.startswith('/'): # path → url url_to_use = client.of_api_url(url, domain) - client.update_request_with_specify_domain(kwargs, domain, is_image) + client.update_request_with_specify_domain(local_kwargs, domain, is_image) jm_log(client.log_topic(), client.decode(url_to_use)) elif is_image: # 图片url - client.update_request_with_specify_domain(kwargs, None, is_image) + client.update_request_with_specify_domain(local_kwargs, None, is_image) - resp = request(url_to_use, **kwargs) + resp = request(url_to_use, **local_kwargs) resp = client.raise_if_resp_should_retry(resp, is_image) + # success → clear failure count for this domain + with client.domain_counter_lock: + client.domain_req_failed_counter[domain] = 0 return resp
🧹 Nitpick comments (6)
src/jmcomic/jm_plugin.py (5)
1277-1279: Rename unused loop variables; simplify inner loop.Fixes B007 and removes redundant enumerate.
- for rindex in range(retry_rounds): + for _ in range(retry_rounds): domain_list = self.get_sorted_domain(client, retry_domain_max_times) - for i, domain in enumerate(domain_list): - if self.failed_count(client, domain) >= retry_domain_max_times: - continue + for domain in domain_list: + # get_sorted_domain 已过滤超限域名
1285-1289: Avoid blind except; count only retryable network errors.Prefer catching requests.exceptions.RequestException; re-raise non-network errors.
Example:
import requests ... try: return do_request(domain) except requests.exceptions.RequestException as e: from common import traceback_print_exec traceback_print_exec() jm_log('req.error', str(e)) self.update_failed_count(client, domain)
1306-1308: Read failure counter under the same lock for consistency.Small concurrency hygiene.
@staticmethod def failed_count(client: JmcomicClient, domain: str) -> int: - # noinspection PyUnresolvedReferences - return client.domain_req_failed_counter.get(domain, 0) + # noinspection PyUnresolvedReferences + with client.domain_counter_lock: + return client.domain_req_failed_counter.get(domain, 0)
1253-1258: Fix full-width punctuation in docstring (RUF002).Keeps linters quiet.
- """ - 实现如下域名重试机制: - - 对域名列表轮询请求,配置:retry_rounds - - 限制单个域名最大失败次数,配置:retry_domain_max_times - - 轮询域名列表前,根据历史失败次数对域名列表排序,失败多的后置 - """ + """ + 实现如下域名重试机制: + - 对域名列表轮询请求, 配置: retry_rounds + - 限制单个域名最大失败次数, 配置: retry_domain_max_times + - 轮询域名列表前, 根据历史失败次数对域名列表排序, 失败多的后置 + """
1234-1234: Import Callable from typing to silence F405.Localize the import or add it at module scope.
Add:
from typing import Callableassets/option/option_test_api.yml (1)
28-29: Style: drop trailing comma in flow sequence (optional).Purely cosmetic.
- whitelist: [ api, ] + whitelist: [ api ]
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
assets/option/option_test_api.yml(1 hunks)src/jmcomic/jm_plugin.py(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
src/jmcomic/jm_plugin.py (3)
src/jmcomic/jm_option.py (1)
new_jm_client(365-432)src/jmcomic/jm_client_interface.py (4)
JmcomicClient(456-609)url(37-38)of_api_url(484-485)get_domain_list(466-470)src/jmcomic/jm_client_impl.py (11)
request_with_retry(50-121)AbstractJmClient(7-229)of_api_url(44-45)update_request_with_specify_domain(134-138)update_request_with_specify_domain(464-471)update_request_with_specify_domain(916-919)raise_if_resp_should_retry(124-132)raise_if_resp_should_retry(966-1002)fallback(209-212)get_domain_list(202-203)get(38-39)
🪛 YAMLlint (1.37.1)
assets/option/option_test_api.yml
[error] 34-34: no new line character at the end of file
(new-line-at-end-of-file)
🪛 Ruff (0.12.2)
src/jmcomic/jm_plugin.py
1224-1224: JmOption may be undefined, or defined from star imports
(F405)
1230-1230: Unused method argument: kwargs
(ARG002)
1234-1234: Callable may be undefined, or defined from star imports
(F405)
1237-1237: JmcomicClient may be undefined, or defined from star imports
(F405)
1247-1247: AbstractJmClient may be undefined, or defined from star imports
(F405)
1248-1248: Callable may be undefined, or defined from star imports
(F405)
1254-1254: Docstring contains ambiguous : (FULLWIDTH COLON). Did you mean : (COLON)?
(RUF002)
1255-1255: Docstring contains ambiguous , (FULLWIDTH COMMA). Did you mean , (COMMA)?
(RUF002)
1255-1255: Docstring contains ambiguous : (FULLWIDTH COLON). Did you mean : (COLON)?
(RUF002)
1256-1256: Docstring contains ambiguous , (FULLWIDTH COMMA). Did you mean , (COMMA)?
(RUF002)
1256-1256: Docstring contains ambiguous : (FULLWIDTH COLON). Did you mean : (COLON)?
(RUF002)
1257-1257: Docstring contains ambiguous , (FULLWIDTH COMMA). Did you mean , (COMMA)?
(RUF002)
1257-1257: Docstring contains ambiguous , (FULLWIDTH COMMA). Did you mean , (COMMA)?
(RUF002)
1266-1266: jm_log may be undefined, or defined from star imports
(F405)
1277-1277: Loop control variable rindex not used within loop body
Rename unused rindex to _rindex
(B007)
1279-1279: Loop control variable i not used within loop body
Rename unused i to _i
(B007)
1285-1285: Do not catch blind exception: Exception
(BLE001)
1288-1288: jm_log may be undefined, or defined from star imports
(F405)
1293-1293: JmcomicClient may be undefined, or defined from star imports
(F405)
1301-1301: AbstractJmClient may be undefined, or defined from star imports
(F405)
1306-1306: JmcomicClient may be undefined, or defined from star imports
(F405)
| - plugin: advanced-retry | ||
| kwargs: | ||
| retry_config: | ||
| retry_rounds: 3 # 一共对域名列表重试3轮 | ||
| retry_domain_max_times: 5 # 当一个域名重试次数超过5次,忽略该域名,不再重试 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Verification agent
🧩 Analysis chain
Add newline at EOF; confirm retry interplay.
Add final newline. Also verify that client.retry_times is intentionally unused when advanced-retry is active to avoid confusion.
🏁 Script executed:
#!/bin/bash
# Inspect where domain_retry_strategy overrides built-in retry and where retry_times is read.
rg -n -C2 'domain_retry_strategy|retry_times' src/jmcomicLength of output: 3936
🏁 Script executed:
#!/bin/bash
# Check newline at EOF
file="assets/option/option_test_api.yml"
if [ ! -f "$file" ]; then
echo "$file not found"
else
tail -c1 "$file" | od -An -t c
fi
echo '--- request_with_retry snippet ---'
rg -n -C3 "def request_with_retry" src/jmcomic/jm_plugin.py || echo "def request_with_retry not found"
echo '--- retry_times usages in jm_plugin.py ---'
rg -n "retry_times" -C3 src/jmcomic/jm_plugin.py || echo "retry_times not found"Length of output: 842
🏁 Script executed:
#!/bin/bash
python3 - << 'EOF'
from pathlib import Path
f = Path('assets/option/option_test_api.yml')
b = f.read_bytes()
print("newline at EOF" if b.endswith(b'\n') else "no newline at EOF")
EOFLength of output: 29
Add final newline to assets/option/option_test_api.yml. Confirmed that when advanced-retry (domain_retry_strategy) is set, JmcomicClient always uses request_with_retry and ignores client.retry_times, so retry_times is intentionally unused.
🧰 Tools
🪛 YAMLlint (1.37.1)
[error] 34-34: no new line character at the end of file
(new-line-at-end-of-file)
🤖 Prompt for AI Agents
In assets/option/option_test_api.yml around lines 30 to 34, the file is missing
a trailing newline; add a final newline character at the end of the file so the
YAML ends with a newline. Also leave the advanced-retry block unchanged (the
note that JmcomicClient uses request_with_retry when domain_retry_strategy is
set and ignores client.retry_times is expected), so only add the newline and do
not modify the retry configuration.
This reverts commit a1b9597.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (2)
.github/workflows/release_auto.yml (2)
32-32: Upgrade to softprops/action-gh-release v2: LGTMv2 is fine here with contents: write permission already set. As a minor cleanup, consider passing the token via input instead of env for consistency with v2 examples.
- - name: Create Release - uses: softprops/action-gh-release@v2 - env: - GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} - with: + - name: Create Release + uses: softprops/action-gh-release@v2 + with: + token: ${{ secrets.GITHUB_TOKEN }} tag_name: ${{ steps.tb.outputs.tag }} - body_path: release_body.txt generate_release_notes: true
38-38: generate_release_notes + body_path: confirm desired behaviorIf you intend to use fully auto-generated notes, drop body_path. If you want to keep your custom release_body.txt content alongside auto notes, add append_body: true.
Option A (auto notes only):
- body_path: release_body.txt generate_release_notes: trueOption B (auto notes + append custom body):
generate_release_notes: true + append_body: true + body_path: release_body.txt
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
.github/release.yml(1 hunks).github/workflows/release_auto.yml(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- .github/release.yml
重写json()方法,专门处理API响应的清理
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/jmcomic/jm_client_interface.py (1)
116-118: Be tolerant to 'code' type (int vs str).Some APIs return "200" (str). Safer to normalize before compare.
- return super().is_success and self.json()['code'] == 200 + code = self.json().get('code') + return super().is_success and str(code) == '200'Please confirm real API samples keep code as int; if so, ignore.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/jmcomic/jm_client_interface.py(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
src/jmcomic/jm_client_interface.py (2)
src/jmcomic/jm_exception.py (5)
resp(23-24)resp(31-35)ExceptionTool(69-191)raises_resp(125-135)JsonResolveFailException(46-47)src/jmcomic/jm_client_impl.py (1)
require_resp_status_ok(893-899)
🪛 Ruff (0.12.2)
src/jmcomic/jm_client_interface.py
104-104: Comment contains ambiguous , (FULLWIDTH COMMA). Did you mean , (COMMA)?
(RUF003)
105-105: field_cache may be undefined, or defined from star imports
(F405)
106-106: Dict may be undefined, or defined from star imports
(F405)
109-109: Comment contains ambiguous ( (FULLWIDTH LEFT PARENTHESIS). Did you mean ( (LEFT PARENTHESIS)?
(RUF003)
109-109: Comment contains ambiguous ) (FULLWIDTH RIGHT PARENTHESIS). Did you mean ) (RIGHT PARENTHESIS)?
(RUF003)
111-111: Consider moving this statement to an else block
(TRY300)
112-112: Do not catch blind exception: Exception
(BLE001)
113-113: ExceptionTool may be undefined, or defined from star imports
(F405)
113-113: JsonResolveFailException may be undefined, or defined from star imports
(F405)
🪛 GitHub Actions: Run Test (API)
src/jmcomic/jm_client_interface.py
[error] 107-107: Command 'python -m unittest' failed due to ImportError: cannot import name 'safe_parse_json' from 'jmcomic.jm_toolkit' (/home/runner/work/JMComic-Crawler-Python/JMComic-Crawler-Python/src/jmcomic/jm_toolkit.py).
src/jmcomic/jm_client_interface.py
Outdated
| # 重写json()方法,专门处理API响应的清理 | ||
| @field_cache() | ||
| def json(self) -> Dict: | ||
| from .jm_toolkit import safe_parse_json # 导入清理函数 | ||
| try: | ||
| # 仅对API响应进行清理(保留原始JmJsonResp的逻辑不变) | ||
| clean_text = safe_parse_json(self.resp.text) | ||
| return clean_text | ||
| except Exception as e: | ||
| ExceptionTool.raises_resp(f'API json解析失败: {e}', self, JsonResolveFailException) | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fix ImportError and make JSON parsing robust (pipeline red).
safe_parse_json is not available and the import occurs before the try/except, causing an ImportError and test failure. Also avoid catching bare Exception and address Ruff RUF003 punctuation nits.
Apply this diff:
- # 重写json()方法,专门处理API响应的清理
+ # 重写json()方法, 专门处理API响应的清理
@field_cache()
def json(self) -> Dict:
- from .jm_toolkit import safe_parse_json # 导入清理函数
- try:
- # 仅对API响应进行清理(保留原始JmJsonResp的逻辑不变)
- clean_text = safe_parse_json(self.resp.text)
- return clean_text
- except Exception as e:
- ExceptionTool.raises_resp(f'API json解析失败: {e}', self, JsonResolveFailException)
+ # 延迟导入, 若不存在则优雅降级到 resp.json()
+ try:
+ from .jm_toolkit import safe_parse_json # type: ignore
+ except Exception:
+ safe_parse_json = None # type: ignore[assignment]
+
+ try:
+ # 仅对API响应进行清理 (保留原始 JmJsonResp 的逻辑不变)
+ text = self.resp.text
+ data = safe_parse_json(text) if callable(safe_parse_json) else self.resp.json()
+ if not isinstance(data, dict):
+ raise TypeError(f"Expected dict, got {type(data).__name__}")
+ return data
+ except (ValueError, TypeError) as e:
+ ExceptionTool.raises_resp(f'API json解析失败: {e}', self, JsonResolveFailException)Add this import at the top of the file to silence F405 on Dict:
from typing import Dict🧰 Tools
🪛 Ruff (0.12.2)
104-104: Comment contains ambiguous , (FULLWIDTH COMMA). Did you mean , (COMMA)?
(RUF003)
105-105: field_cache may be undefined, or defined from star imports
(F405)
106-106: Dict may be undefined, or defined from star imports
(F405)
109-109: Comment contains ambiguous ( (FULLWIDTH LEFT PARENTHESIS). Did you mean ( (LEFT PARENTHESIS)?
(RUF003)
109-109: Comment contains ambiguous ) (FULLWIDTH RIGHT PARENTHESIS). Did you mean ) (RIGHT PARENTHESIS)?
(RUF003)
111-111: Consider moving this statement to an else block
(TRY300)
112-112: Do not catch blind exception: Exception
(BLE001)
113-113: ExceptionTool may be undefined, or defined from star imports
(F405)
113-113: JsonResolveFailException may be undefined, or defined from star imports
(F405)
🪛 GitHub Actions: Run Test (API)
[error] 107-107: Command 'python -m unittest' failed due to ImportError: cannot import name 'safe_parse_json' from 'jmcomic.jm_toolkit' (/home/runner/work/JMComic-Crawler-Python/JMComic-Crawler-Python/src/jmcomic/jm_toolkit.py).
|
@coderabbitai review latest commit |
|
@hect0x7 I'll review the latest commit in this PR which enhances the domain retry mechanism. Let me analyze the changes across all the modified files. ✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 3
♻️ Duplicate comments (1)
src/jmcomic/jm_client_interface.py (1)
104-114: Unify JmApiResp.json, drop missing safe_parse_json import, add robust fallback and type checks.Removes the unresolved import, keeps caching, and makes parsing resilient.
- # 重写json()方法,专门处理API响应的清理 + # 重写 json() 方法, 专门处理 API 响应的清理 @field_cache() def json(self) -> Dict: - from .jm_toolkit import safe_parse_json # 导入清理函数 - try: - # 仅对API响应进行清理(保留原始JmJsonResp的逻辑不变) - clean_text = safe_parse_json(self.resp.text) - return clean_text - except Exception as e: - ExceptionTool.raises_resp(f'API json解析失败: {e}', self, JsonResolveFailException) + from json import JSONDecodeError + # 1) Try native JSON first + try: + data = self.resp.json() + if isinstance(data, dict): + return data + except (ValueError, TypeError, JSONDecodeError): + pass + # 2) Fallback: extract first valid JSON object from text + try: + data = JmcomicText.try_parse_json_object(self.resp.text) + if not isinstance(data, dict): + raise TypeError(f'Expected dict, got {type(data).__name__}') + return data + except (ValueError, TypeError, JSONDecodeError) as e: + ExceptionTool.raises_resp(f'API json解析失败: {e}', self, JsonResolveFailException)
🧹 Nitpick comments (2)
src/jmcomic/jm_client_interface.py (2)
104-110: Fix fullwidth punctuation in comments (RUF003).Use ASCII comma/parentheses to satisfy linters.
- # 重写json()方法,专门处理API响应的清理 + # 重写 json() 方法, 专门处理 API 响应的清理 - # 仅对API响应进行清理(保留原始JmJsonResp的逻辑不变) + # 仅对 API 响应进行清理 (保留原始 JmJsonResp 的逻辑不变)
1-1: Import Dict explicitly to silence F405 on typing.Star import does not guarantee
Dict. Keep it explicit.Add near the top of the file:
from typing import Dict
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/jmcomic/jm_client_interface.py(1 hunks)src/jmcomic/jm_toolkit.py(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
src/jmcomic/jm_toolkit.py (2)
src/jmcomic/jm_client_interface.py (4)
text(33-34)json(88-92)json(106-113)json(119-124)src/jmcomic/jm_exception.py (2)
JsonResolveFailException(46-47)ExceptionTool(69-191)
src/jmcomic/jm_client_interface.py (2)
src/jmcomic/jm_exception.py (4)
resp(23-24)resp(31-35)ExceptionTool(69-191)raises_resp(125-135)src/jmcomic/jm_toolkit.py (1)
try_parse_json_object(351-359)
🪛 Ruff (0.12.2)
src/jmcomic/jm_toolkit.py
64-64: Comment contains ambiguous , (FULLWIDTH COMMA). Did you mean , (COMMA)?
(RUF003)
65-65: re may be undefined, or defined from star imports
(F405)
src/jmcomic/jm_client_interface.py
104-104: Comment contains ambiguous , (FULLWIDTH COMMA). Did you mean , (COMMA)?
(RUF003)
105-105: field_cache may be undefined, or defined from star imports
(F405)
106-106: Dict may be undefined, or defined from star imports
(F405)
109-109: Comment contains ambiguous ( (FULLWIDTH LEFT PARENTHESIS). Did you mean ( (LEFT PARENTHESIS)?
(RUF003)
109-109: Comment contains ambiguous ) (FULLWIDTH RIGHT PARENTHESIS). Did you mean ) (RIGHT PARENTHESIS)?
(RUF003)
111-111: Consider moving this statement to an else block
(TRY300)
112-112: Do not catch blind exception: Exception
(BLE001)
113-113: ExceptionTool may be undefined, or defined from star imports
(F405)
113-113: JsonResolveFailException may be undefined, or defined from star imports
(F405)
119-119: Redefinition of unused json from line 106
(F811)
119-119: Dict may be undefined, or defined from star imports
(F405)
122-122: JmcomicText may be undefined, or defined from star imports
(F405)
123-123: Do not catch blind exception: Exception
(BLE001)
124-124: ExceptionTool may be undefined, or defined from star imports
(F405)
124-124: JsonResolveFailException may be undefined, or defined from star imports
(F405)
🔇 Additional comments (1)
src/jmcomic/jm_client_interface.py (1)
115-118: Guard is_success against non-dict JSON only via json()’s type checks.With the unified json(),
self.json()['code']is safe; no change needed beyond the json() fix.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 3
♻️ Duplicate comments (1)
src/jmcomic/jm_client_interface.py (1)
104-114: Single, cached json() with safe fallback; fix ImportError and broad except.
- Two json() methods exist; keep one cached method and merge logic.
- Importing
safe_parse_jsonunguarded reintroduces the ImportError seen previously.- Catch specific exceptions and normalize errors. Also switch to ASCII punctuation to silence RUF003.
Apply:
- # 重写json()方法,专门处理API响应的清理 - @field_cache() - def json(self) -> Dict: - from .jm_toolkit import safe_parse_json # 导入清理函数 - try: - # 仅对API响应进行清理(保留原始JmJsonResp的逻辑不变) - clean_text = safe_parse_json(self.resp.text) - return clean_text - except Exception as e: - ExceptionTool.raises_resp(f'API json解析失败: {e}', self, JsonResolveFailException) + # 重写 json() 方法, 专门处理 API 响应的清理 + @field_cache() + def json(self) -> Dict: + try: + # 尝试使用清理器; 若缺失则优雅降级 + try: + from .jm_toolkit import safe_parse_json # type: ignore + except Exception: + safe_parse_json = None # type: ignore[assignment] + text = self.resp.text + if callable(safe_parse_json): + data = safe_parse_json(text) # type: ignore[misc] + else: + try: + data = self.resp.json() + except ValueError: + from .jm_toolkit import JmcomicText + data = JmcomicText.try_parse_json_object(text) + if not isinstance(data, dict): + raise TypeError(f"Expected dict, got {type(data).__name__}") + return data + except (ValueError, TypeError) as e: + ExceptionTool.raises_resp(f'API json解析失败: {e}', self, JsonResolveFailException)Additionally (outside the hunk), add missing typing import to silence F405:
from typing import Dict
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/jmcomic/jm_client_interface.py(1 hunks)src/jmcomic/jm_toolkit.py(2 hunks)
🧰 Additional context used
🪛 Ruff (0.12.2)
src/jmcomic/jm_toolkit.py
64-64: Comment contains ambiguous , (FULLWIDTH COMMA). Did you mean , (COMMA)?
(RUF003)
65-65: re may be undefined, or defined from star imports
(F405)
src/jmcomic/jm_client_interface.py
104-104: Comment contains ambiguous , (FULLWIDTH COMMA). Did you mean , (COMMA)?
(RUF003)
105-105: field_cache may be undefined, or defined from star imports
(F405)
106-106: Dict may be undefined, or defined from star imports
(F405)
109-109: Comment contains ambiguous ( (FULLWIDTH LEFT PARENTHESIS). Did you mean ( (LEFT PARENTHESIS)?
(RUF003)
109-109: Comment contains ambiguous ) (FULLWIDTH RIGHT PARENTHESIS). Did you mean ) (RIGHT PARENTHESIS)?
(RUF003)
111-111: Consider moving this statement to an else block
(TRY300)
112-112: Do not catch blind exception: Exception
(BLE001)
113-113: ExceptionTool may be undefined, or defined from star imports
(F405)
113-113: JsonResolveFailException may be undefined, or defined from star imports
(F405)
119-119: Redefinition of unused json from line 106
(F811)
119-119: Dict may be undefined, or defined from star imports
(F405)
122-122: JmcomicText may be undefined, or defined from star imports
(F405)
123-123: Do not catch blind exception: Exception
(BLE001)
124-124: ExceptionTool may be undefined, or defined from star imports
(F405)
124-124: JsonResolveFailException may be undefined, or defined from star imports
(F405)
| # noinspection PyTypeChecker | ||
| @classmethod | ||
| def try_parse_json_object(cls, text: str) -> dict: | ||
| import json | ||
| text = text.strip() | ||
| if text.startswith('{') and text.endswith('}'): | ||
| # fast case | ||
| return json.loads(text) | ||
|
|
||
| for match in cls.pattern_api_response_json_object.finditer(text): | ||
| return json.loads(match.group(0)) | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Make JSON extraction deterministic; avoid shallow {.*?} regex and ensure proper errors.
- Current regex stops at the first
}and fails on nested objects or braces inside strings; it also returnsNonesilently when no match. - Use
json.JSONDecoder().raw_decodefrom the first{for correctness and raise clear exceptions; keep a regex fallback only as last resort.
Apply:
- # noinspection PyTypeChecker
- @classmethod
- def try_parse_json_object(cls, text: str) -> dict:
- import json
- text = text.strip()
- if text.startswith('{') and text.endswith('}'):
- # fast case
- return json.loads(text)
-
- for match in cls.pattern_api_response_json_object.finditer(text):
- return json.loads(match.group(0))
+ # noinspection PyTypeChecker
+ @classmethod
+ def try_parse_json_object(cls, text: str) -> dict:
+ import json
+ text = text.lstrip()
+ # fast path
+ if text.startswith('{'):
+ return json.loads(text)
+ # robust path: locate first '{' and decode from there
+ start = text.find('{')
+ if start == -1:
+ raise ValueError('No JSON object found in text')
+ try:
+ obj, _ = json.JSONDecoder().raw_decode(text, idx=start)
+ except json.JSONDecodeError as e:
+ # shallow fallback (may still fail on complex inputs)
+ match = cls.pattern_api_response_json_object.search(text)
+ if not match:
+ raise e
+ obj = json.loads(match.group(0))
+ if not isinstance(obj, dict):
+ raise TypeError(f'Expected JSON object, got {type(obj).__name__}')
+ return obj📝 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.
| # noinspection PyTypeChecker | |
| @classmethod | |
| def try_parse_json_object(cls, text: str) -> dict: | |
| import json | |
| text = text.strip() | |
| if text.startswith('{') and text.endswith('}'): | |
| # fast case | |
| return json.loads(text) | |
| for match in cls.pattern_api_response_json_object.finditer(text): | |
| return json.loads(match.group(0)) | |
| # noinspection PyTypeChecker | |
| @classmethod | |
| def try_parse_json_object(cls, text: str) -> dict: | |
| import json | |
| text = text.lstrip() | |
| # fast path | |
| if text.startswith('{'): | |
| return json.loads(text) | |
| # robust path: locate first '{' and decode from there | |
| start = text.find('{') | |
| if start == -1: | |
| raise ValueError('No JSON object found in text') | |
| try: | |
| obj, _ = json.JSONDecoder().raw_decode(text, idx=start) | |
| except json.JSONDecodeError as e: | |
| # shallow fallback (may still fail on complex inputs) | |
| match = cls.pattern_api_response_json_object.search(text) | |
| if not match: | |
| raise e | |
| obj = json.loads(match.group(0)) | |
| if not isinstance(obj, dict): | |
| raise TypeError(f'Expected JSON object, got {type(obj).__name__}') | |
| return obj |
🤖 Prompt for AI Agents
In src/jmcomic/jm_toolkit.py around lines 349 to 360, the current JSON
extraction uses a shallow regex and can silently return None; replace it by
locating the first '{' in the stripped text, then use
json.JSONDecoder().raw_decode starting at that index to parse a full JSON value
(catch JSONDecodeError), verify the parsed value is a dict and return it, and on
failure raise a clear ValueError mentioning the parse error and snippet; keep
the existing regex search only as a last-resort fallback if raw_decode fails,
and ensure the function never returns None but always raises on unparseable
input.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/jmcomic/jm_client_interface.py(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
src/jmcomic/jm_client_interface.py (2)
src/jmcomic/jm_toolkit.py (1)
try_parse_json_object(351-359)src/jmcomic/jm_exception.py (5)
resp(23-24)resp(31-35)ExceptionTool(69-191)raises_resp(125-135)JsonResolveFailException(46-47)
🪛 Ruff (0.12.2)
src/jmcomic/jm_client_interface.py
104-104: Comment contains ambiguous , (FULLWIDTH COMMA). Did you mean , (COMMA)?
(RUF003)
105-105: field_cache may be undefined, or defined from star imports
(F405)
106-106: Dict may be undefined, or defined from star imports
(F405)
108-108: JmcomicText may be undefined, or defined from star imports
(F405)
109-109: Do not catch blind exception: Exception
(BLE001)
110-110: ExceptionTool may be undefined, or defined from star imports
(F405)
110-110: JsonResolveFailException may be undefined, or defined from star imports
(F405)
src/jmcomic/jm_client_interface.py
Outdated
| # 重写json()方法,专门处理API响应的清理 | ||
| @field_cache() | ||
| def json(self) -> Dict: | ||
| try: | ||
| return JmcomicText.try_parse_json_object(self.resp.text) | ||
| except Exception as e: | ||
| ExceptionTool.raises_resp(f'json解析失败: {e}', self, JsonResolveFailException) | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Harden JmApiResp.json: avoid blind except, ensure dict return, fix punctuation.
try_parse_json_object() can return None; current code would silently return None and later KeyError/TypeError surface elsewhere. Also catches bare Exception and uses fullwidth comma in the comment (RUF003).
Apply:
- # 重写json()方法,专门处理API响应的清理
+ # 重写json()方法, 专门处理API响应的清理
@field_cache()
def json(self) -> Dict:
- try:
- return JmcomicText.try_parse_json_object(self.resp.text)
- except Exception as e:
- ExceptionTool.raises_resp(f'json解析失败: {e}', self, JsonResolveFailException)
+ try:
+ data = JmcomicText.try_parse_json_object(self.resp.text)
+ if not isinstance(data, dict):
+ raise TypeError(f"Expected dict, got {type(data).__name__}")
+ return data
+ except (ValueError, TypeError) as e:
+ ExceptionTool.raises_resp(f'API json解析失败: {e}', self, JsonResolveFailException)Optionally add this import near the top to silence F405 for Dict:
from typing import Dict🧰 Tools
🪛 Ruff (0.12.2)
104-104: Comment contains ambiguous , (FULLWIDTH COMMA). Did you mean , (COMMA)?
(RUF003)
105-105: field_cache may be undefined, or defined from star imports
(F405)
106-106: Dict may be undefined, or defined from star imports
(F405)
108-108: JmcomicText may be undefined, or defined from star imports
(F405)
109-109: Do not catch blind exception: Exception
(BLE001)
110-110: ExceptionTool may be undefined, or defined from star imports
(F405)
110-110: JsonResolveFailException may be undefined, or defined from star imports
(F405)
🤖 Prompt for AI Agents
In src/jmcomic/jm_client_interface.py around lines 104 to 111, the json() method
currently catches all exceptions and may return None when
JmcomicText.try_parse_json_object() yields None; change it to avoid a bare
except and guarantee a Dict return by: import Dict from typing at the top if not
present, call try_parse_json_object and if the result is falsy or not a dict
raise JsonResolveFailException via ExceptionTool.raises_resp with an ASCII comma
in the message; only catch specific exceptions if needed (or catch Exception as
e and re-raise with raises_resp) so the method never silently returns None and
always returns a Dict on success.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/jmcomic/jm_client_impl.py (1)
893-900: Logic inverted inrequire_resp_status_ok: raises on success.This currently raises when
status == 'ok'. It should raise only when status is not OK.- if data.status == 'ok': - ExceptionTool.raises_resp(data.msg, resp) + if data.status != 'ok': + ExceptionTool.raises_resp(data.msg, resp)
♻️ Duplicate comments (2)
src/jmcomic/jm_toolkit.py (2)
64-66: Make JSON regex multiline-safe and fix ambiguous punctuation.
.*?won’t match newlines; multi-line JSON will fail to extract. Also replace fullwidth punctuation to satisfy Ruff.- # 提取api接口返回值里的json,防止返回值里有无关日志导致json解析报错 - pattern_api_response_json_object = compile(r'\{.*?}') + # 提取 API 接口返回值里的 JSON, 防止返回值里有无关日志导致 JSON 解析报错 + pattern_api_response_json_object = compile(r'\{[\s\S]*?\}')
349-365: Harden JSON extraction; avoid blindexceptand enforce object type.Current approach fails on multi-line/nested JSON and catches all exceptions (BLE001). Use
raw_decodefrom the first{, fall back to regex scan, and ensure the root is a JSON object.- @classmethod - def try_parse_json_object(cls, resp_text: str) -> dict: - import json - text = resp_text.strip() - if text.startswith('{') and text.endswith('}'): - # fast case - return json.loads(text) - - for match in cls.pattern_api_response_json_object.finditer(text): - try: - return json.loads(match.group(0)) - except Exception as e: - jm_log('parse_json_object.error', e) - - raise AssertionError(f'未解析出json数据: {cls.limit_text(resp_text, 200)}') + @classmethod + def try_parse_json_object(cls, resp_text: str) -> dict: + import json + text = resp_text.lstrip() + # Fast path: whole text is a JSON object (allow trailing whitespace) + if text.startswith('{') and text.rstrip().endswith('}'): + try: + obj = json.loads(text) + ExceptionTool.require_true(isinstance(obj, dict), f'JSON根对象必须为object, 实际为: {type(obj).__name__}') + return obj + except json.JSONDecodeError: + pass + + # Robust path: decode from first '{' + start = text.find('{') + if start != -1: + try: + obj, _ = json.JSONDecoder().raw_decode(text, idx=start) + ExceptionTool.require_true(isinstance(obj, dict), f'JSON根对象必须为object, 实际为: {type(obj).__name__}') + return obj + except json.JSONDecodeError as e: + # Fallback: scan candidates with regex (now multiline-safe) + for match in cls.pattern_api_response_json_object.finditer(text): + try: + candidate = json.loads(match.group(0)) + if isinstance(candidate, dict): + return candidate + except json.JSONDecodeError: + continue + jm_log('parse_json_object.error', e) + + raise AssertionError(f'未解析出json数据: {cls.limit_text(resp_text, 200)}')
🧹 Nitpick comments (1)
src/jmcomic/jm_client_impl.py (1)
996-1001: Trimmed error text is good; fix punctuation and reuse localtextvariable.Avoid fullwidth punctuation (RUF001) and prefer the already-fetched
textfor consistency.- f'请求不是json格式,强制重试!响应文本: [{JmcomicText.limit_text(resp.text, 200)}]' + f'请求不是 JSON 格式, 强制重试! 响应文本: [{JmcomicText.limit_text(text, 200)}]'
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/jmcomic/__init__.py(1 hunks)src/jmcomic/jm_client_impl.py(1 hunks)src/jmcomic/jm_toolkit.py(4 hunks)
✅ Files skipped from review due to trivial changes (1)
- src/jmcomic/init.py
🧰 Additional context used
🧬 Code graph analysis (2)
src/jmcomic/jm_client_impl.py (2)
src/jmcomic/jm_toolkit.py (2)
JmcomicText(6-368)limit_text(367-368)src/jmcomic/jm_client_interface.py (2)
text(33-34)JmResp(10-45)
src/jmcomic/jm_toolkit.py (3)
src/jmcomic/jm_client_interface.py (3)
json(88-92)json(106-110)text(33-34)src/jmcomic/jm_config.py (1)
jm_log(383-385)src/jmcomic/jm_exception.py (2)
JsonResolveFailException(46-47)ExceptionTool(69-191)
🪛 Ruff (0.12.2)
src/jmcomic/jm_client_impl.py
998-998: String contains ambiguous , (FULLWIDTH COMMA). Did you mean , (COMMA)?
(RUF001)
998-998: String contains ambiguous ! (FULLWIDTH EXCLAMATION MARK). Did you mean ! (EXCLAMATION MARK)?
(RUF001)
998-998: JmcomicText may be undefined, or defined from star imports
(F405)
src/jmcomic/jm_toolkit.py
64-64: Comment contains ambiguous , (FULLWIDTH COMMA). Did you mean , (COMMA)?
(RUF003)
361-361: Do not catch blind exception: Exception
(BLE001)
362-362: jm_log may be undefined, or defined from star imports
(F405)
364-364: Avoid specifying long messages outside the exception class
(TRY003)
🔇 Additional comments (2)
src/jmcomic/jm_toolkit.py (2)
477-479: Non-functional alignment/comment tweak looks good.No behavior change; keeps API/HTML adapters consistent.
495-497: LGTM on the minor formatting/comment change.Consistent with search-page adaptation.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
♻️ Duplicate comments (2)
src/jmcomic/jm_toolkit.py (2)
64-65: Make JSON regex multiline-safe and fix fullwidth punctuation.Use a DOTALL-safe pattern; current
.*?fails on multi-line JSON. Also replace the fullwidth comma to satisfy Ruff.Apply:
- # 提取api接口返回值里的json,防止返回值里有无关日志导致json解析报错 - pattern_api_response_json_object = compile(r'\{.*?}') + # 提取 api 接口返回值里的 JSON, 防止返回值里有无关日志导致 JSON 解析报错 + pattern_api_response_json_object = compile(r'\{[\s\S]*?}')
349-365: Harden JSON extraction; fix undefined jm_log; avoid blind except.Use JSONDecoder for balanced parsing, keep regex as fallback, and log via JmModuleConfig.jm_log. This also resolves BLE001 and F405.
- # noinspection PyTypeChecker - @classmethod - def try_parse_json_object(cls, resp_text: str) -> dict: - import json - text = resp_text.strip() - if text.startswith('{') and text.endswith('}'): - # fast case - return json.loads(text) - - for match in cls.pattern_api_response_json_object.finditer(text): - try: - return json.loads(match.group(0)) - except Exception as e: - jm_log('parse_json_object.error', e) - - raise AssertionError(f'未解析出json数据: {cls.limit_text(resp_text, 200)}') + # noinspection PyTypeChecker + @classmethod + def try_parse_json_object(cls, resp_text: str) -> dict: + import json + text = resp_text.lstrip() + + # Fast path: whole text is an object + if text.startswith('{') and text.rstrip().endswith('}'): + try: + obj = json.loads(text) + if isinstance(obj, dict): + return obj + except json.JSONDecodeError: + pass + + # Robust path: decode from first '{' + start = text.find('{') + if start != -1: + try: + obj, _ = json.JSONDecoder().raw_decode(text, idx=start) + if isinstance(obj, dict): + return obj + except json.JSONDecodeError: + pass + + # Fallback: scan candidates via regex + for match in cls.pattern_api_response_json_object.finditer(text): + try: + obj = json.loads(match.group(0)) + if isinstance(obj, dict): + return obj + except json.JSONDecodeError as e: + JmModuleConfig.jm_log('parse_json_object.error', f'{e}') + continue + + raise AssertionError(f'未解析出json数据: {cls.limit_text(resp_text, 200)}')
🧹 Nitpick comments (2)
src/jmcomic/jm_toolkit.py (1)
367-369: Fix off-by-one in limit_text (avoids appending “…(0)”).When len(text) == length the current code still appends suffix.
- return text if len(text) < length else (text[:length] + f'...({len(text[length:])})') + return text if len(text) <= length else (text[:length] + f'...({len(text) - length})')src/jmcomic/jm_client_interface.py (1)
104-111: Narrow exceptions and enforce Dict return in JmApiResp.json.Prevents silent type drift and addresses BLE001; clearer API error message.
- # 重写json()方法,可以忽略一些非json格式的脏数据 + # 重写json()方法, 可以忽略一些非 JSON 格式的脏数据 @field_cache() def json(self) -> Dict: - try: - return JmcomicText.try_parse_json_object(self.resp.text) - except Exception as e: - ExceptionTool.raises_resp(f'json解析失败: {e}', self, JsonResolveFailException) + import json + try: + data = JmcomicText.try_parse_json_object(self.resp.text) + if not isinstance(data, dict): + raise TypeError(f"Expected dict, got {type(data).__name__}") + return data + except (AssertionError, ValueError, TypeError, json.JSONDecodeError) as e: + ExceptionTool.raises_resp(f'API json解析失败: {e}', self, JsonResolveFailException)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/jmcomic/jm_client_interface.py(1 hunks)src/jmcomic/jm_toolkit.py(4 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
src/jmcomic/jm_client_interface.py (2)
src/jmcomic/jm_toolkit.py (1)
try_parse_json_object(351-364)src/jmcomic/jm_exception.py (4)
resp(23-24)resp(31-35)raises_resp(125-135)JsonResolveFailException(46-47)
src/jmcomic/jm_toolkit.py (5)
src/jmcomic/jm_client_interface.py (3)
json(88-92)json(106-110)text(33-34)src/jmcomic/jm_config.py (1)
jm_log(383-385)src/jmcomic/jm_entity.py (2)
title(71-72)tags(346-356)src/jmcomic/jm_client_impl.py (1)
check_special_text(554-569)src/jmcomic/jm_exception.py (2)
JsonResolveFailException(46-47)ExceptionTool(69-191)
🪛 Ruff (0.12.2)
src/jmcomic/jm_client_interface.py
104-104: Comment contains ambiguous , (FULLWIDTH COMMA). Did you mean , (COMMA)?
(RUF003)
105-105: field_cache may be undefined, or defined from star imports
(F405)
106-106: Dict may be undefined, or defined from star imports
(F405)
108-108: JmcomicText may be undefined, or defined from star imports
(F405)
109-109: Do not catch blind exception: Exception
(BLE001)
110-110: ExceptionTool may be undefined, or defined from star imports
(F405)
110-110: JsonResolveFailException may be undefined, or defined from star imports
(F405)
src/jmcomic/jm_toolkit.py
64-64: Comment contains ambiguous , (FULLWIDTH COMMA). Did you mean , (COMMA)?
(RUF003)
361-361: Do not catch blind exception: Exception
(BLE001)
362-362: jm_log may be undefined, or defined from star imports
(F405)
364-364: Avoid specifying long messages outside the exception class
(TRY003)
🔇 Additional comments (2)
src/jmcomic/jm_toolkit.py (2)
476-477: LGTM: normalize to name/tags for API-page compatibility.
491-492: LGTM: same normalization in category parser.
Summary by CodeRabbit
New Features
Improvements
Chores