[py]: allow CDP and BiDi event callbacks to determine event triggering order

[nxs7]

User description

🔗 Related Issues

An implementation of #16280

💥 What does this PR do?

This PR adds a new function, WebSocketConnection.add_callback_with_sequence, to provide event callbacks with an additional parameter, the event sequence number. This allows callbacks to properly order and process events that are fired repeatedly in a short time frame.

The implementation referred to #13921.

🔄 Types of changes

• New feature (non-breaking change which adds functionality and tests!)

---

PR Type

Enhancement

---

Description

• Add sequence number tracking to WebSocket event callbacks

• Enable event ordering for CDP and BiDi callbacks

• Maintain backward compatibility with existing callback interface

• Support concurrent event processing with proper sequencing

---

Diagram Walkthrough

flowchart LR
  A["WebSocket Message"] --> B["Increment Sequence"]
  B --> C["Process Message with Sequence"]
  C --> D["Execute Callbacks with Sequence"]
  D --> E["Event Ordering Preserved"]

Loading

File Walkthrough

Relevant files

Enhancement

websocket_connection.py Add sequence tracking to WebSocket callbacks                          py/selenium/webdriver/remote/websocket_connection.py Add _sequence counter for tracking event order Create add_callback_with_sequence method for sequence-aware callbacks Modify add_callback to use new sequence method with lambda wrapper Update message processing to pass sequence numbers to callbacks +10/-5

---

[qodo-merge-pro]

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪

🧪 No relevant tests

🔒 No security concerns identified

⚡ Recommended focus areas for review Thread-safety _sequence is incremented from the WebSocket thread without synchronization. If multiple threads can call on_message or if callbacks depend on strict monotonicity across threads, consider using a thread-safe counter or a lock to avoid race conditions and ensure consistent sequencing. self._sequence += 1 self._process_message(self._sequence, message) Backward compatibility add_callback now wraps the user callback via a lambda that ignores the sequence. Ensure the signature matches existing expectations (no extra args) and that exceptions raised inside the wrapped callback are handled consistently as before. def add_callback(self, event, callback): self.add_callback_with_sequence(event, lambda _, x: callback(x)) def add_callback_with_sequence(self, event, callback): event_name = event.event_class if event_name not in self.callbacks: self.callbacks[event_name] = [] def _callback(sequence, params): callback(sequence, event.from_json(params)) self.callbacks[event_name].append(_callback) Error handling in threads Callback execution is dispatched to new Threads; uncaught exceptions will be lost and may terminate threads silently. Consider logging exceptions around callback invocation to aid debugging and maintain observability. params = message["params"] for callback in self.callbacks.get(message["method"], []): Thread(target=callback, args=(sequence, params)).start()

[qodo-merge-pro]

PR Code Suggestions ✨

Explore these optional code suggestions:

Category	Suggestion	Impact
General	Guard callback with exception handling Wrap the callback invocation in a try/except to prevent unhandled exceptions from killing the thread silently. Log the exception with context so failures are visible and diagnosable. py/selenium/webdriver/remote/websocket_connection.py [89-90] def _callback(sequence, params): - callback(sequence, event.from_json(params)) + try: + callback(sequence, event.from_json(params)) + except Exception: + logger.exception("Error in event callback for %s", event_name) Apply / Chat Suggestion importance[1-10]: 8 __ Why: This is a valuable suggestion for robustness, as an unhandled exception within the callback would silently terminate the thread, making debugging difficult. Adding try...except with logging is a crucial improvement.	Medium
Possible issue	Make callback threads daemon Mark per-callback threads as daemon so they don't block interpreter shutdown if callbacks remain running. This prevents process hangs during teardown. py/selenium/webdriver/remote/websocket_connection.py [147] -Thread(target=callback, args=(sequence, params)).start() +t = Thread(target=callback, args=(sequence, params)) +t.daemon = True +t.start() Apply / Chat Suggestion importance[1-10]: 7 __ Why: The suggestion correctly identifies that non-daemon threads for callbacks can prevent the main process from exiting. Making them daemon threads is a good practice that improves the application's shutdown behavior.	Medium
Learned best practice	Validate callback input Validate that callback is callable before registering it to prevent runtime failures when invoked in a thread. py/selenium/webdriver/remote/websocket_connection.py [84-93] def add_callback_with_sequence(self, event, callback): + if not callable(callback): + raise TypeError("callback must be callable") event_name = event.event_class if event_name not in self.callbacks: self.callbacks[event_name] = [] def _callback(sequence, params): callback(sequence, event.from_json(params)) self.callbacks[event_name].append(_callback) return id(_callback) Apply / Chat Suggestion importance[1-10]: 6 __ Why: Relevant best practice - Validate inputs early to fail fast and avoid runtime errors.	Low
Update

[Delta456]

Can you add a test for this?

[cgoldberg]

I opened #16294 for the "daemon threads" issue that the AI bot suggested. We can handle that outside this PR.

[nxs7]

@Delta456 I'd appreciate some suggestions. Since add_callback now calls add_callback_with_sequence internally, the existing tests already cover most of its functionality, except the sequence parameter. However, I don't see an obvious way to test it.

Any ideas?