Skip to content

Commit

Permalink
Resolve "Spot websocket connection doesn't get closed properly" (#318)
Browse files Browse the repository at this point in the history
  • Loading branch information
btschwertfeger authored Nov 26, 2024
1 parent e44d547 commit 2c45944
Show file tree
Hide file tree
Showing 11 changed files with 36 additions and 13 deletions.
4 changes: 2 additions & 2 deletions .github/workflows/_test_futures_private.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -88,12 +88,12 @@ jobs:
FUTURES_SECRET_KEY: ${{ secrets.FUTURES_SECRET_KEY }}
FUTURES_SANDBOX_KEY: ${{ secrets.FUTURES_SANDBOX_KEY }}
FUTURES_SANDBOX_SECRET: ${{ secrets.FUTURES_SANDBOX_SECRET }}
run: pytest -vv -m "futures_auth and not futures_websocket and not flaky" tests
run: pytest -vv -m "futures and futures_auth and not futures_websocket and not flaky" tests

- name: Testing Futures websocket client
env:
FUTURES_API_KEY: ${{ secrets.FUTURES_API_KEY }}
FUTURES_SECRET_KEY: ${{ secrets.FUTURES_SECRET_KEY }}
FUTURES_SANDBOX_KEY: ${{ secrets.FUTURES_SANDBOX_KEY }}
FUTURES_SANDBOX_SECRET: ${{ secrets.FUTURES_SANDBOX_SECRET }}
run: pytest -vv -m "futures_websocket and not flaky" tests
run: pytest -vv -m "futures and futures_auth and futures_websocket and not flaky" tests
3 changes: 3 additions & 0 deletions .github/workflows/_test_futures_public.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -70,3 +70,6 @@ jobs:
- name: Testing Futures REST endpoints
run: pytest -vv -m "futures and not futures_auth and not futures_websocket" tests

- name: Testing Futures websocket endpoints
run: pytest -vv -m "futures and not futures_auth and futures_websocket" tests
2 changes: 1 addition & 1 deletion .github/workflows/_test_nft_private.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -77,4 +77,4 @@ jobs:
env:
SPOT_API_KEY: ${{ secrets.SPOT_API_KEY }}
SPOT_SECRET_KEY: ${{ secrets.SPOT_SECRET_KEY }}
run: pytest -vv -m nft_auth tests
run: pytest -vv -m "nft and nft_auth" tests
8 changes: 7 additions & 1 deletion .github/workflows/_test_spot_private.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -80,8 +80,14 @@ jobs:
echo "$env:GITHUB_WORKSPACE\.venv\Scripts" | Out-File -FilePath $env:GITHUB_PATH -Encoding utf8 -Append
uv pip install ".[test]"
- name: Testing Spot REST endpoints
env:
SPOT_API_KEY: ${{ secrets.SPOT_API_KEY }}
SPOT_SECRET_KEY: ${{ secrets.SPOT_SECRET_KEY }}
run: pytest -vv -m "spot and spot_auth and not spot_websocket" tests

- name: Testing Spot websocket client
env:
SPOT_API_KEY: ${{ secrets.SPOT_API_KEY }}
SPOT_SECRET_KEY: ${{ secrets.SPOT_SECRET_KEY }}
run: pytest -vv -m spot_websocket tests
run: pytest -vv -m "spot and spot_auth and spot_websocket" tests
4 changes: 4 additions & 0 deletions .github/workflows/_test_spot_public.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ jobs:
github.com:443
objects.githubusercontent.com:443
pypi.org:443
ws.kraken.com:443
- name: Checkout repository
uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2
Expand Down Expand Up @@ -70,3 +71,6 @@ jobs:
- name: Testing Spot REST endpoints
run: pytest -vv -m "spot and not spot_auth and not spot_websocket" tests

- name: Testing Spot websocket endpoints
run: pytest -vv -m "spot and not spot_auth and spot_websocket" tests
6 changes: 4 additions & 2 deletions kraken/futures/websocket/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@
from random import random
from typing import TYPE_CHECKING, Any

import websockets
from websockets.asyncio.client import connect

from kraken.exceptions import MaxReconnectError

Expand Down Expand Up @@ -95,9 +95,11 @@ async def __run( # noqa: C901
self.__new_challenge = None
self.__last_challenge = None

async with websockets.connect( # pylint: disable=no-member # noqa: PLR1702
async with connect( # pylint: disable=no-member # noqa: PLR1702
f"wss://{self.__ws_endpoint}",
additional_headers={"User-Agent": "btschwertfeger/python-kraken-sdk"},
ping_interval=30,
max_queue=None, # FIXME: This is not recommended by the docs https://websockets.readthedocs.io/en/stable/reference/asyncio/client.html#module-websockets.asyncio.client
) as socket:
LOG.info("Websocket connected!")
self.socket = socket
Expand Down
5 changes: 3 additions & 2 deletions kraken/spot/websocket/connectors.py
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@
from time import time
from typing import TYPE_CHECKING, Any, Final

import websockets
from websockets.asyncio.client import connect

from kraken.exceptions import MaxReconnectError

Expand Down Expand Up @@ -127,10 +127,11 @@ async def __run(self: ConnectSpotWebsocketBase, event: asyncio.Event) -> None:
)
LOG.debug("Websocket token: %s", self.ws_conn_details)

async with websockets.connect( # pylint: disable=no-member
async with connect( # pylint: disable=no-member
f"wss://{self.__ws_endpoint}",
additional_headers={"User-Agent": "btschwertfeger/python-kraken-sdk"},
ping_interval=30,
max_queue=None, # FIXME: This is not recommended by the docs https://websockets.readthedocs.io/en/stable/reference/asyncio/client.html#module-websockets.asyncio.client
) as socket:
LOG.info("Websocket connected!")
self.socket = socket
Expand Down
2 changes: 1 addition & 1 deletion pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ requires-python = ">=3.11"
dependencies = [
"asyncio>=3.4",
"requests",
"websockets>=14",
"websockets>=14.1",
"click",
"cloup",
"orjson",
Expand Down
1 change: 1 addition & 0 deletions tests/spot/test_spot_base_api.py
Original file line number Diff line number Diff line change
Expand Up @@ -122,6 +122,7 @@ async def check() -> None:

@pytest.mark.spot
@pytest.mark.spot_auth
@pytest.mark.timeout(120)
def test_spot_rest_async_client_post_report(
spot_api_key: str,
spot_secret_key: str,
Expand Down
13 changes: 9 additions & 4 deletions tests/spot/test_spot_orderbook.py
Original file line number Diff line number Diff line change
Expand Up @@ -133,8 +133,11 @@ def test_add_book(caplog: pytest.LogCaptureFixture) -> None:
async def execute_add_book() -> None:
async with SpotOrderBookClientWrapper() as orderbook:

await orderbook.add_book(pairs=["BTC/USD"])
await async_sleep(2)
# having multiple pairs to test the cancellation queue error absence
await orderbook.add_book(
pairs=["BTC/USD", "DOT/USD", "ETH/USD", "MATIC/USD", "BTC/EUR"],
)
await async_sleep(4)

book: dict | None = orderbook.get(pair="BTC/USD")
assert isinstance(book, dict)
Expand All @@ -154,8 +157,10 @@ async def execute_add_book() -> None:
asyncio.run(execute_add_book())

for expected in (
'{"method": "subscribe", "result": {"channel": "book", "depth": 10, "snapshot": true, "symbol": "BTC/USD"}, "success": true, "time_in": ',
'{"channel": "book", "type": "snapshot", "data": [{"symbol": "BTC/USD", "bids": ',
'{"method": "subscribe", "result": {"channel": "book", "depth": 10, '
'"snapshot": true, "symbol": "BTC/USD"}, "success": true, "time_in": ',
'{"channel": "book", "type": "snapshot", "data": '
'[{"symbol": "BTC/USD", "bids": ',
):
assert expected in caplog.text

Expand Down
1 change: 1 addition & 0 deletions tests/spot/test_spot_user.py
Original file line number Diff line number Diff line change
Expand Up @@ -297,6 +297,7 @@ def test_get_trade_volume(spot_auth_user: User) -> None:
@pytest.mark.spot
@pytest.mark.spot_auth
@pytest.mark.spot_user
@pytest.mark.timeout(120)
def test_request_save_export_report(spot_auth_user: User) -> None:
"""
Checks the ``save_export_report`` function by requesting an
Expand Down

0 comments on commit 2c45944

Please sign in to comment.