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

オープン
  • 何か問題がありますか？
#587 コメント 1 件 リアクション 0 件 担当者 0 名 GitHub で見る

メンテナーはふだん 1 日以内に返信

まだ誰も着手していません。

評価

難易度
4/5
見積もり時間
3〜5日
初心者へのやさしさ
55/100
issue の種類
バグ
明瞭さ
明確に書かれている
活発さ
活発
技術スタック
python
領域
api, backend

調査の方向性

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.

索引モデルが issue の本文から書いたものです。

説明

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.

主要言語
Python
スター
3k
フォーク
712
平均マージ
1日 13時間
マージ済み PR（30日）
9

環境構築

このプロジェクトには開発コンテナ、Dockerfile、コントリビューションガイドがありません。まず README を読み、一般的な手順ははじめてのコントリビューションガイドを参照してください。

はじめの一歩

  1. issue を最後まで読み、次にプロジェクトのコントリビューションガイドを読みます。
  2. 着手することを issue にコメントします — 二人が同じ作業をするのを防げます。
  3. リポジトリをフォークし、ブランチを切って変更します。
  4. issue 番号を参照したプルリクエストを送ります。

binance/binance-connector-python のほかの issue

binance/binance-connector-python の issue をすべて見る

似ている issue

Python の issue をもっと見る

新しい issue をメールで受け取る

初心者向けの GitHub issue を短くまとめたダイジェスト。