fix: support SOCKS proxies in updater requests

[zouyonghe]

Fixes #7584

Summary

The updater started failing in environments that expose a global socks5:// proxy through system environment variables. Users could no longer fetch release metadata, and the update flow would surface Server disconnected errors in the console.

Root Cause

RepoZipUpdator.fetch_release_info() was using aiohttp.ClientSession(trust_env=True). That makes aiohttp read system proxy variables, but the updater path did not use a SOCKS-capable client. When the environment contained a socks5:// proxy, the request path broke before release metadata could be parsed.

The same updater chain also reused an aiohttp-based download helper for release archives, which left the actual update download path exposed to the same proxy compatibility problem.

Fix

This change keeps the updater API unchanged and limits the behavior change to the updater chain:

• switch release metadata requests in RepoZipUpdator to httpx.AsyncClient(trust_env=True)
• add an internal streamed download helper in RepoZipUpdator that also uses httpx
• reuse that helper for AstrBot core updates and repository zip downloads
• keep the existing response parsing and error surface for callers unchanged

httpx[socks] is already declared by the project, so this fix does not introduce a new dependency just to support SOCKS proxies.

Validation

I verified the change with:

• uv run pytest tests/test_updator_socks.py tests/test_dashboard.py::test_check_update tests/test_dashboard.py::test_do_update
• uv run ruff format .
• uv run ruff check .

The new regression tests cover both release metadata fetching and zip download behavior through the updater path.

Summary by Sourcery

Switch updater network operations to an HTTPX-based client to support SOCKS-aware proxy handling while preserving the existing updater API and behavior for callers.

New Features:

• Enable SOCKS proxy support for updater release metadata and archive downloads by using an HTTPX async client that honors environment proxy settings.

Enhancements:

• Introduce a shared HTTPX async client factory in RepoZipUpdator with configurable certificate verification settings.
• Replace aiohttp-based release metadata fetching and repo zip downloads with HTTPX, improving error logging with status codes and truncated response bodies.
• Ensure partial download artifacts are cleaned up on failures and log the failing URL and target path for easier debugging.

Tests:

• Add regression tests covering HTTPX client configuration, SOCKS-proxy-aware release info fetching, HTTPX-based zip downloads, error logging on HTTP failures, and cleanup behavior on interrupted download streams.

[sourcery-ai]

Hey - I've found 1 issue

Prompt for AI Agents

Please address the comments from this code review:

## Individual Comments

### Comment 1
<location path="astrbot/core/zip_updator.py" line_range="67-70" />
<code_context>
-                    )
-                    raise Exception(f"请求失败，状态码: {response.status}")
-                result = await response.json()
+            async with self._create_httpx_client() as client:
+                response = await client.get(url)
+                response.raise_for_status()
+                result = response.json()
             if not result:
                 return []
</code_context>
<issue_to_address>
**suggestion:** The new httpx-based implementation drops the detailed error logging of non-200 responses.

With `response.raise_for_status()` we lose the previous logging of status code and body for non-200 responses, which reduces debuggability for API/proxy issues. Please catch `httpx.HTTPStatusError`, log `exc.response.status_code` and `exc.response.text` (possibly truncated), then re-raise or wrap the exception so callers still get useful diagnostic information.

Suggested implementation:

```python
            async with self._create_httpx_client() as client:
                try:
                    response = await client.get(url)
                    response.raise_for_status()
                except httpx.HTTPStatusError as exc:
                    # 记录详细的状态码和响应内容（截断以防止日志过长）
                    body = exc.response.text or ""
                    max_len = 1000
                    if len(body) > max_len:
                        body = body[:max_len] + "...[truncated]"
                    logger.error(
                        "请求失败，状态码: %s，响应内容: %s",
                        exc.response.status_code,
                        body,
                    )
                    raise
                result = response.json()
            if not result:
                return []

```

1. At the top of `astrbot/core/zip_updator.py`, ensure `httpx` is imported, e.g.:
   `import httpx`
2. Ensure `logger` is defined/imported in this module (for example, via `import logging` and `logger = logging.getLogger(__name__)` or using whatever logging convention the rest of the project uses).
</issue_to_address>

---

Sourcery is free for open source - if you like our reviews please consider sharing them ✨

• X
• Mastodon
• LinkedIn
• Facebook

_{Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.}

[gemini-code-assist]

Code Review

This pull request migrates the update system from aiohttp to httpx to better support environment-configured proxies and streamline SSL verification using certifi. The changes refactor file downloading and release information fetching into a more centralized and testable structure, supported by a new suite of unit tests. A recommendation was made to utilize the existing github_api_release_parser helper method to avoid manual response parsing and improve code maintainability.

[gemini-code-assist]

The current implementation of fetch_release_info manually parses the JSON response into a list of dictionaries. However, there is already a helper method github_api_release_parser (defined at line 93) that performs this exact transformation. Reusing the existing method would reduce code duplication and improve maintainability.

Suggested change

	async with self._create_httpx_client() as client:
	response = await client.get(url)
	response.raise_for_status()
	result = response.json()
	async with self._create_httpx_client() as client:
	response = await client.get(url)
	response.raise_for_status()
	result = response.json()
	if not result:
	return []
	ret = self.github_api_release_parser(result if isinstance(result, list) else [result])

[zouyonghe]

@sourcery-ai review

[sourcery-ai]

Hey - I've found 1 issue

Prompt for AI Agents

Please address the comments from this code review:

## Individual Comments

### Comment 1
<location path="astrbot/core/zip_updator.py" line_range="55-64" />
<code_context>
+            return body
+        return body[:max_len] + "...[truncated]"
+
+    async def _download_file(
+        self, url: str, path: str, timeout: float = 1800.0
+    ) -> None:
+        target_path = Path(path)
+        target_path.parent.mkdir(parents=True, exist_ok=True)
+
+        async with self._create_httpx_client(timeout=timeout) as client:
+            async with client.stream("GET", url) as response:
+                response.raise_for_status()
+                with target_path.open("wb") as file:
+                    async for chunk in response.aiter_bytes(8192):
+                        file.write(chunk)
+
     async def fetch_release_info(self, url: str, latest: bool = True) -> list:
</code_context>
<issue_to_address>
**suggestion:** Consider handling partial files when download fails partway through.

Currently, if an error occurs while streaming, a partially written file may remain on disk. Since this class already tracks `rm_on_error`, you could wrap the streaming/writing block in a `try`/`except` and, on exception, unlink `target_path` when `rm_on_error` is set so callers don’t mistake an incomplete file for a valid one.
</issue_to_address>

---

Sourcery is free for open source - if you like our reviews please consider sharing them ✨

• X
• Mastodon
• LinkedIn
• Facebook

_{Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.}

[zouyonghe]

@sourcery-ai review

[sourcery-ai]

Hey - I've found 1 issue

Prompt for AI Agents

Please address the comments from this code review:

## Individual Comments

### Comment 1
<location path="tests/test_updator_socks.py" line_range="135-144" />
<code_context>
+async def test_fetch_release_info_uses_httpx_client_with_env_proxy_support(
</code_context>
<issue_to_address>
**suggestion (testing):** Consider asserting all relevant httpx client options (e.g. follow_redirects and timeout) to fully lock in the new client configuration.

The test already covers `trust_env` and `verify`. Since `_create_httpx_client` also sets `follow_redirects=True` and a specific `timeout`, consider asserting those too (if they’re important to the bugfix) so refactors can’t drop them without breaking tests.

Suggested implementation:

```python
    import astrbot.core.zip_updator as zip_updator_module
    import httpx

```

```python
    # existing expectations about the httpx client configuration
    assert _FakeAsyncClient.init_kwargs["trust_env"] is True
    assert _FakeAsyncClient.init_kwargs["verify"] is False

    # new expectations to fully lock in the httpx client configuration
    assert _FakeAsyncClient.init_kwargs["follow_redirects"] is True

    timeout = _FakeAsyncClient.init_kwargs["timeout"]
    # Ensure a timeout is explicitly configured and is an httpx.Timeout instance.
    # This guards against future refactors that drop or change the timeout type.
    assert isinstance(timeout, httpx.Timeout)

```

If `_create_httpx_client` uses a specific timeout value or a module-level constant (for example, `zip_updator_module.HTTP_TIMEOUT_SECONDS` or similar), you may want to strengthen the timeout assertion to check the actual value, e.g.:

`assert timeout.read == zip_updator_module.HTTP_TIMEOUT_SECONDS`

Adjust the assertion to match the real constant or numeric value used in `astrbot.core.zip_updator._create_httpx_client`.
</issue_to_address>

---

Sourcery is free for open source - if you like our reviews please consider sharing them ✨

• X
• Mastodon
• LinkedIn
• Facebook

_{Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.}

[SourceryAI]

Hey - I've found 1 issue

Prompt for AI Agents

Please address the comments from this code review:

## Individual Comments

### Comment 1
<location path="tests/test_updator_socks.py" line_range="240" />
<code_context>
+
+
+@pytest.mark.asyncio
+async def test_fetch_release_info_logs_status_code_and_truncated_body_on_http_error(
+    monkeypatch: pytest.MonkeyPatch,
+) -> None:
</code_context>
<issue_to_address>
**suggestion (testing):** Strengthen the truncation behavior assertion for HTTP error logging.

Right now the test only asserts that `"...[truncated]"` appears, which confirms truncation occurs but not that we limit the logged body to `max_len`. To better lock in the expected behavior, also assert that the substring before `...[truncated]` has the expected length (e.g., 1000 when the input body exceeds 1000), matching the `_truncate_response_body` contract and making future regressions more visible.

Suggested implementation:

```python
    # Ensure truncation marker is present.
    marker = "...[truncated]"
    assert marker in caplog.text

    # Extract the portion of the log line that precedes the marker.
    body_prefix = caplog.text.split(marker)[0]

    # The log line may contain prefix text (e.g. status code, labels) before the body.
    # We only care that the *final* chunk of text before the marker is limited to the
    # configured maximum length.
    expected_max_len = 1000
    logged_body_tail = body_prefix[-expected_max_len:]

    # The body is longer than expected_max_len in this test setup, so we should see a
    # tail of exactly expected_max_len characters logged before the truncation marker.
    assert len(logged_body_tail) == expected_max_len

```

I only see part of the test file, so you may need to:

1. Adjust the `SEARCH` block to match the exact line currently asserting truncation (e.g., `assert "...[truncated]" in caplog.text` or a slightly different variant).
2. Ensure the test fixture constructs a response body longer than `expected_max_len` (1000) so that the strengthened assertion is meaningful.
3. If the maximum length constant is defined elsewhere (e.g., `_MAX_LOG_BODY_LEN` or similar), you may want to import and use that constant instead of the hard-coded `1000` to keep the test in sync with production behavior.
</issue_to_address>

---

Hi @zouyonghe! 👋

Thanks for trying out Sourcery by commenting with @sourcery-ai review! 🚀

Install the sourcery-ai bot to get automatic code reviews on every pull request ✨

_{Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.}

[SourceryAI]

suggestion (testing): Strengthen the truncation behavior assertion for HTTP error logging.

Right now the test only asserts that "...[truncated]" appears, which confirms truncation occurs but not that we limit the logged body to max_len. To better lock in the expected behavior, also assert that the substring before ...[truncated] has the expected length (e.g., 1000 when the input body exceeds 1000), matching the _truncate_response_body contract and making future regressions more visible.

Suggested implementation:

    # Ensure truncation marker is present.
    marker = "...[truncated]"
    assert marker in caplog.text

    # Extract the portion of the log line that precedes the marker.
    body_prefix = caplog.text.split(marker)[0]

    # The log line may contain prefix text (e.g. status code, labels) before the body.
    # We only care that the *final* chunk of text before the marker is limited to the
    # configured maximum length.
    expected_max_len = 1000
    logged_body_tail = body_prefix[-expected_max_len:]

    # The body is longer than expected_max_len in this test setup, so we should see a
    # tail of exactly expected_max_len characters logged before the truncation marker.
    assert len(logged_body_tail) == expected_max_len

I only see part of the test file, so you may need to:

1. Adjust the SEARCH block to match the exact line currently asserting truncation (e.g., assert "...[truncated]" in caplog.text or a slightly different variant).
2. Ensure the test fixture constructs a response body longer than expected_max_len (1000) so that the strengthened assertion is meaningful.
3. If the maximum length constant is defined elsewhere (e.g., _MAX_LOG_BODY_LEN or similar), you may want to import and use that constant instead of the hard-coded 1000 to keep the test in sync with production behavior.

[zouyonghe]

@sourcery-ai review

[sourcery-ai]

Hey - I've left some high level feedback:

• In _download_file, consider logging the URL and target path before re-raising on exceptions so that failures in streamed downloads are easier to diagnose in real environments.
• The HTTPX client factory _create_httpx_client is currently hard-coded to use certifi.where() for verify; if you foresee self-hosted or custom CA setups, exposing verify (or the entire client factory) as a configurable dependency would make the updater more flexible.
• The fake HTTPX client in tests relies on mutable class-level state and manual _reset_fake_client calls; using per-test fixtures that construct fresh instances or store state on the instance would make the tests more robust against interference when a test fails early.

Prompt for AI Agents

Please address the comments from this code review:

## Overall Comments
- In `_download_file`, consider logging the URL and target path before re-raising on exceptions so that failures in streamed downloads are easier to diagnose in real environments.
- The HTTPX client factory `_create_httpx_client` is currently hard-coded to use `certifi.where()` for `verify`; if you foresee self-hosted or custom CA setups, exposing `verify` (or the entire client factory) as a configurable dependency would make the updater more flexible.
- The fake HTTPX client in tests relies on mutable class-level state and manual `_reset_fake_client` calls; using per-test fixtures that construct fresh instances or store state on the instance would make the tests more robust against interference when a test fails early.

---

Sourcery is free for open source - if you like our reviews please consider sharing them ✨

• X
• Mastodon
• LinkedIn
• Facebook

_{Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.}

[zouyonghe]

Addressed the overall review feedback in 00fac0d1.

• _download_file() now logs the source URL and target path before cleaning up partial files and re-raising.
• RepoZipUpdator now accepts an overridable verify setting for the HTTPX client, and that override is passed through the concrete updater subclasses as well.
• The HTTPX test doubles in tests/test_updator_socks.py were refactored to use per-test state fixtures instead of class-level mutable state and manual reset helpers.

Validation used:

• uv run pytest tests/test_updator_socks.py tests/test_dashboard.py::test_check_update tests/test_dashboard.py::test_do_update
• uv run ruff format .
• uv run ruff check .

[zouyonghe]

@sourcery-ai review

[sourcery-ai]

Hey - I've reviewed your changes and they look great!

---

Sourcery is free for open source - if you like our reviews please consider sharing them ✨

• X
• Mastodon
• LinkedIn
• Facebook

_{Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.}