Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Accepting and getting connections on the threading server without launching a new thread #1421

Open
Shir0kamii opened this issue Dec 8, 2023 · 6 comments

Comments

@Shir0kamii
Copy link

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.

@aaugustin
Copy link
Member

aaugustin commented Dec 11, 2023

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.)

@aaugustin
Copy link
Member

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.

@aaugustin
Copy link
Member

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)

@aaugustin
Copy link
Member

If that last option works, I'll add it to the FAQ.

@Shir0kamii
Copy link
Author

Indeed, the last option is what would work best in our case, thanks for the suggestion!

@aaugustin
Copy link
Member

As they say - third time is the charm :-)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants