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

Aperta
  • Qualcosa non va?
#587 1 commento 0 reazioni 0 assegnatari Vedi su GitHub

I maintainer di solito rispondono entro 1 giorno

Nessuno ha ancora preso questa issue.

Valutazione

Difficoltà
4/5
Tempo stimato
3-5 giorni
Idoneità per principianti
55/100
Tipo di issue
Bug
Chiarezza
Specificata chiaramente
Stato di attività
Attiva
Stack tecnologico
python
Ambito
api, backend

Direzione di ricerca

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.

Scritto dal modello di indicizzazione a partire dal testo della issue.

Descrizione

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.

Lingua principale
Python
Stelle
3k
Fork
712
Merge medio
1g 13h
PR unite (30g)
9

Preparare l'ambiente

Questo progetto non fornisce container di sviluppo, Dockerfile né guida per i contributori, quindi l'ambiente è a tuo carico: parti dal suo README e consulta la nostra guida al primo contributo per i passaggi generali.

Come iniziare

  1. Leggi tutta la issue e poi la guida ai contributi del progetto.
  2. Commenta sulla issue per dire che te ne occupi tu — evita che due persone facciano lo stesso lavoro.
  3. Fai un fork del repository e lavora su un branch.
  4. Apri una pull request che faccia riferimento al numero della issue.

Altre issue di binance/binance-connector-python

Tutte le issue di binance/binance-connector-python

Issue simili

Altre issue su Python

Ricevi le nuove issue nella tua casella

Un breve riepilogo di issue GitHub adatte ai principianti.