Conversation
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
| self.mode = self._get_client_mode() | ||
| self.http_proxies = http_proxies | ||
| if self.http_proxies: | ||
| set_http_client_proxies(self.http_proxies) |
There was a problem hiding this comment.
Global proxy state causes unexpected cross-instance interference
The http_proxies parameter is stored as an instance attribute on ClobClient, suggesting per-instance proxy configuration, but set_http_client_proxies modifies a global _http_client shared across all instances. Creating multiple ClobClient instances with different proxy settings causes all clients to use the last-configured proxy, which contradicts the API's apparent design. This will cause unexpected behavior when users attempt to use different proxies for different clients.
Additional Locations (1)
| return { | ||
| f"{scheme}://": httpx.HTTPTransport(proxy=url) | ||
| for scheme, url in proxies.items() | ||
| } |
There was a problem hiding this comment.
Proxy transports silently downgrade HTTP/2 to HTTP/1.1
The _build_proxy_mounts function creates httpx.HTTPTransport instances without passing http2=True, but the default client and non-proxy client both use http2=True. In httpx, when custom mounts are used, the transport's configuration takes precedence. Since HTTPTransport defaults to HTTP/1.1, requests through the proxy will silently use HTTP/1.1 instead of HTTP/2, causing unexpected protocol downgrade and potential performance regression.
There was a problem hiding this comment.
Pull request overview
This PR adds HTTP proxy support to the ClobClient to enable requests through proxy servers, which is useful for bypassing Cloudflare restrictions or routing traffic through residential proxies.
Key changes:
- Added
http_proxiesoptional parameter toClobClient.__init__accepting a dict with 'http' and/or 'https' proxy URLs - Implemented proxy configuration functions in
http_helpers/helpers.pyto validate and apply proxy settings to the global HTTP client - Added unit tests for proxy validation and configuration
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 8 comments.
| File | Description |
|---|---|
py_clob_client/client.py |
Added http_proxies parameter to ClobClient constructor and calls set_http_client_proxies to configure the global HTTP client |
py_clob_client/http_helpers/helpers.py |
Added _prepare_http_proxies, _build_proxy_mounts, _create_http_client, and set_http_client_proxies functions to handle proxy configuration and validation |
tests/http_helpers/test_helpers.py |
Added tests for proxy validation, rejection of invalid formats, and verification that proxy mounts are configured correctly; includes tearDown to reset proxy state |
Comments suppressed due to low confidence (1)
py_clob_client/client.py:138
- The docstring for init should be updated to document the new http_proxies parameter. The docstring currently describes the three client modes but doesn't mention the proxy configuration option.
Add documentation that includes:
- Parameter description for http_proxies
- Expected format (e.g., {"http": "http://proxy.com:8080", "https": "https://proxy.com:8080"})
- Supported schemes (http and https only)
- Any limitations (e.g., that this uses a global HTTP client)
"""
Initializes the clob client
The client can be started in 3 modes:
1) Level 0: Requires only the clob host url
Allows access to open CLOB endpoints
2) Level 1: Requires the host, chain_id and a private key.
Allows access to L1 authenticated endpoints + all unauthenticated endpoints
3) Level 2: Requires the host, chain_id, a private key, and Credentials.
Allows access to all endpoints
"""
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| self.http_proxies = http_proxies | ||
| if self.http_proxies: | ||
| set_http_client_proxies(self.http_proxies) |
There was a problem hiding this comment.
Using a global HTTP client creates issues when multiple ClobClient instances are created with different proxy configurations. The current implementation uses a single global _http_client in helpers.py, which means:
- Creating a second ClobClient with different proxy settings will override the first client's proxy configuration
- Creating a ClobClient without proxies after one with proxies will reset the proxy for all existing clients
- This affects concurrent usage and makes it impossible to use different proxies for different clients
Consider one of these alternatives:
- Store an instance-specific http client on each ClobClient instance instead of using a global client
- Pass the http_proxies parameter to each request method and configure the client on-demand
- Document this limitation clearly if the global client is intentional
Example scenario that will fail:
client1 = ClobClient(host, http_proxies={"http": "http://proxy1.com"})
client2 = ClobClient(host, http_proxies={"http": "http://proxy2.com"})
# Now client1 will also use proxy2.com instead of proxy1.com| def set_http_client_proxies(http_proxies: dict[str, str] | None): | ||
| global _http_client | ||
| new_client = _create_http_client(http_proxies) | ||
| old_client = _http_client | ||
| _http_client = new_client | ||
| if old_client is not None: | ||
| old_client.close() |
There was a problem hiding this comment.
The proxy validation in _prepare_http_proxies should be called before setting the global _http_client to avoid leaving the client in an inconsistent state if validation fails. Currently, if _prepare_http_proxies raises an exception, the old client has already been closed (line 42), but the new client creation failed, potentially leaving _http_client in an undefined state.
The flow should be:
- Validate http_proxies
- Create new client (which may fail)
- Only then swap and close old client
| self.http_proxies = http_proxies | ||
| if self.http_proxies: | ||
| set_http_client_proxies(self.http_proxies) |
There was a problem hiding this comment.
The http_proxies parameter should be validated before being assigned to self.http_proxies. Currently, validation only occurs inside set_http_client_proxies, which means:
- Invalid proxy configurations will be stored in self.http_proxies
- The exception will be raised from set_http_client_proxies rather than directly from init, making debugging harder
- The self.http_proxies attribute will contain invalid data even after the exception
Consider calling _prepare_http_proxies to validate before assignment, or add a validation step in init.
| self.http_proxies = http_proxies | |
| if self.http_proxies: | |
| set_http_client_proxies(self.http_proxies) | |
| validated_http_proxies = http_proxies | |
| if validated_http_proxies: | |
| set_http_client_proxies(validated_http_proxies) | |
| self.http_proxies = validated_http_proxies |
| def set_http_client_proxies(http_proxies: dict[str, str] | None): | ||
| global _http_client | ||
| new_client = _create_http_client(http_proxies) | ||
| old_client = _http_client | ||
| _http_client = new_client | ||
| if old_client is not None: | ||
| old_client.close() |
There was a problem hiding this comment.
The global _http_client modification in set_http_client_proxies is not thread-safe. If multiple threads create ClobClient instances simultaneously, race conditions could occur:
- Multiple threads could call set_http_client_proxies concurrently
- The old_client.close() call could close a client that another thread is using
- Threads could see different _http_client instances mid-swap
If concurrent client creation is a supported use case, consider adding thread synchronization (e.g., a lock) around the global _http_client modification.
| def test_set_http_client_proxies_configures_mounts(self): | ||
| proxy_url = "http://localhost:8080" | ||
| set_http_client_proxies({"http": proxy_url}) | ||
| client = helpers_module._http_client | ||
| self.assertIn("http://", client._mounts) | ||
| self.assertIsInstance(client._mounts["http://"], httpx.HTTPTransport) |
There was a problem hiding this comment.
The test coverage for proxy functionality is missing critical scenarios:
- No test verifies that http_proxies validation occurs when creating a ClobClient instance
- No test covers the scenario where multiple ClobClient instances are created with different proxy settings
- No test verifies that invalid http_proxies passed to ClobClient.init raises an appropriate exception
- No test ensures that setting http_proxies=None works correctly when a client already has proxies configured
Consider adding tests for these edge cases to ensure the feature works correctly in real-world usage patterns.
| if not isinstance(http_proxies, dict): | ||
| raise PolyApiException(error_msg="http_proxies must be a dict") | ||
|
|
||
| prepared: dict[str, str] = {} | ||
| for scheme in ("http", "https"): | ||
| if scheme in http_proxies: | ||
| value = http_proxies[scheme] | ||
| if not isinstance(value, str): | ||
| raise PolyApiException( | ||
| error_msg="http_proxies values must be strings", | ||
| ) | ||
| prepared[scheme] = value | ||
|
|
||
| if not prepared: | ||
| raise PolyApiException( | ||
| error_msg="http_proxies must include at least one of 'http' or 'https'", | ||
| ) |
There was a problem hiding this comment.
The error messages in _prepare_http_proxies could be more helpful by providing examples or guidance. Consider enhancing them:
- Line 49: "http_proxies must be a dict" → "http_proxies must be a dict, e.g., {'http': 'http://proxy.com:8080'}"
- Line 57: "http_proxies values must be strings" → "http_proxies values must be strings representing proxy URLs"
- Line 63: The current message is good, but could also mention the expected format
More descriptive error messages help developers quickly understand and fix configuration issues.
| prepared: dict[str, str] = {} | ||
| for scheme in ("http", "https"): | ||
| if scheme in http_proxies: | ||
| value = http_proxies[scheme] | ||
| if not isinstance(value, str): | ||
| raise PolyApiException( | ||
| error_msg="http_proxies values must be strings", | ||
| ) | ||
| prepared[scheme] = value |
There was a problem hiding this comment.
The _prepare_http_proxies function validates that values are strings but doesn't validate that they are valid proxy URLs. Invalid URL formats will only fail later when httpx tries to use them, making debugging harder.
Consider adding basic URL validation to fail fast with a clear error message. For example, check that:
- The URL has a valid scheme (http://, https://, socks5://)
- The URL is parseable
- The URL matches what httpx expects for proxy configuration
This would provide better error messages at configuration time rather than at runtime.
| from unittest import TestCase | ||
| import httpx | ||
|
|
||
| import py_clob_client.http_helpers.helpers as helpers_module |
There was a problem hiding this comment.
Module 'py_clob_client.http_helpers.helpers' is imported with both 'import' and 'import from'.
1 participant
Overview
Adding HTTP proxy configuration on Client instance through attr/property.
Description
-> Adding new optional attribute on Client instance: http_proxy
-> Adding a few functions on http_helpers/helpers.py to receive a dict with proxy data and convert it to HTTPX pattern.
-> Adding a function to replace default client if proxy setted up
-> Also added some tests to pattern and attr acceptance.
Types of changes
Notes
-> Here's the link to the docs about proxies on HTTPX.
I'm submitting it cause I found out some serveres may be unable to reach Polymarket due to it's Cloudflare blacklist, so it would be great if we can set up residential proxies to enable automations and some cloud-based computing
Status
[WIP]if necessary (changes not yet made).Note
Adds configurable HTTP proxy support and validation to the HTTP client, with client-level opt-in and tests.
http_proxiesonClobClient; when provided, callsset_http_client_proxiesto reconfigure the shared HTTPX client_prepare_http_proxies(validateshttp/httpskeys and string values),_build_proxy_mounts,_create_http_client, andset_http_client_proxiesinhttp_helpers/helpers.pyto create/replace the HTTPX client with proxy mounts and close the old clientWritten by Cursor Bugbot for commit 2e94242. This will update automatically on new commits. Configure here.