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

Abierto
  • ¿Algo no está bien?
#587 1 comentario 0 reacciones 0 asignados Ver en GitHub

Los mantenedores suelen responder en 1 día

Nadie ha tomado este issue todavía.

Evaluación

Dificultad
4/5
Tiempo estimado
3-5 días
Aptitud para principiantes
55/100
Tipo de issue
Error
Claridad
Bien especificado
Estado de actividad
Activo
Stack tecnológico
python
Área
api, backend

Línea de trabajo

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.

Escrito por el modelo de indexación a partir del texto del issue.

Descripción

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.

Lenguaje dominante
Python
Estrellas
3k
Forks
712
Merge medio
1 d 13 h
PR fusionados (30 d)
9

Preparar el entorno

Este proyecto no incluye contenedor de desarrollo, Dockerfile ni guía de contribución, así que la configuración corre por tu cuenta: empieza por su README y consulta nuestra guía para la primera contribución para los pasos generales.

Primeros pasos

  1. Lee el issue completo y luego la guía de contribución del proyecto.
  2. Comenta en el issue que vas a ocuparte — evita que dos personas hagan lo mismo.
  3. Haz un fork del repositorio y trabaja en una rama.
  4. Abre un pull request que haga referencia al número del issue.

Más de binance/binance-connector-python

Todos los issues de binance/binance-connector-python

Issues similares

Más issues de Python

Recibe los nuevos issues en tu correo

Un resumen breve de issues de GitHub para principiantes.