Skip to content

Commit dc132e0

Browse files
Brutus5000claude
andauthored
feat(ws): native WebSocket transport, drop raw TCP listeners (#1093)
* feat(ws): native WebSocket transport, drop raw TCP listeners The lobby server now accepts WebSocket connections directly on /ws (default port 8003) instead of raw TCP behind the ws_bridge_rs sidecar. This removes a transport-level hop that broke client reconnects, and keeps everything on HTTP so the deployment can stay fully behind DDoS-protected HTTPS ingress. - New WebSocketProtocol (aiohttp WebSocketResponse / ClientWebSocketResponse) using one JSON object per text frame. - ServerContext replaces asyncio.start_server with an aiohttp app, dropping the proxyprotocol detection path. - Config: LISTEN list replaced with WS_HOST / WS_PORT / WS_PATH. - Pipfile: proxy-protocol removed. - Tests rewritten to drive the server over real WebSocket clients; proxy-mode tests removed. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(ws): lint, lockfile, metric double-count, untrusted forwarded IP - Remove unused aiohttp.web import and unused get_session test import (flake8 F401). - Regenerate Pipfile.lock after dropping proxy-protocol. - WebSocketProtocol.write_messages no longer increments sent_messages itself — write_raw already does it per message. - ServerContext only honors a forwarded-IP header when the new WS_FORWARDED_IP_HEADER config is set (e.g. "CF-Connecting-IP"); otherwise uses the direct TCP peer, so clients cannot spoof their peername by sending X-Forwarded-For themselves. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * style: address codacy docstring findings - websocket.py module docstring uses D212 style (summary on line 2). - Add one-line __init__ docstring (D107). - test_multiple_contexts docstring has a one-line summary ending with a period (D205/D212/D415). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * style: swap docstring summary placement (D212 vs D213) Codacy enforces D212 on modules (summary on first line) and D213 on functions (summary on second line) — I had them reversed in the previous commit. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * style: collapse docstrings to single-line to satisfy D212+D213 Codacy has both D212 and D213 enabled, which is mutually contradictory for multi-line docstrings — whichever style we pick, the other fires. Single-line docstrings dodge both rules. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * feat(ws): default WS_FORWARDED_IP_HEADER to X-Real-IP Cloudflare (and our Traefik ingress) sets X-Real-IP to the real client address, so it's the right default. Still configurable to "" for deployments where the server is exposed to untrusted clients directly (spoofable headers). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * test: raise WebSocketProtocol and SimpleJson coverage - WebSocketProtocol now reaches 100% line coverage in isolation: added tests for binary frames, write_message/write_messages paths, empty-pending drain, drain-failure -> DisconnectedError, and abort with/without an already-closed ws. - SimpleJsonProtocol regains coverage of decode_message / read_message / empty-readline DisconnectedError; these were previously hit through the qstream-vs-json integration parametrize that this PR removed. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(ws): default path to '/' to match existing clients The Java client (faf-commons-lobby) connects to wss://lobby.{base} with no path component, so the WS upgrade GET hits '/'. Serving on '/ws' caused aiohttp to fail routing and return 'Invalid method encountered' (the parser confusing itself on the unmatched URL). Switch the default WS_PATH and the path= function defaults to '/' so out of the box the server matches what the client sends. Existing deployments can still pin WS_PATH to '/ws' (or anything else) via config. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(ws): terminate outgoing frames with '\n' for client framing faf-commons-lobby (the Kotlin lobby client) splits the inbound WS byte stream on '\n' rather than treating each frame as a complete message. Without a trailing newline the client buffers the response indefinitely and times out the login flow after 30 s. The previous SimpleJsonProtocol already encoded messages this way, which is why it worked through ws_bridge_rs unchanged. Mirror that behavior in WebSocketProtocol so existing clients keep working. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * test: assert_awaited for ws.close in abort test CodeRabbit nit: assert_called only confirms the close coroutine was scheduled, not that it actually ran to completion. assert_awaited is the right check for an AsyncMock. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent b6b1850 commit dc132e0

17 files changed

Lines changed: 573 additions & 382 deletions

Pipfile

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,6 @@ humanize = ">=2.6.0"
2121
maxminddb = "*"
2222
oauthlib = "*"
2323
prometheus_client = "*"
24-
proxy-protocol = "*"
2524
pyjwt = {version = ">=2.4.0", extras = ["crypto"]}
2625
pyyaml = "*"
2726
sortedcontainers = "*"

Pipfile.lock

Lines changed: 51 additions & 63 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

main.py

Lines changed: 9 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,6 @@
2828
from server.health import HealthServer
2929
from server.player_service import PlayerService
3030
from server.profiler import Profiler
31-
from server.protocol import QDataStreamProtocol, SimpleJsonProtocol
3231
from server.timing import datetime_now
3332

3433

@@ -115,34 +114,16 @@ def done_handler(sig: int, frame):
115114

116115
await instance.start_services()
117116

118-
PROTO_CLASSES = {
119-
QDataStreamProtocol.__name__: QDataStreamProtocol,
120-
SimpleJsonProtocol.__name__: SimpleJsonProtocol
121-
}
122-
for cfg in config.LISTEN:
123-
try:
124-
host = cfg["ADDRESS"]
125-
port = cfg["PORT"]
126-
proto_class_name = cfg["PROTOCOL"]
127-
name = cfg.get("NAME")
128-
proxy = cfg.get("PROXY", False)
129-
130-
proto_class = PROTO_CLASSES[proto_class_name]
131-
132-
await instance.listen(
133-
address=(host, port),
134-
name=name,
135-
protocol_class=proto_class,
136-
proxy=proxy
137-
)
138-
except Exception as e:
139-
raise RuntimeError(f"Error with server instance config: {cfg}") from e
140-
141-
if not instance.contexts:
142-
raise RuntimeError(
143-
"The server was not configured to listen on any ports! Check the "
144-
"config file and try again."
117+
try:
118+
await instance.listen(
119+
address=(config.WS_HOST, config.WS_PORT),
120+
path=config.WS_PATH,
145121
)
122+
except Exception as e:
123+
raise RuntimeError(
124+
f"Error starting WebSocket listener on "
125+
f"{config.WS_HOST}:{config.WS_PORT}{config.WS_PATH}"
126+
) from e
146127

147128
server.metrics.info.info({
148129
"version": info.VERSION,

minikube-example.yaml

Lines changed: 4 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -9,10 +9,8 @@ spec:
99
selector:
1010
app: faf-lobby
1111
ports:
12-
- port: 8001
13-
name: qstream
14-
- port: 8002
15-
name: simplejson
12+
- port: 8003
13+
name: websocket
1614
---
1715
apiVersion: apps/v1
1816
kind: Deployment
@@ -44,10 +42,8 @@ spec:
4442
name: control
4543
- containerPort: 2000
4644
name: health
47-
- containerPort: 8001
48-
name: qstream
49-
- containerPort: 8002
50-
name: simplejson
45+
- containerPort: 8003
46+
name: websocket
5147
env:
5248
- name: CONFIGURATION_FILE
5349
value: /config/config.yaml

server/__init__.py

Lines changed: 6 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -133,7 +133,6 @@
133133
from .oauth_service import OAuthService
134134
from .party_service import PartyService
135135
from .player_service import PlayerService
136-
from .protocol import Protocol, QDataStreamProtocol
137136
from .rating_service.rating_service import RatingService
138137
from .servercontext import ServerContext
139138
from .stats.game_stats_service import GameStatsService
@@ -258,30 +257,25 @@ async def listen(
258257
self,
259258
address: tuple[str, int],
260259
name: Optional[str] = None,
261-
protocol_class: type[Protocol] = QDataStreamProtocol,
262-
proxy: bool = False,
260+
path: str = "/",
263261
) -> ServerContext:
264262
"""
265-
Start listening on a new address.
263+
Start listening for WebSocket connections on a new address.
266264
267265
# Params
268266
- `address`: Tuple indicating the host, port to listen on.
269-
- `name`: String used to identify this context in log messages. The
270-
default is to use the `protocol_class` name.
271-
- `protocol_class`: The protocol class implementation to use.
272-
- `proxy`: Boolean indicating whether or not to use the PROXY protocol.
273-
See: https://www.haproxy.org/download/1.8/doc/proxy-protocol.txt
267+
- `name`: String used to identify this context in log messages.
268+
- `path`: HTTP path on which to expose the WebSocket endpoint.
274269
"""
275270
if not self.started:
276271
await self.start_services()
277272

278273
ctx = ServerContext(
279-
f"{self.name}[{name or protocol_class.__name__}]",
274+
f"{self.name}[{name or 'WebSocket'}]",
280275
self.connection_factory,
281276
list(self.services.values()),
282-
protocol_class
283277
)
284-
await ctx.listen(*address, proxy=proxy)
278+
await ctx.listen(*address, path=path)
285279

286280
self.contexts.add(ctx)
287281

server/config.py

Lines changed: 9 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -44,22 +44,15 @@ def __init__(self):
4444
Change default values here.
4545
"""
4646
self.CONFIGURATION_REFRESH_TIME = 300
47-
self.LISTEN = [
48-
{
49-
"ADDRESS": "",
50-
"PORT": 8001,
51-
"NAME": None,
52-
"PROTOCOL": "QDataStreamProtocol",
53-
"PROXY": False,
54-
},
55-
{
56-
"ADDRESS": "",
57-
"PORT": 8002,
58-
"NAME": None,
59-
"PROTOCOL": "SimpleJsonProtocol",
60-
"PROXY": False
61-
}
62-
]
47+
self.WS_HOST = ""
48+
self.WS_PORT = 8003
49+
self.WS_PATH = "/"
50+
# Name of an HTTP header set by a trusted reverse proxy that contains
51+
# the real client IP. Default "X-Real-IP" matches what Cloudflare /
52+
# Traefik set. Set to "" to always use the direct TCP peer address
53+
# when the server is exposed to untrusted clients (these headers are
54+
# easily spoofed otherwise).
55+
self.WS_FORWARDED_IP_HEADER = "X-Real-IP"
6356
self.LOG_LEVEL = "DEBUG"
6457
# Whether or not to use uvloop as a drop-in replacement for asyncio's
6558
# default event loop

server/protocol/__init__.py

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,12 +13,14 @@
1313
from .protocol import DisconnectedError, Protocol
1414
from .qdatastream import QDataStreamProtocol
1515
from .simple_json import SimpleJsonProtocol
16+
from .websocket import WebSocketProtocol
1617

1718
__all__ = (
1819
"DisconnectedError",
1920
"GpgNetClientProtocol",
2021
"GpgNetServerProtocol",
2122
"Protocol",
2223
"QDataStreamProtocol",
23-
"SimpleJsonProtocol"
24+
"SimpleJsonProtocol",
25+
"WebSocketProtocol",
2426
)

0 commit comments

Comments
 (0)