diff --git a/SPEC.md b/SPEC.md index a9f44f6..1833148 100644 --- a/SPEC.md +++ b/SPEC.md @@ -485,6 +485,30 @@ A client should nonetheless be prepared to receive a **text** frame where it expected binary, and treat it as a fatal server-side error rather than attempting to unpack it. +### 7.5 The client stopping + +Everything above is the server closing. A client may also stop first - it +has collected the episodes it was asked for, or its operator interrupted it - +and that is not an error. The server keeps no state that outlives the +connection, so a client that disappears costs nothing beyond the feedback it +had not yet sent. + +A client that is finished **SHOULD** send a WebSocket close frame with status +**1000 (normal closure)** before dropping the socket, as RFC 6455 section +5.5.1 asks. Nothing breaks without one - the server sees the connection end +either way - but the difference is visible in its logs, as +`ConnectionClosedError: no close frame received or sent` rather than a clean +`ConnectionClosedOK`, and an operator reading those logs should not have to +wonder whether a client crashed. + +`examples/conformance_server.py` reports a missing close frame as a note, not +a violation, which is the level this rule deserves. + +> **Historical note.** Until 2026-09-11 the C++ reference client did not send +> one. It went unnoticed because in every test until then the *server* ran +> out of steps first and closed the connection itself; E7, where the client +> finishes first, made it visible on all 45 runs. + --- ## 8. Conformance checklist diff --git a/examples/README.md b/examples/README.md index 2f6bca8..b62f592 100644 --- a/examples/README.md +++ b/examples/README.md @@ -41,7 +41,7 @@ see whether the values it prints are the ones the server sent. |---|---|---| | Language | Python | C++17 | | Dependencies | `msgpack`, `websockets` | **none** | -| Lines | 275 | 814 | +| Lines | 275 | 843 | | Notably absent | numpy, and every `plugrl_*` package | libstdc++ and libc are the only links | The C++ one is the interesting case. It was written on a machine with no diff --git a/examples/conformance_server.py b/examples/conformance_server.py index fde59ca..40444f7 100644 --- a/examples/conformance_server.py +++ b/examples/conformance_server.py @@ -31,6 +31,7 @@ import numpy as np import websockets.asyncio.server as ws_server +import websockets.exceptions as ws_exceptions # Use the real codec, not a re-implementation: the point is to test a client # against what plugrl-server actually does. @@ -245,6 +246,21 @@ async def handle(self, websocket) -> None: self.check_feedback(payload) awaiting = str(MessageType.INFER) self.exchanges += 1 + except ws_exceptions.ConnectionClosedOK: + # The client said goodbye and left. SPEC.md section 7.5 allows + # that at any point, so it is not a failure - a client that has + # collected the episodes it wanted is finished, and the server + # does not get to call that a protocol violation. + report.ok("7.5 a client that stops sends a close frame") + except ws_exceptions.ConnectionClosedError: + # It vanished without a close frame. Section 7.5 asks for one, + # and nothing breaks without it, so this is a note. + report.advise( + False, + "7.5 a client that stops sends a close frame", + "the connection dropped with no close frame; RFC 6455 asks " + "for one and SPEC.md section 7.5 repeats the ask", + ) except Exception as exc: # noqa: BLE001 - the report is the output report.fail("connection", f"{type(exc).__name__}: {exc}") finally: diff --git a/examples/plugrl_client.cpp b/examples/plugrl_client.cpp index eb28927..4f6fce6 100644 --- a/examples/plugrl_client.cpp +++ b/examples/plugrl_client.cpp @@ -445,7 +445,36 @@ class WebSocket { handshake(host, port); } - ~WebSocket() { if (fd_ >= 0) ::close(fd_); } + // RFC 6455 section 5.5.1: say goodbye before dropping the socket. Without + // this the server sees the connection vanish and reports + // `ConnectionClosedError: no close frame received or sent` - which is what + // it did on all 45 runs of E7, where the client finishes first. It only + // went unnoticed before because in every earlier test the *server* ran out + // of steps first and closed the connection itself. + void close_cleanly() { + if (fd_ < 0) return; + std::string frame; + frame.push_back(static_cast(0x88)); // FIN + close opcode + frame.push_back(static_cast(0x80 | 2)); // masked, 2-byte payload + uint8_t key[4]; + for (int i = 0; i < 4; ++i) key[i] = static_cast(rng_() & 0xff); + frame.append(reinterpret_cast(key), 4); + uint8_t status[2] = {0x03, 0xe8}; // 1000, normal closure + for (int i = 0; i < 2; ++i) + frame.push_back(static_cast(status[i] ^ key[i % 4])); + try { + write_all(frame.data(), frame.size()); + } catch (const std::exception&) { + // Already gone. Nothing useful to do on the way out. + } + ::close(fd_); + fd_ = -1; + } + + ~WebSocket() { + close_cleanly(); + if (fd_ >= 0) ::close(fd_); + } void send_binary(const std::string& payload) { std::string frame;