WebSocket API: in-flight requests are lost on serverShutdown, rotation or a dropped connection (caller gets a 20 s timeout)

Đang mở
  • Có gì không ổn?
#587 1 bình luận 0 reaction 0 người được giao Xem trên GitHub

Maintainer thường phản hồi trong vòng 1 ngày

Chưa có ai nhận issue này.

Đánh giá

Độ khó
4/5
Thời gian dự kiến
3-5 ngày
Mức phù hợp với người mới
55/100
Loại issue
Lỗi
Độ rõ ràng
Đặc tả rõ ràng
Mức độ hoạt động
Sôi nổi
Công nghệ
python
Lĩnh vực
api, backend

Hướng nghiên cứu

Start with common/src/binance_common/websocket.py, especially receive_loop, reconnect(), _discard_connection and send_message; the report also points to common/tests/unit/test_fault_injection.py and test_initiates_renewal_on_server_shutdown. Run the three InFlight fault injection tests to reproduce the lost replies and delayed errors. Done means pending callers receive replies from the old socket when available, and get a prompt connection error when it closes.

Do mô hình lập chỉ mục viết ra từ nội dung của issue.

Mô tả

Summary

A WebSocket API request that is in flight when its connection is replaced or dropped never gets its answer. The caller waits the full 20 s and receives {"error": "timeout"}, even when the server already executed the request. For order.place this is the worst case for a trading client: the order exists, the client reports a timeout, and a retry places it again.

Three paths lead there, all in common/src/binance_common/websocket.py (master 13fda51, binance-common 4.8.0):

  1. serverShutdown (lines 325-337). receive_loop awaits schedule_reconnect(..., 5, close_old_connection=False) inline, so the old socket is not read for 5 s plus the time to open the new connection. connection.reconnect stays False on this path, so send_message keeps routing new requests to the old socket during that window.
  2. reconnect() (lines 845-846), used both for serverShutdown and for the 23 h rotation. It runs connection.pending_request.clear() without resolving the futures. A reply that arrives afterwards has an id that is no longer in pending_request and falls through to the "Received message" branch, so the waiting caller never sees it.
  3. Connection closed (receive_loop exit and _discard_connection, lines 427-449 and 495-522). The socket is closed and removed from the pool, but the pending futures are not failed. Callers only find out when asyncio.wait_for(future, timeout=20) expires (the same 20 s is hardcoded at lines 1076, 1344, 1698, 1790 and 1895).

The Binance docs describe serverShutdown as sent "when the server is about to shut down ... Please establish a new connection as soon as possible", so the old connection is still alive when the event arrives and can still answer requests already sent on it.

Reproduction

Tests against a real local aiohttp WebSocket server (no mocks), branch Amadeus-22:hunt/failure-modes:

cd common
python -m pytest -q tests/unit/test_fault_injection.py -k InFlight
FAILED test_request_after_server_shutdown_gets_the_servers_answer   - server answered order.place at once, caller got nothing after 3.0s
FAILED test_request_pending_when_connection_drops_fails_promptly    - connection closed at once, caller still waiting after 3.0s
FAILED test_request_answered_during_rotation_reaches_the_caller     - request in flight at rotation got nothing after 3.0s

With the library's real 20 s wait, all three cases return {'error': 'timeout'} after 20.0 s, while the server log shows order.place received (and answered, in the shutdown and rotation cases).

Proposed fix (small, websocket.py only)
  • serverShutdown: start the replacement with asyncio.create_task(...) instead of awaiting it inside receive_loop, and mark the connection as draining so new requests go to the new connection while the old loop keeps delivering replies to requests already sent.
  • reconnect(): do not clear() pending futures that the old socket can still answer; before the old socket is finally closed, fail whatever is left with ConnectionError.
  • Connection closed: fail every unfinished future with ConnectionError when receive_loop exits. send_message already turns exceptions into {"error": ...}, so callers learn immediately instead of after 20 s.

On a scratch copy these three changes make the tests above pass; the only existing test that needs updating is test_initiates_renewal_on_server_shutdown, which asserts the inline await.

Not verified
  • Against the live API: how long Binance keeps answering on a connection after sending serverShutdown. The drop and rotation cases do not depend on it.
  • The session-logon variant (the extra 1 s re-logon window in reconnect()).

The hardcoded 20 s request wait also ignores ConfigurationWebSocketAPI.timeout, documented as "Request timeout in milliseconds (default: 5000)"; I left that out of scope.

Happy to open a PR with the fix and these tests if this direction looks right.

Ngôn ngữ chính
Python
Star
3k
Fork
712
Merge trung bình
1 ngày 13 giờ
Pull request đã merge (30 ngày)
9

Chuẩn bị môi trường

Dự án này không cung cấp dev container, Dockerfile hay hướng dẫn đóng góp, nên bạn cần tự thiết lập môi trường: hãy bắt đầu từ README và xem hướng dẫn đóng góp lần đầu của chúng tôi để biết các bước chung.

Bắt đầu từ đâu

  1. Đọc hết issue, rồi đọc hướng dẫn đóng góp của dự án.
  2. Bình luận trên issue rằng bạn sẽ nhận — tránh hai người làm cùng một việc.
  3. Fork repository và làm thay đổi trên một nhánh.
  4. Mở pull request có tham chiếu số hiệu của issue.

Issue khác của binance/binance-connector-python

Tất cả issue của binance/binance-connector-python

Issue tương tự

Thêm issue về Python

Nhận issue mới trong hộp thư của bạn

Bản tóm tắt ngắn những issue GitHub phù hợp với người mới.