fix: bundle httpx SOCKS proxy support#7093
Merged
zouyonghe merged 4 commits intoAstrBotDevs:masterfrom Mar 28, 2026
Merged
Conversation
dosubot
bot
added
the
size:M
2 tasks
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Hey - I've found 2 issues
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location path="tests/test_httpx_socks_dependency.py" line_range="25-28" />
<code_context>
+ return entries
+
+
+def _read_pyproject_dependencies() -> list[str]:
+ with PYPROJECT_PATH.open("rb") as file:
+ pyproject = tomllib.load(file)
+ return pyproject["project"]["dependencies"]
+
+
</code_context>
<issue_to_address>
**issue (testing):** The test only checks presence in `project.dependencies`, not alignment between `requirements.txt` and `pyproject.toml` (e.g. version specifiers).
Right now the tests only assert that each file independently contains `httpx[socks]`. A change like `httpx[socks]==1.0.0` in one file and `httpx[socks]>=1.0.0` in the other would still pass. Please add a test that compares the `httpx[socks]` spec from both files and asserts they are identical (or otherwise compatible under the alignment rule you intend to enforce).
</issue_to_address>
### Comment 2
<location path="tests/test_httpx_socks_dependency.py" line_range="9-18" />
<code_context>
+HTTPX_SOCKS_PATTERN = re.compile(r"^httpx\[socks\](?:\s*[<>=!~].*)?$")
</code_context>
<issue_to_address>
**suggestion (testing):** Add explicit assertion messages so failures clearly explain that SOCKS proxy support is misconfigured.
For example, instead of bare `assert` statements, use something like:
```python
assert _contains_httpx_socks_dependency(...), (
"Expected httpx[socks] dependency for SOCKS proxy support"
)
```
This makes it clear that a missing or renamed dependency is what broke the test, which helps future contributors quickly understand and fix failures.
Suggested implementation:
```python
assert _contains_httpx_socks_dependency(
requirements
), "Expected httpx[socks] dependency in requirements.txt for SOCKS proxy support"
```
```python
assert _contains_httpx_socks_dependency(
pyproject_requirements
), "Expected httpx[socks] dependency in pyproject.toml for SOCKS proxy support"
```
I only see the top of the file, so I assumed test code of the form:
- `assert _contains_httpx_socks_dependency(requirements)`
- `assert _contains_httpx_socks_dependency(pyproject_requirements)`
Please update every bare `assert _contains_httpx_socks_dependency(...)` in this test module to include an explicit message that mentions:
1. That `httpx[socks]` is expected, and
2. Which source (e.g. `requirements.txt` or `pyproject.toml`) is being checked.
If there are differently named variables or additional tests, mirror the pattern above, tailoring the message to the specific dependency source being asserted.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Comment on lines
+25
to
+28
| def _read_pyproject_dependencies() -> list[str]: | ||
| with PYPROJECT_PATH.open("rb") as file: | ||
| pyproject = tomllib.load(file) | ||
| return pyproject["project"]["dependencies"] |
Contributor
There was a problem hiding this comment.
issue (testing): The test only checks presence in project.dependencies, not alignment between requirements.txt and pyproject.toml (e.g. version specifiers).
Right now the tests only assert that each file independently contains httpx[socks]. A change like httpx[socks]==1.0.0 in one file and httpx[socks]>=1.0.0 in the other would still pass. Please add a test that compares the httpx[socks] spec from both files and asserts they are identical (or otherwise compatible under the alignment rule you intend to enforce).
Comment on lines
+9
to
+18
| HTTPX_SOCKS_PATTERN = re.compile(r"^httpx\[socks\](?:\s*[<>=!~].*)?$") | ||
|
|
||
|
|
||
| def _contains_httpx_socks_dependency(entries: list[str]) -> bool: | ||
| return any(HTTPX_SOCKS_PATTERN.match(entry.strip()) for entry in entries) | ||
|
|
||
|
|
||
| def _read_requirements() -> list[str]: | ||
| entries = [] | ||
| for line in REQUIREMENTS_PATH.read_text(encoding="utf-8").splitlines(): |
Contributor
There was a problem hiding this comment.
suggestion (testing): Add explicit assertion messages so failures clearly explain that SOCKS proxy support is misconfigured.
For example, instead of bare assert statements, use something like:
assert _contains_httpx_socks_dependency(...), (
"Expected httpx[socks] dependency for SOCKS proxy support"
)This makes it clear that a missing or renamed dependency is what broke the test, which helps future contributors quickly understand and fix failures.
Suggested implementation:
assert _contains_httpx_socks_dependency(
requirements
), "Expected httpx[socks] dependency in requirements.txt for SOCKS proxy support" assert _contains_httpx_socks_dependency(
pyproject_requirements
), "Expected httpx[socks] dependency in pyproject.toml for SOCKS proxy support"I only see the top of the file, so I assumed test code of the form:
assert _contains_httpx_socks_dependency(requirements)assert _contains_httpx_socks_dependency(pyproject_requirements)
Please update every bare assert _contains_httpx_socks_dependency(...) in this test module to include an explicit message that mentions:
- That
httpx[socks]is expected, and - Which source (e.g.
requirements.txtorpyproject.toml) is being checked.
If there are differently named variables or additional tests, mirror the pattern above, tailoring the message to the specific dependency source being asserted.
dosubot
bot
added
the
area:core
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Code Review
This pull request adds the httpx[socks] dependency to both pyproject.toml and requirements.txt. It also introduces a new test file, tests/test_httpx_socks_dependency.py, which verifies that the dependency is correctly declared in both configuration files. I have no feedback to provide as there were no review comments to evaluate.
Member
Author
|
Addressed the review feedback in
|
Member
Author
|
@sourcery-ai review |
Contributor
There was a problem hiding this comment.
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_httpx_socks_dependency.py" line_range="9" />
<code_context>
+PROJECT_ROOT = Path(__file__).resolve().parents[1]
+REQUIREMENTS_PATH = PROJECT_ROOT / "requirements.txt"
+PYPROJECT_PATH = PROJECT_ROOT / "pyproject.toml"
+HTTPX_SOCKS_PATTERN = re.compile(r"^httpx\[socks\](?:\s*[<>=!~].*)?$")
+
+
</code_context>
<issue_to_address>
**suggestion:** Regex is too strict to allow future use of environment markers in dependency specs.
The current `HTTPX_SOCKS_PATTERN` only matches optional version specifiers and then end-of-line, so something like `httpx[socks]; python_version >= "3.11"` would be treated as invalid even though it’s a valid dependency form. Relax the pattern to allow a `;` and trailing environment markers (e.g. `r"^httpx\[socks\](?:\s*[<>=!~].*)?(?:\s*;.*)?$"`) and add/adjust a test to ensure such forms are accepted.
Suggested implementation:
```python
HTTPX_SOCKS_PATTERN = re.compile(
r"^httpx\[socks\](?:\s*[<>=!~].*)?(?:\s*;.*)?$"
)
```
```python
def test_httpx_socks_pattern_allows_environment_markers():
entry = 'httpx[socks]; python_version >= "3.11"'
assert HTTPX_SOCKS_PATTERN.match(entry)
def _read_httpx_socks_dependency(entries: list[str]) -> str | None:
```
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Member
Author
|
Addressed the latest review in d0431a1:
|
Member
Author
|
@sourcery-ai review |
Contributor
There was a problem hiding this comment.
Hey - I've found 2 issues
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location path="tests/test_httpx_socks_dependency.py" line_range="9-12" />
<code_context>
+PROJECT_ROOT = Path(__file__).resolve().parents[1]
+REQUIREMENTS_PATH = PROJECT_ROOT / "requirements.txt"
+PYPROJECT_PATH = PROJECT_ROOT / "pyproject.toml"
+HTTPX_SOCKS_PATTERN = re.compile(r"^httpx\[socks\](?:\s*[<>=!~][^;]*)?(?:\s*;.*)?$")
+
+
</code_context>
<issue_to_address>
**suggestion (testing):** Add negative tests to ensure the pattern does not match non-SOCKS httpx dependencies or malformed entries.
Currently we only verify that `HTTPX_SOCKS_PATTERN` matches a valid `httpx[socks]` requirement. Please also add assertions that it does *not* match invalid cases such as `httpx`, `httpx[http2]`, `httpx[socks-extra]`, and entries with extra leading/trailing text, so future regex changes that are too broad are caught by tests.
```suggestion
HTTPX_SOCKS_PATTERN = re.compile(r"^httpx\[socks\](?:\s*[<>=!~][^;]*)?(?:\s*;.*)?$")
def test_httpx_socks_pattern_matches_valid_entries() -> None:
valid_entries = [
"httpx[socks]",
"httpx[socks]==0.27.0",
"httpx[socks]>=0.27.0,<0.28.0",
'httpx[socks]>=0.27.0 ; python_version < "3.13"',
'httpx[socks] ; python_version < "3.13"',
]
for entry in valid_entries:
match = HTTPX_SOCKS_PATTERN.match(entry)
assert match is not None, f"Expected pattern to match valid entry: {entry}"
assert match.group(0) == entry
def test_httpx_socks_pattern_does_not_match_invalid_entries() -> None:
invalid_entries = [
"httpx",
"httpx==0.27.0",
"httpx[http2]",
"httpx[socks-extra]",
"httpx [socks]",
"someprefix httpx[socks]",
"httpx[socks] trailing-text",
"httpx[socks] extra ; markers",
"httpx[socks]andmore",
]
for entry in invalid_entries:
assert HTTPX_SOCKS_PATTERN.match(entry) is None, (
f"Expected pattern not to match invalid entry: {entry}"
)
def _read_httpx_socks_dependency(entries: list[str]) -> str | None:
```
</issue_to_address>
### Comment 2
<location path="tests/test_httpx_socks_dependency.py" line_range="67-70" />
<code_context>
+ )
+
+
+def test_httpx_socks_pattern_allows_environment_markers() -> None:
+ entry = 'httpx[socks]; python_version >= "3.11"'
+
+ assert HTTPX_SOCKS_PATTERN.match(entry), (
+ "Expected httpx[socks] dependency pattern to allow environment markers "
+ "for SOCKS proxy support"
</code_context>
<issue_to_address>
**suggestion (testing):** Extend the pattern tests to cover version specifiers and minor formatting variations.
Consider parametrizing this test with additional valid patterns, such as:
- `httpx[socks]==0.28.1`
- `httpx[socks]>=0.27,<0.29`
- `httpx[socks] >=0.27 ; python_version < "3.13"`
This would improve coverage for version specifiers and whitespace variations so future changes to `requirements.txt`/`pyproject.toml` remain compatible with the regex.
Suggested implementation:
```python
import pytest
@pytest.mark.parametrize(
"entry",
[
'httpx[socks]; python_version >= "3.11"',
"httpx[socks]==0.28.1",
"httpx[socks]>=0.27,<0.29",
'httpx[socks] >=0.27 ; python_version < "3.13"',
],
)
def test_httpx_socks_pattern_matches_valid_variants(entry: str) -> None:
assert HTTPX_SOCKS_PATTERN.match(entry), (
"Expected httpx[socks] dependency pattern to allow environment markers, "
"version specifiers, and whitespace variations for SOCKS proxy support"
)
```
If `pytest` is already imported elsewhere in this file, remove the new `import pytest` line from the replacement block to avoid duplicate imports. Otherwise, keep it so that `@pytest.mark.parametrize` works correctly.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Comment on lines
+9
to
+12
| HTTPX_SOCKS_PATTERN = re.compile(r"^httpx\[socks\](?:\s*[<>=!~][^;]*)?(?:\s*;.*)?$") | ||
|
|
||
|
|
||
| def _read_httpx_socks_dependency(entries: list[str]) -> str | None: |
Contributor
There was a problem hiding this comment.
suggestion (testing): Add negative tests to ensure the pattern does not match non-SOCKS httpx dependencies or malformed entries.
Currently we only verify that HTTPX_SOCKS_PATTERN matches a valid httpx[socks] requirement. Please also add assertions that it does not match invalid cases such as httpx, httpx[http2], httpx[socks-extra], and entries with extra leading/trailing text, so future regex changes that are too broad are caught by tests.
Suggested change
| HTTPX_SOCKS_PATTERN = re.compile(r"^httpx\[socks\](?:\s*[<>=!~][^;]*)?(?:\s*;.*)?$") | |
| def _read_httpx_socks_dependency(entries: list[str]) -> str | None: | |
| HTTPX_SOCKS_PATTERN = re.compile(r"^httpx\[socks\](?:\s*[<>=!~][^;]*)?(?:\s*;.*)?$") | |
| def test_httpx_socks_pattern_matches_valid_entries() -> None: | |
| valid_entries = [ | |
| "httpx[socks]", | |
| "httpx[socks]==0.27.0", | |
| "httpx[socks]>=0.27.0,<0.28.0", | |
| 'httpx[socks]>=0.27.0 ; python_version < "3.13"', | |
| 'httpx[socks] ; python_version < "3.13"', | |
| ] | |
| for entry in valid_entries: | |
| match = HTTPX_SOCKS_PATTERN.match(entry) | |
| assert match is not None, f"Expected pattern to match valid entry: {entry}" | |
| assert match.group(0) == entry | |
| def test_httpx_socks_pattern_does_not_match_invalid_entries() -> None: | |
| invalid_entries = [ | |
| "httpx", | |
| "httpx==0.27.0", | |
| "httpx[http2]", | |
| "httpx[socks-extra]", | |
| "httpx [socks]", | |
| "someprefix httpx[socks]", | |
| "httpx[socks] trailing-text", | |
| "httpx[socks] extra ; markers", | |
| "httpx[socks]andmore", | |
| ] | |
| for entry in invalid_entries: | |
| assert HTTPX_SOCKS_PATTERN.match(entry) is None, ( | |
| f"Expected pattern not to match invalid entry: {entry}" | |
| ) | |
| def _read_httpx_socks_dependency(entries: list[str]) -> str | None: |
Comment on lines
+67
to
+70
| def test_httpx_socks_pattern_allows_environment_markers() -> None: | ||
| entry = 'httpx[socks]; python_version >= "3.11"' | ||
|
|
||
| assert HTTPX_SOCKS_PATTERN.match(entry), ( |
Contributor
There was a problem hiding this comment.
suggestion (testing): Extend the pattern tests to cover version specifiers and minor formatting variations.
Consider parametrizing this test with additional valid patterns, such as:
httpx[socks]==0.28.1httpx[socks]>=0.27,<0.29httpx[socks] >=0.27 ; python_version < "3.13"
This would improve coverage for version specifiers and whitespace variations so future changes torequirements.txt/pyproject.tomlremain compatible with the regex.
Suggested implementation:
import pytest
@pytest.mark.parametrize(
"entry",
[
'httpx[socks]; python_version >= "3.11"',
"httpx[socks]==0.28.1",
"httpx[socks]>=0.27,<0.29",
'httpx[socks] >=0.27 ; python_version < "3.13"',
],
)
def test_httpx_socks_pattern_matches_valid_variants(entry: str) -> None:
assert HTTPX_SOCKS_PATTERN.match(entry), (
"Expected httpx[socks] dependency pattern to allow environment markers, "
"version specifiers, and whitespace variations for SOCKS proxy support"
)If pytest is already imported elsewhere in this file, remove the new import pytest line from the replacement block to avoid duplicate imports. Otherwise, keep it so that @pytest.mark.parametrize works correctly.
dosubot
bot
added
size:L
Member
Author
|
Addressed the latest review in
|
Member
Author
|
@sourcery-ai review |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
httpx[socks]dependency so SOCKS proxies from environment variables work for AstrBot'shttpxclientsrequirements.txtandpyproject.tomlaligned with a regression test that guards the missing dependencyTesting
uv run ruff format tests/test_httpx_socks_dependency.pyuv run ruff check tests/test_httpx_socks_dependency.pyuv run pytest tests/test_httpx_socks_dependency.pypnpm run prepare:backendresources/backend/python/bin/python3can createhttpx.AsyncClient(proxy='socks5://127.0.0.1:1080')without the previoussocksioimport errorReferences
Summary by Sourcery
Add explicit httpx[socks] dependency and guard it with tests to ensure SOCKS proxy support remains correctly configured across dependency files.
New Features:
Tests: