Skip to content

fix: honor PROXY_URL for live-trading REST calls - #229

Open
z20251130 wants to merge 1 commit into
OpenByteInc:mainfrom
z20251130:fix/live-rest-proxy
Open

fix: honor PROXY_URL for live-trading REST calls#229
z20251130 wants to merge 1 commit into
OpenByteInc:mainfrom
z20251130:fix/live-rest-proxy

Conversation

@z20251130

Copy link
Copy Markdown

Summary

env.example documents PROXY_URL as covering "market data, exchange and broker API traffic", and market data (CCXT) honors it. The live-trading funnel BaseRestClient._request used plain requests without proxies, so on deployments that reach exchanges only through PROXY_URL (mainland CN / containerized setups), every private REST call failed — position snapshots, order placement, account queries — with ConnectionResetError(104), while strategy signals kept working from cached market data. This made live auto-trading silently break in a way that looked like an exchange problem.

This PR wires PROXY_URL into the funnel, following the existing _get_requests_verify() pattern in the same file (whose docstring already anticipated proxied calls).

Root cause

  • BaseRestClient._request (the single HTTP entry point for all six exchange clients — binance spot/futures, okx, bitget, bybit, gate, htx) passed no proxies to requests.request.
  • requests only understands standard HTTPS_PROXY/HTTP_PROXY/ALL_PROXY env vars, not the platform's PROXY_URL, and live trading code never bridged the two.

Observed in production logs (Binance demo, strategy live deployment):

position_snapshot_failed:('Connection aborted.', ConnectionResetError(104, 'Connection reset by peer'))
Order rejected because the exchange position snapshot failed: BNB/USDT

Changes

  • app/services/live_trading/base.py: add cached _get_proxies() next to _get_requests_verify() and pass it to requests.request in _request.
    • PROXY_URL set → explicit {"http": url, "https": url} proxies (explicit wins over env, matching the documented precedence; socks5/socks5h work — PySocks is already a dependency).
    • PROXY_URL unset → None: nothing is forced, standard HTTPS_PROXY/HTTP_PROXY/ALL_PROXY env vars and NO_PROXY bypasses keep applying via trust_envzero behavior change for deployments not using PROXY_URL.

Test plan

  • New unit tests (tests/test_live_rest_proxy.py): explicit http proxy applied, socks5h applied, unset forces nothing (trust_env stays in charge), per-process caching, funnel forwarding — 5/5 pass.
  • End-to-end on a real deployment (Binance USD-M demo endpoint, standard proxy env vars removed, only PROXY_URL set): GET /fapi/v1/time → 200, GET /fapi/v1/exchangeInfo → 200 with 882 symbols through BinanceFuturesClient._request.
  • Verified the same client fails with ConnectionResetError(104) when no proxy is configured at all (reproduces the reported symptom).

API documentation

  • No route/schema changes — no OpenAPI regeneration needed.

Backward compatibility

  • Deployments without PROXY_URL: unchanged (helper resolves to None, trust_env behavior identical).
  • Deployments with PROXY_URL: live-trading traffic starts honoring the documented setting; if both PROXY_URL and standard env vars are set, PROXY_URL wins for live trading (consistent with its documented scope).

env.example documents PROXY_URL as covering 'market data, exchange and
broker API traffic'. Market data (CCXT) honors it, but the live-trading
funnel BaseRestClient._request used plain requests without proxies, so
on deployments that reach exchanges only through PROXY_URL, every
private REST call (position snapshot, orders, account queries) failed
with ConnectionResetError while signals kept working from cached
market data.

Add a cached _get_proxies() resolver next to _get_requests_verify():
PROXY_URL set -> explicit http/https proxies (socks5(h) works, PySocks
is already a dependency); unset -> None, so standard HTTPS_PROXY /
HTTP_PROXY / ALL_PROXY env vars and NO_PROXY bypasses keep applying
via trust_env, unchanged from current behavior.

Covered by unit tests for explicit/socks/unset/cached resolution.
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.

1 participant