wkeeling / selenium-wire

Extends Selenium's Python bindings to give you the ability to inspect requests made by the browser.
MIT License
1.89k stars 248 forks source link

WSS Connections being closed/without reference after del driver.requests #585

Closed montovaneli closed 2 years ago

montovaneli commented 2 years ago

Hey,

I'm using Windows 10 and Python 3.10.

I was trying to clear req.ws_messages only, but when I user del driver.requests, it seems to disconnect/lose reference to the WSS connections, so I need to driver.refresh() to reconnect.

I tested it using any live from tiktok.com/live

If I simply check len(req.ws_messages) after del driver.requests, it returns nothing until I refresh the driver.

for req in driver.requests:
    if req.ws_messages:
        print(len(req.ws_messages)) # prints a number

del driver.requests

while 1:
    for req in driver.requests:
        if req.ws_messages:
            print(len(req.ws_messages)) # prints nothing until I refresh the page.

The first question is: Is it the expected behavior?

The second one is that I've managed to clear only req.ws_messages adding:

clear_ws_messages function to the RequestStorage class:

def clear_ws_messages(self) -> None:
    with self._lock:
        self._ws_messages.clear()

And a ws_messages property to the InspectRequestsMixin class:

@property
def ws_messages(self) -> List[WebSocketMessage]:
    """Retrieves the WebSocket messages sent between the browser and server.

    Captured messages can be cleared with 'del', e.g:

        del firefox.ws_messages

    Returns:
        A list of WebSocketMessage instances representing the messages sent
        between the browser and server.
    """
    return self.backend.storage.load_ws_messages()

@ws_messages.deleter
def ws_messages(self):
    self.backend.storage.clear_ws_messages()

But I'm worried about the implications of these changes.

Thanks!

wkeeling commented 2 years ago

Hello and thanks for raising this.

The reason that del driver.requests loses reference to the WSS connections is because it clears out all top level requests, including the original WSS handshake requests against which the websocket messages are attached. As new websocket messages flow in, they continue to be captured, but you can't physically access them because their parent handshake requests no longer appear in driver.requests.

The code you've added I think is fine for clearing out all websocket messages stored globally. The only implication I can see is more from a design perspective: websocket messages are held per-request (per handshake), and are ideally accessed via that handshake request. Exposing them globally on the driver is thus perhaps less logical - since driver.ws_messages would presumably give you every message across all handshake requests.

But if clearing them out is the primary objective, then I think this is fine. I may look at implementing this as a feature, but perhaps as a specific method call. A few people have commented on related websocket issues so it could be useful to have.

Thanks again.

wkeeling commented 2 years ago

There's also a potential workaround. Rather than using del driver.requests, you may be able to clear out just the websocket messages by identifying the handshake requests, e.g.

for request in driver.requests:
    if request.url.startswith("wss://"):
        request.ws_messages.clear()