Skip to content

Conversation

@hect0x7
Copy link
Owner

@hect0x7 hect0x7 commented Sep 6, 2025

Summary by CodeRabbit

  • New Features

    • Advanced domain-aware retry with per-domain failure tracking and configurable limits.
    • Test configuration entry to enable the advanced retry behavior.
  • Improvements

    • More robust API response parsing to extract/sanitize embedded JSON.
    • Shorter, truncated response snippets in error messages to reduce log verbosity.
  • Chores

    • Enabled automatic release notes generation and upgraded release workflow action.
    • Minor type-annotation refinement and package version bump.

@coderabbitai
Copy link

coderabbitai bot commented Sep 6, 2025

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 @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

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.

📥 Commits

Reviewing files that changed from the base of the PR and between 0913403 and 650d271.

📒 Files selected for processing (4)
  • src/jmcomic/__init__.py (1 hunks)
  • src/jmcomic/jm_client_impl.py (2 hunks)
  • src/jmcomic/jm_client_interface.py (1 hunks)
  • src/jmcomic/jm_toolkit.py (4 hunks)

Walkthrough

Adds 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

Cohort / File(s) Summary
Advanced retry plugin
src/jmcomic/jm_plugin.py
Adds AdvancedRetryPlugin with __init__, invoke, new_jm_client hook, request_with_retry (domain-resolving, multi-round retries, exception logging, fallback), get_sorted_domain, update_failed_count, and failed_count. Introduces per-client lock(s) and per-domain failure counters; imports Lock locally.
API response parsing — client
src/jmcomic/jm_client_interface.py
Adds JmApiResp.json() that sanitizes/parses API response text via JmcomicText.try_parse_json_object and caches result; raises JsonResolveFailException on parse failure.
API response parsing — toolkit
src/jmcomic/jm_toolkit.py
Adds pattern_api_response_json_object, JmcomicText.try_parse_json_object(resp_text: str) -> dict, and limit_text; logs parse errors and raises when no JSON found. Minor formatting tweaks in HTML parsers.
Minor client annotation
src/jmcomic/jm_option.py
Refines a local variable type annotation in new_jm_client from AbstractJmClient to JmcomicClient (annotation-only change).
Client retry message truncation
src/jmcomic/jm_client_impl.py
When reporting non-JSON responses, truncates resp.text in error messages using JmcomicText.limit_text(resp.text, 200).
Package version
src/jmcomic/__init__.py
Bumps __version__ from 2.6.6 to 2.6.7.
Options / tests
assets/option/option_test_api.yml
Registers advanced-retry plugin under plugins.after_init with retry_config (retry_rounds: 3, retry_domain_max_times: 5) and minor formatting adjustments.
Release config
.github/release.yml
Adds changelog/release notes configuration with categories and exclusion rules.
Release workflow
.github/workflows/release_auto.yml
Updates Create Release step to use softprops/action-gh-release@v2 and enables generate_release_notes: true.

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
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related PRs

"I hopped through logs with whiskers bright,
Counting domains in morning light.
I tracked each stumble, counted the tries,
Bounced back thrice with steady eyes.
— 🐇✨"

Pre-merge checks (2 passed, 1 warning)

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 19.05% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Title Check ✅ Passed The title “增强域名重试机制” directly and succinctly captures the primary enhancement introduced in this pull request, namely the domain retry mechanism improvements, without extraneous details or file lists.
Description Check ✅ Passed The pull request description indicates it enhances the domain retry mechanism, which aligns directly with the substantive changes in the AdvancedRetryPlugin and related configuration updates.
✨ Finishing Touches
  • 📝 Generate Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch dev

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link

@coderabbitai coderabbitai bot left a 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 for JmcomicClient to avoid F405 and potential runtime evaluation.

  • Using JmcomicClient here 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 JmcomicClient
src/jmcomic/jm_plugin.py (3)

1237-1242: Prefer defaultdict(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-provided kwargs in-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

📥 Commits

Reviewing files that changed from the base of the PR and between 49c4891 and 6cf0d74.

📒 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_config on the instance is appropriate.


1291-1297: LGTM.

Sorting by failure count and filtering by times is 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.

Comment on lines +1231 to +1233
self.require_param(isinstance(retry_config, dict), '必须配置retry_config为dict')
self.retry_config = retry_config

Copy link

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.

Suggested change
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.

Comment on lines +1260 to +1274
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

Copy link

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.

Suggested change
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.

Copy link

@coderabbitai coderabbitai bot left a 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 Callable
assets/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

📥 Commits

Reviewing files that changed from the base of the PR and between 6cf0d74 and 32059dd.

📒 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)

Comment on lines +30 to +34
- plugin: advanced-retry
kwargs:
retry_config:
retry_rounds: 3 # 一共对域名列表重试3轮
retry_domain_max_times: 5 # 当一个域名重试次数超过5次,忽略该域名,不再重试
Copy link

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/jmcomic

Length 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")
EOF

Length 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.

Copy link

@coderabbitai coderabbitai bot left a 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: LGTM

v2 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 behavior

If 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: true

Option 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

📥 Commits

Reviewing files that changed from the base of the PR and between a1b9597 and 3355c33.

📒 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响应的清理
Copy link

@coderabbitai coderabbitai bot left a 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

📥 Commits

Reviewing files that changed from the base of the PR and between 3355c33 and d39cc8b.

📒 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).

Comment on lines 104 to 114
# 重写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)

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

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).

@hect0x7
Copy link
Owner Author

hect0x7 commented Sep 9, 2025

@coderabbitai review latest commit

@coderabbitai
Copy link

coderabbitai bot commented Sep 9, 2025

@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 performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

Copy link

@coderabbitai coderabbitai bot left a 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

📥 Commits

Reviewing files that changed from the base of the PR and between d39cc8b and e695d72.

📒 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.

Copy link

@coderabbitai coderabbitai bot left a 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_json unguarded 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

📥 Commits

Reviewing files that changed from the base of the PR and between d39cc8b and e695d72.

📒 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)

Comment on lines 349 to 360
# 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))

Copy link

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 returns None silently when no match.
  • Use json.JSONDecoder().raw_decode from 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.

Suggested change
# 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.

Copy link

@coderabbitai coderabbitai bot left a 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

📥 Commits

Reviewing files that changed from the base of the PR and between 432bb79 and f2245cf.

📒 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)

Comment on lines 104 to 111
# 重写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)

Copy link

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.

Copy link

@coderabbitai coderabbitai bot left a 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 in require_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 blind except and enforce object type.

Current approach fails on multi-line/nested JSON and catches all exceptions (BLE001). Use raw_decode from 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 local text variable.

Avoid fullwidth punctuation (RUF001) and prefer the already-fetched text for 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

📥 Commits

Reviewing files that changed from the base of the PR and between f2245cf and 34715ba.

📒 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.

Copy link

@coderabbitai coderabbitai bot left a 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

📥 Commits

Reviewing files that changed from the base of the PR and between 34715ba and f65903f.

📒 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.

@hect0x7 hect0x7 merged commit 7943b9b into master Sep 9, 2025
4 checks passed
@hect0x7 hect0x7 deleted the dev branch September 9, 2025 16:04
@coderabbitai coderabbitai bot mentioned this pull request Sep 21, 2025
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants