Accepting and getting connections on the threading server without launching a new thread
Shir0kamii opened this issue · 7 comments
Hi,
In the company I work for, we are building a software that acts as a man-in-the-middle between some hardware and a server it connects to. We plan to write integrations tests using your library. The tests we would like to write with pytest would look like this:
def test_anything(server_connection, client_connection):
client_connection.send("foo")
msg = server_connection.recv()
# test how our software affected the message between client and server
assert msg == "foo123"
For this, we need to be able to accept a connection on the websocket server without spinning a new thread with a handler.
We are planning to fork your library and implement that feature on our side. I could open a MR if this is something that you are interested in integrating, but if so, I would like to know how you would see it done.
API-wise, i'm thinking something like this would look fine:
# Notice we don't pass a handler since we won't need it
# We could also pass a dumb handler if we want to keep it mandatory
with websocket.sync.server.serve(host="localhost", port=8000) as server:
server_connection = server.accept()
For this to work, the websocket subprotocol selection and handshake would need to be done outside the request handler, maybe in the new accept
method, which would also contain the polling code. The serve_forever
method would then be calling accept
and launching threads with the provided custom handler and the accepted websocket connection.
What do you think about that? Do you think other people could have the same use case?
TL;DR: We will fork your library to add the feature to accept connections without launching a handler in a new thread. We are willing to upstream the feature, so discussing how to implement it ahead of time would be better if it interest you.
PS: This is for the treading server, but the same could be done for the asyncio server in a separate PR on my own time.
To make sure we're clear on the setup:
- you want to run the server in another thread (so it runs in the background while the test code runs in the main thread)
- you don't want the server to spawn new threads for each client connection
Can you confirm?
FYI here's how I would do implement an end-to-end test. I would run the following connection handler:
server_connection = None
connection_ready = threading.Event()
connection_done = threading.Event()
def handler(websocket):
global server_connection, connection_ready, connection_done
try:
server_connection = websocket
connection_ready.set()
connection_done.wait()
finally:
connection_ready = threading.Event()
connection_done = threading.Event()
Then the tests look like:
def test_anything():
with start_server() as server_address: // spawns a new thread and runs the server there; closes the server and joins the thread on exit; left as an exercise for the reader
client_connection = start_client_connection(server_address) // not sure how you tell your hardware to connect!
connection_ready.wait()
global server_connection // now you can use server_connection
try:
// test goes here
finally:
connection_done.set() // terminate handler
Then you can add ungodly amounts of pytest magic (server_connection and client_connection fixtures, etc.) to make the API shorter :-)
A word of caution threads + unittest is hell; it's very easy to end up in a situation where tests just deadlock. I recommend to pay attention to terminating connections and threads cleanly; that's a worthwhile investment. (And yes I know that websockets' own test suite is a bit flaky -- the cobbler's children, etc.)
Or, if you want to go low level, you could also use the Sans-I/O layer for testing.
Docs here: https://websockets.readthedocs.io/en/stable/howto/sansio.html
Essentially you'd receive a TCP connection from your hardware and pass them to the Sans-I/O layer. Perhaps you can have a minimalistic bridge that's good enough for your tests. That would likely still be less work than forking this library.
Wait - I have a far simpler idea - instantiate directly a ServerConnection - that should just work?
import socket
import websockets.server
import websockets.sync.server
sock = socket.create_server((HOST, PORT))
conn, addr = sock.accept()
server_connection = websockets.sync.server.ServerConnection(conn, websockets.server.ServerProtocol())
At that point, you can do what you want, I think.
You just bypassed all the server machinery by accepting a server connection.
Also I think it's going to be much easier to make that version robust; cleanup is as simple as:
server_connection.close()
conn.close()
sock.close()
(+ exception handling)
If that last option works, I'll add it to the FAQ.
Indeed, the last option is what would work best in our case, thanks for the suggestion!
As they say - third time is the charm :-)