From d1ae5a080bd413f0e4d0f122c260a6e46f67c5f5 Mon Sep 17 00:00:00 2001 From: Vishnu Vardhan Date: Wed, 19 Aug 2026 15:23:18 +0530 Subject: [PATCH 01/12] fix(selenium-devtools-py): capture network on selenium 4.44+ --- .../src/selenium_devtools/bidi.py | 218 +++++++++++++++--- .../src/selenium_devtools/constants.py | 15 +- .../selenium-devtools-py/tests/test_bidi.py | 215 ++++++++++++++++- 3 files changed, 397 insertions(+), 51 deletions(-) diff --git a/packages/selenium-devtools-py/src/selenium_devtools/bidi.py b/packages/selenium-devtools-py/src/selenium_devtools/bidi.py index 0b966a53..2727d481 100644 --- a/packages/selenium-devtools-py/src/selenium_devtools/bidi.py +++ b/packages/selenium-devtools-py/src/selenium_devtools/bidi.py @@ -6,24 +6,32 @@ without selenium; ``attach`` does the selenium wiring and is defensive — a BiDi failure is a logged no-op, never a raised error into the user's test. -Two selenium-version realities shape this module (selenium 4.36): +Two constants shape this module: * BiDi only opens when the session was created with ``webSocketUrl`` truthy (``options.web_socket_url = True`` at build). We can't set that from inside the ``execute`` wrapper — the session already exists — so attach() checks the capability and degrades if it's missing. -* selenium's high-level ``network.add_request_handler`` *intercepts* (pauses) - requests. We deliberately avoid it: we subscribe to the network events via - the low-level connection so requests are observed but never stalled. - -The network half of that has a ceiling: selenium 4.44 regenerated the BiDi layer -from a schema, dropping ``NetworkEvent`` and renaming ``Network.conn`` to -``_conn``, so ``_attach_network`` degrades to a warning there and the Network tab -stays empty. ``pyproject.toml`` caps the extra below it. The observe-only -replacement is ``Network._event_manager.add_event_handler``, whose callback -receives a deserialized event rather than one carrying ``.params`` — a port, not -a rename, tracked as issue #293. Console capture and the preload are unaffected; -``tests/test_selenium_surface.py`` guards all of it against the installed version. +* Network events are OBSERVED, never intercepted. Every selenium API that takes + a request or response handler registers an intercept, which pauses each + request until selenium continues it; that would change the timing of the page + under test. Both paths below subscribe instead. + +There are two of those paths because selenium 4.44 regenerated the BiDi layer +from a schema, and the pre-4.44 one no longer exists there: + +* **4.44+** — ``Network.add_event_handler``, public and observe-only. Its + generated event dataclasses are lossy (``BeforeRequestSentParameters`` + declares only ``initiator``, and its deserializer DROPS every param not + declared, taking the request, its id and the timestamp with it), so + ``register_raw_event_configs`` registers configs whose event class is ``dict`` + and the handlers receive raw params. +* **≤4.43** — ``driver.network.conn`` plus ``NetworkEvent``, which is all those + versions offer. + +``event_params`` is what lets one pair of handlers serve both, and the mapping +helpers below never learn which path ran. ``tests/test_selenium_surface.py`` +guards both surfaces against the installed selenium. """ from __future__ import annotations @@ -36,6 +44,7 @@ BIDI_CAPABILITY, BIDI_LEVEL_MAP, BIDI_NET_BEFORE_REQUEST, + BIDI_NET_EVENT_KEYS, BIDI_NET_RESPONSE_COMPLETED, LOGGER_NAME, SELENIUM_NETWORK_SURFACE_MOVED_AT, @@ -353,64 +362,193 @@ def on_js_error(entry: Any) -> None: def network_unavailable_reason(exc: Exception) -> str: - """Why network capture is off, naming the selenium version when that is why. - - The bare exception is unreadable as a cause: on selenium 4.44+ it surfaces as - ``cannot import name 'NetworkEvent'``, which reads like a broken install - rather than a version the adapter has not caught up with. Console capture and - the DOM preload keep working, so this warning is the only signal the user - gets that the Network tab will stay empty. + """Why the connection path could not attach. + + Reached on 4.44+ only when ``register_raw_event_configs`` declined, i.e. the + regenerated layer is installed but did not present the API it is defined by. + The bare exception there is ``cannot import name 'NetworkEvent'``, which + reads like a broken install rather than a path that does not apply — so that + case says what it means and asks for a report, because it is a combination + the adapter does not know about. """ version = selenium_version() if version >= SELENIUM_NETWORK_SURFACE_MOVED_AT: installed = ".".join(str(part) for part in version) moved_at = ".".join(str(p) for p in SELENIUM_NETWORK_SURFACE_MOVED_AT) return ( - f"network capture is unavailable on selenium {installed}: selenium " - f"{moved_at} regenerated the BiDi layer and moved the internals this " - "subscribes through. Console, DOM and command capture are " - f"unaffected, and selenium < {moved_at} captures network. Tracked at " - "https://github.com/webdriverio/devtools/issues/293" + f"network capture could not attach on selenium {installed}: its " + f"{moved_at}+ event-handler API was not usable, and the older " + f"connection path does not exist there ({exc}). Console, DOM and " + "command capture are unaffected. Please report this with your " + "selenium version: https://github.com/webdriverio/devtools/issues" ) return f"network channel unavailable: {exc}" -def _attach_network(driver: Any, capturer: SessionCapturer) -> bool: - """Subscribe to network events WITHOUT interception (see module docstring). +def event_params(event: Any) -> Dict[str, Any]: + """The BiDi event params dict, from either subscription path. - Uses the low-level connection so requests are only observed. Returns False - (and logs) on any failure — network BiDi is best-effort. + Legacy selenium hands the callback a ``NetworkEvent`` carrying ``.params``; + 4.44+ hands it whatever its deserializer produced, which is the raw params + dict for the configs registered by ``register_raw_event_configs``. Both + collapse here so the mapping helpers below only ever see a plain dict. + """ + if isinstance(event, dict): + return event + params = getattr(event, "params", None) + return params if isinstance(params, dict) else {} + + +def register_raw_event_configs() -> bool: + """Register raw-params event configs on selenium 4.44+. False = legacy path. + + MUST run before anything touches ``driver.network``: the deserializers are + built once, in ``Network.__init__``, from whatever configs exist then. + + Registered under the adapter's own keys so the result does not depend on + selenium's own duplicate config entries — it ships two keys mapping to + ``network.beforeRequestSent`` with different classes, and which one wins is + decided by dict iteration order. Ours are added last and win either way. + + Side effect worth knowing: deserializers are keyed by BiDi event, not by + config key, so this also makes selenium's own ``response_completed`` handlers + receive the raw dict in this process. Nothing else in a test run subscribes + to it, and the dict carries strictly more than the dataclass it replaces. """ try: - conn = driver.network.conn - from selenium.webdriver.common.bidi.network import NetworkEvent # lazy - from selenium.webdriver.common.bidi.session import Session # lazy - except Exception as exc: # noqa: BLE001 - _warn(network_unavailable_reason(exc)) + from selenium.webdriver.common.bidi.network import EventConfig, Network + except ImportError: + return False # ≤4.43: no generated layer, use the connection path + configs = getattr(Network, "EVENT_CONFIGS", None) + if not isinstance(configs, dict) or not hasattr(Network, "add_event_handler"): return False + for bidi_event, key in BIDI_NET_EVENT_KEYS.items(): + configs.setdefault(key, EventConfig(key, bidi_event, dict)) + return True + + +_reported_incomplete: set = set() + + +def _incomplete_event(params: Dict[str, Any], label: str) -> bool: + """True (and warns ONCE per event type) when an event arrived without the + fields capture needs. + + The 4.44+ path relies on selenium deserializing to the raw params, so a + future release that reinstates a typed class for these events would strip + the request and timestamp and leave every entry uncorrelated. That would + otherwise show up as a quietly empty Network tab — the exact failure this + port exists to end — so it is stated instead. + + Once, because the condition is a property of the selenium build rather than + of a request: warning per event would put one line per HTTP request of the + run into the user's console and bury the message it is trying to deliver. + """ + if params.get("request") is not None: + return False + if label not in _reported_incomplete: + _reported_incomplete.add(label) + _warn( + f"{label} arrived without a request field, so it cannot be " + f"correlated — selenium delivered {sorted(params) or 'nothing'}. " + "Network capture is degraded; please report this with your selenium " + "version. Further occurrences are not logged." + ) + return True + + +def _attach_network(driver: Any, capturer: SessionCapturer) -> bool: + """Subscribe to network events WITHOUT interception (see module docstring). + + Returns False (and logs) on any failure — network BiDi is best-effort. + """ + use_event_manager = register_raw_event_configs() pending: Dict[str, Dict[str, Any]] = {} def on_request_sent(event: Any) -> None: try: - kwargs = request_sent_kwargs(getattr(event, "params", {}) or {}) + params = event_params(event) + if _incomplete_event(params, BIDI_NET_BEFORE_REQUEST): + return + kwargs = request_sent_kwargs(params) if kwargs is not None: pending[kwargs["request_id"]] = kwargs capturer.capture_network(**kwargs) except Exception as exc: # noqa: BLE001 _warn(f"beforeRequestSent handler threw: {exc}") + captured = {"n": 0} + def on_response_completed(event: Any) -> None: try: - kwargs = response_completed_kwargs( - getattr(event, "params", {}) or {}, pending - ) + params = event_params(event) + if _incomplete_event(params, BIDI_NET_RESPONSE_COMPLETED): + return + kwargs = response_completed_kwargs(params, pending) if kwargs is not None: pending.pop(kwargs["request_id"], None) capturer.capture_network(**kwargs) + captured["n"] += 1 + # The first one is the proof the subscription is live end to + # end; the rest are a count, because an empty Network tab and a + # tab nobody looked at are indistinguishable after the fact. + if captured["n"] == 1: + _log.info("network capture live, first response: %s", kwargs["url"]) + _log.debug("network entries captured: %d", captured["n"]) except Exception as exc: # noqa: BLE001 _warn(f"responseCompleted handler threw: {exc}") + if use_event_manager: + return _subscribe_via_event_manager( + driver, on_request_sent, on_response_completed + ) + return _subscribe_via_connection(driver, on_request_sent, on_response_completed) + + +def _subscribe_via_event_manager( + driver: Any, on_request_sent: Any, on_response_completed: Any +) -> bool: + """selenium 4.44+: subscribe through the public ``add_event_handler``. + + Deliberately not ``add_request_handler``/``add_response_handler``: both + register an intercept even in their high-level form, which pauses every + request until selenium continues it. This only observes. + """ + try: + network = driver.network + network.add_event_handler( + BIDI_NET_EVENT_KEYS[BIDI_NET_BEFORE_REQUEST], on_request_sent + ) + network.add_event_handler( + BIDI_NET_EVENT_KEYS[BIDI_NET_RESPONSE_COMPLETED], on_response_completed + ) + _log.info( + "network capture subscribed via the event-handler API (selenium %s)", + ".".join(str(p) for p in selenium_version()), + ) + return True + except Exception as exc: # noqa: BLE001 + _warn(f"network subscribe failed: {exc}") + return False + +def _subscribe_via_connection( + driver: Any, on_request_sent: Any, on_response_completed: Any +) -> bool: + """selenium ≤4.43: subscribe over the low-level connection. + + Kept because it is the only path on those versions, not as a fallback for a + 4.44+ failure — there `driver.network.conn` and ``NetworkEvent`` do not + exist, so this cannot recover anything the path above could not do. + """ + try: + conn = driver.network.conn + from selenium.webdriver.common.bidi.network import NetworkEvent # lazy + from selenium.webdriver.common.bidi.session import Session # lazy + except Exception as exc: # noqa: BLE001 + _warn(network_unavailable_reason(exc)) + return False + try: conn.execute( Session(conn).subscribe( @@ -421,6 +559,10 @@ def on_response_completed(event: Any) -> None: conn.add_callback( NetworkEvent(BIDI_NET_RESPONSE_COMPLETED), on_response_completed ) + _log.info( + "network capture subscribed via the connection (selenium %s)", + ".".join(str(p) for p in selenium_version()), + ) return True except Exception as exc: # noqa: BLE001 _warn(f"network subscribe failed: {exc}") diff --git a/packages/selenium-devtools-py/src/selenium_devtools/constants.py b/packages/selenium-devtools-py/src/selenium_devtools/constants.py index fa9e3e30..ba35627f 100644 --- a/packages/selenium-devtools-py/src/selenium_devtools/constants.py +++ b/packages/selenium-devtools-py/src/selenium_devtools/constants.py @@ -102,10 +102,19 @@ BIDI_NET_RESPONSE_COMPLETED = "network.responseCompleted" # selenium 4.44 regenerated the BiDi layer from a schema: ``NetworkEvent`` left # ``bidi.network`` and ``Network.conn`` became ``_conn``, so the subscribe above -# cannot be built there and network capture degrades to nothing. Console capture -# and the preload are unaffected. One source of truth for that version, read by -# bidi.py (to explain the degradation at runtime) and by the surface guards. +# cannot be built there. One source of truth for that version, read by bidi.py to +# pick its subscription path and by the surface guards. SELENIUM_NETWORK_SURFACE_MOVED_AT = (4, 44) +# Keys the adapter registers into ``Network.EVENT_CONFIGS`` on 4.44+, so its +# handlers receive RAW event params. Registered under our own names rather than +# reusing selenium's, because selenium's generated event dataclasses model only +# each event's own extension field — ``BeforeRequestSentParameters`` declares +# just ``initiator`` — and its deserializer DROPS every param not declared, so +# the typed path loses the request, its id and the timestamp. +BIDI_NET_EVENT_KEYS = { + BIDI_NET_BEFORE_REQUEST: "devtools_before_request_sent", + BIDI_NET_RESPONSE_COMPLETED: "devtools_response_completed", +} # selenium's BiDi log entries already carry lowercase levels; this normalizes # the stragglers to the shared LogLevel union. Unmapped levels fall back to log. BIDI_LEVEL_MAP = { diff --git a/packages/selenium-devtools-py/tests/test_bidi.py b/packages/selenium-devtools-py/tests/test_bidi.py index 0cca117f..b5933a61 100644 --- a/packages/selenium-devtools-py/tests/test_bidi.py +++ b/packages/selenium-devtools-py/tests/test_bidi.py @@ -1,3 +1,5 @@ +import sys +import types import unittest from unittest import mock @@ -371,11 +373,201 @@ def network(self): self.assertTrue(bidi.attach(Driver(), cap)) +def fake_network_module(with_event_manager=True): + """A stand-in for selenium 4.44+'s regenerated `bidi.network`. + + Faked rather than skipped-without-selenium: the installed selenium here is + 4.36, so the 4.44+ path has no other way to be exercised, and it is the path + every new user is on. + """ + module = types.ModuleType("selenium.webdriver.common.bidi.network") + + class EventConfig: + def __init__(self, event_key, bidi_event, event_class): + self.event_key = event_key + self.bidi_event = bidi_event + self.event_class = event_class + + class Network: + EVENT_CONFIGS = {} + + def __init__(self): + self.handlers = {} + # selenium builds its deserializers here, from the configs that + # exist at construction time. Mirrored because that timing is the + # whole reason registration has to come first. + self.deserializers = { + config.bidi_event: config.event_class + for config in self.EVENT_CONFIGS.values() + } + + def add_event_handler(self, event, callback, contexts=None): + # selenium raises for an unregistered key rather than ignoring it. + config = self.EVENT_CONFIGS.get(event) + if config is None: + raise ValueError(f"Event '{event}' not found") + self.handlers[event] = callback + return len(self.handlers) + + if not with_event_manager: + del Network.add_event_handler + del Network.EVENT_CONFIGS + + module.EventConfig = EventConfig + module.Network = Network + return module + + +class NewSeleniumDriver: + """A 4.44+ driver. Records WHEN `.network` is first touched, because the + deserializers are built in `Network.__init__` — registering after that is + too late, and the failure would be silent.""" + + def __init__(self, network, log): + self.caps = {"webSocketUrl": "ws://x"} + self._network = network + self._log = log + + @property + def network(self): + self._log.append("network accessed") + return self._network + + @property + def script(self): + raise RuntimeError("console not under test") + + +class TestTheEventManagerPath(unittest.TestCase): + """selenium 4.44+ — subscribing through the public `add_event_handler`.""" + + def test_raw_params_reach_the_handlers_and_are_captured(self): + module = fake_network_module() + network = module.Network() + capturer = SessionCapturer(FakeTransport()) + + with mock.patch.dict( + sys.modules, {"selenium.webdriver.common.bidi.network": module} + ): + self.assertTrue(bidi._attach_network(NewSeleniumDriver(network, []), capturer)) + + sent = network.handlers["devtools_before_request_sent"] + done = network.handlers["devtools_response_completed"] + + # Raw params, exactly as selenium's dict-config deserializer delivers. + sent({"request": {"request": "R1", "url": "https://x/a.js", "method": "GET"}, + "timestamp": 1000}) + done({"request": {"request": "R1"}, "timestamp": 1200, + "response": {"status": 200, "statusText": "OK", + "mimeType": "text/javascript", "bytesReceived": 12}}) + + # capture_network sends one batch per call, each a list of one frame. + frames = [ + batch[0] for scope, batch in capturer._tx.sent + if scope == "networkRequests" + ] + self.assertEqual(len(frames), 2) + self.assertEqual(frames[0]["url"], "https://x/a.js") + self.assertEqual(frames[1]["status"], 200) # correlated with the request + + def test_configs_are_registered_before_network_is_constructed(self): + """The ordering IS the mechanism. `Network.__init__` builds one + deserializer per event from the configs present at that moment, so + registering after the first `driver.network` silently keeps the lossy + typed path and every entry loses its request id.""" + log = [] + + class RecordingConfigs(dict): + def setdefault(self, *args, **kwargs): + log.append("registered") + return super().setdefault(*args, **kwargs) + + module = fake_network_module() + module.Network.EVENT_CONFIGS = RecordingConfigs() + + with mock.patch.dict( + sys.modules, {"selenium.webdriver.common.bidi.network": module} + ): + bidi._attach_network( + NewSeleniumDriver(module.Network(), log), SessionCapturer(FakeTransport()) + ) + + self.assertEqual(log[0], "registered") + self.assertIn("network accessed", log) + + def test_both_events_register_a_raw_config(self): + module = fake_network_module() + with mock.patch.dict( + sys.modules, {"selenium.webdriver.common.bidi.network": module} + ): + self.assertTrue(bidi.register_raw_event_configs()) + + configs = module.Network.EVENT_CONFIGS + for bidi_event, key in bidi.BIDI_NET_EVENT_KEYS.items(): + self.assertEqual(configs[key].bidi_event, bidi_event) + # dict is what makes selenium pass the params through untouched. + self.assertIs(configs[key].event_class, dict) + + def test_legacy_selenium_keeps_the_connection_path(self): + module = fake_network_module(with_event_manager=False) + with mock.patch.dict( + sys.modules, {"selenium.webdriver.common.bidi.network": module} + ): + self.assertFalse(bidi.register_raw_event_configs()) + + +class TestEventParams(unittest.TestCase): + def test_a_raw_dict_is_its_own_params(self): + self.assertEqual(bidi.event_params({"request": {}}), {"request": {}}) + + def test_a_legacy_event_object_is_unwrapped(self): + event = types.SimpleNamespace(params={"request": {"url": "u"}}) + self.assertEqual(bidi.event_params(event), {"request": {"url": "u"}}) + + def test_anything_else_degrades_to_empty(self): + self.assertEqual(bidi.event_params(object()), {}) + + +class TestADegradedEventIsReported(unittest.TestCase): + """If a future selenium reinstates a typed class for these events, the + request is stripped and nothing can be correlated. That must not look like + a quiet network-free page — it is the failure this port exists to end.""" + + def setUp(self): + bidi._reported_incomplete.clear() + + def test_an_event_without_a_request_warns(self): + with self.assertLogs("selenium_devtools.bidi", level="WARNING") as logs: + self.assertTrue(bidi._incomplete_event({"initiator": {}}, "network.x")) + + self.assertIn("cannot be correlated", "\n".join(logs.output)) + + def test_it_warns_once_per_event_not_once_per_request(self): + # The condition is a property of the selenium build, not of a request: + # warning per event would put one line per HTTP request into the user's + # console and bury the message. + with self.assertLogs("selenium_devtools.bidi", level="WARNING") as logs: + for _ in range(5): + self.assertTrue(bidi._incomplete_event({}, "network.x")) + + self.assertEqual(len(logs.output), 1) + + def test_each_event_type_reports_separately(self): + with self.assertLogs("selenium_devtools.bidi", level="WARNING") as logs: + bidi._incomplete_event({}, "network.beforeRequestSent") + bidi._incomplete_event({}, "network.responseCompleted") + + self.assertEqual(len(logs.output), 2) + + def test_a_complete_event_is_silent(self): + self.assertFalse(bidi._incomplete_event({"request": {"request": "R"}}, "network.x")) + + class TestWhyNetworkCaptureIsOff(unittest.TestCase): - """The user-facing extra is uncapped, so a user CAN be on a selenium whose - BiDi layer moved. This warning is then the only signal that the Network tab - will stay empty, so it has to name the cause rather than echo an ImportError - that reads like a broken install.""" + """The connection path is unreachable on 4.44+, so arriving there means the + regenerated layer is installed but did not present the API that defines it. + That is a combination the adapter does not know about, and the warning has to + say so rather than echo an ImportError that reads like a broken install.""" def test_a_moved_surface_is_reported_as_a_version_gap(self): major, minor = bidi.SELENIUM_NETWORK_SURFACE_MOVED_AT @@ -392,19 +584,22 @@ def test_a_moved_surface_is_reported_as_a_version_gap(self): # the "selenium < X captures network" advice, so a bare assertIn passes # even when the sentence blames the installed version for the move. self.assertIn(f"on selenium {major}.{minor + 1}", reason) - self.assertIn(f"selenium {major}.{minor} regenerated", reason) - self.assertIn("293", reason) # where the fix is tracked - # The half that still works must be said, or this reads as total loss. + self.assertIn(f"{major}.{minor}+ event-handler API", reason) + # The underlying error, and the half that still works — without it this + # reads as total loss. + self.assertIn("cannot import name", reason) self.assertIn("Console", reason) + self.assertIn("report", reason) def test_an_ordinary_failure_still_reports_the_exception(self): - # Below the moved version the cause is NOT the selenium release, so - # blaming it would send the reader somewhere with no answer. + # Below the moved version the connection path is the ONLY path, so a + # failure there is ordinary and blaming the selenium release would send + # the reader somewhere with no answer. with mock.patch.object(bidi, "selenium_version", return_value=(4, 36)): reason = bidi.network_unavailable_reason(RuntimeError("no bidi socket")) self.assertIn("no bidi socket", reason) - self.assertNotIn("293", reason) + self.assertNotIn("event-handler API", reason) if __name__ == "__main__": From 05c40c2cbd3556fb662298407d0c8f4213619a1c Mon Sep 17 00:00:00 2001 From: Vishnu Vardhan Date: Wed, 19 Aug 2026 15:23:30 +0530 Subject: [PATCH 02/12] test(selenium-devtools-py): guard both selenium BiDi surfaces --- .../tests/test_selenium_surface.py | 123 +++++++++++------- 1 file changed, 77 insertions(+), 46 deletions(-) diff --git a/packages/selenium-devtools-py/tests/test_selenium_surface.py b/packages/selenium-devtools-py/tests/test_selenium_surface.py index 25d5b086..d16666b2 100644 --- a/packages/selenium-devtools-py/tests/test_selenium_surface.py +++ b/packages/selenium-devtools-py/tests/test_selenium_surface.py @@ -12,17 +12,17 @@ That prediction was already history when these first ran: selenium 4.44 shipped the regenerated layer, and CI (which resolves a newer selenium than a local 3.9 -can install) failed on the first run. `NetworkEvent` is gone from `bidi.network` -and `Network.conn` is now `_conn`, so network capture has been silently dead on -4.44+ — no error, just an empty Network tab behind one warning. The observe-only -replacement is `Network._event_manager.add_event_handler`, whose callback takes a -deserialized event object rather than `.params`, so adopting it is a port and not -a rename (issue #293). Until that lands the extra is capped below 4.44. +can install) failed on the first run, on a breakage that had already shipped. + +`bidi.py` now has a path for each surface, so BOTH are guarded here and which +class applies is decided by the installed version. Neither is optional: whichever +one the installed selenium presents is the only thing standing between a working +Network tab and an empty one. Skipped when selenium is absent. That is not free: the CI job must install the adapter's own runtime dependency or these never run where they are meant to protect. selenium is an OPTIONAL extra of this package, so the job must select it — -`pip install -e '.[selenium]'` in `.github/workflows/python.yml`. A plain +`pip install -e '.[test]'` in `.github/workflows/python.yml`. A plain `pip install -e .` installs nothing and every guard here silently skips. """ @@ -41,49 +41,80 @@ @unittest.skipUnless(_HAS_SELENIUM, "selenium is not installed") -class TestTheSupportedSeleniumRange(unittest.TestCase): - def test_the_installed_selenium_still_carries_the_network_surface(self): - """One legible failure for the whole network breakage. - - The two guards below are skipped past that version so this is the only - thing that reports it — a reader gets the version and the reason, not two - stack traces about a missing name and a missing attribute. - - This failing is NOT a broken install: the user-facing extra is uncapped, - so a developer can legitimately have a newer selenium here. It means the - adapter has not caught up, and `bidi.py` says the same thing at runtime - to the user who is actually losing the Network tab.""" - installed = selenium_version() - - self.assertLess( - installed, - SELENIUM_NETWORK_SURFACE_MOVED_AT, - f"selenium {'.'.join(str(p) for p in installed)} is at or past " - f"{'.'.join(str(p) for p in SELENIUM_NETWORK_SURFACE_MOVED_AT)}, " - "which regenerated the BiDi layer: bidi.py subscribes to network " - "events through NetworkEvent and Network.conn, and neither exists " - "any more, so network capture silently degrades to nothing. Console " - "capture and the preload are unaffected. Porting to the new " - "_event_manager surface is issue #293; the `test` extra pins below " - "this release so CI runs the guards on a supported selenium.", +@unittest.skipIf(not _NETWORK_SURFACE_MOVED, "selenium predates the regenerated layer") +class TestTheRegeneratedNetworkSurface(unittest.TestCase): + """selenium 4.44+ — what `_subscribe_via_event_manager` needs to exist. + + `EVENT_CONFIGS` is a public class attribute and `add_event_handler` a public + method, so this is a supported-API dependency rather than reaching inside. + What is NOT public is that one deserializer is built per BiDi event at + `Network.__init__`, which is why registering a `dict` config wins and why it + must happen before the first `driver.network`. That is the fragile part, and + the shape assertions below are what would catch it changing.""" + + def test_the_public_event_handler_api_is_present(self): + from selenium.webdriver.common.bidi.network import Network + + self.assertTrue(callable(getattr(Network, "add_event_handler", None))) + self.assertIsInstance(getattr(Network, "EVENT_CONFIGS", None), dict) + + def test_event_configs_carry_the_two_events_capture_needs(self): + from selenium.webdriver.common.bidi.network import Network + + from selenium_devtools.constants import ( + BIDI_NET_BEFORE_REQUEST, + BIDI_NET_RESPONSE_COMPLETED, ) + # Registration reuses selenium's own EventConfig shape, so the names it + # subscribes by have to be the ones selenium routes on. + wired = {config.bidi_event for config in Network.EVENT_CONFIGS.values()} + self.assertIn(BIDI_NET_BEFORE_REQUEST, wired) + self.assertIn(BIDI_NET_RESPONSE_COMPLETED, wired) + + def test_event_config_takes_the_three_fields_registration_supplies(self): + from selenium.webdriver.common.bidi.network import EventConfig + + config = EventConfig("k", "network.responseCompleted", dict) + + self.assertEqual(config.event_key, "k") + self.assertEqual(config.bidi_event, "network.responseCompleted") + self.assertIs(config.event_class, dict) + + def test_the_generated_event_classes_are_still_lossy(self): + """The reason raw `dict` configs are registered at all. + + If selenium ever models the full event, this fails and the registration + can go — the typed object would then carry the request and timestamp. It + failing is good news, not a break.""" + import dataclasses + + from selenium.webdriver.common.bidi.network import ( + BeforeRequestSentParameters, + ) + + if not dataclasses.is_dataclass(BeforeRequestSentParameters): + self.skipTest("no longer a dataclass — registration needs rechecking") + declared = {f.name for f in dataclasses.fields(BeforeRequestSentParameters)} + self.assertNotIn("request", declared) + self.assertNotIn("timestamp", declared) + @unittest.skipUnless(_HAS_SELENIUM, "selenium is not installed") -@unittest.skipIf( - _NETWORK_SURFACE_MOVED, - "selenium is past the supported range — TestTheSupportedSeleniumRange reports it", -) -class TestTheNetworkInternalsTheAdapterUses(unittest.TestCase): - """`bidi.py` reaches through `driver.network.conn` to subscribe WITHOUT - interception — selenium's `add_request_handler` registers an intercept even - in its high-level style, which pauses each request until selenium continues - it. That is a deliberate trade of public API for not stalling a user's page - loads, and it is what these pin. - - Only this class is gated on the cap: the console, preload and driver-channel - guards below still hold on newer selenium, and skipping them there would - drop the coverage exactly where a bump is most likely to move something.""" +@unittest.skipIf(_NETWORK_SURFACE_MOVED, "selenium uses the regenerated surface") +class TestThePreRegenerationNetworkInternals(unittest.TestCase): + """selenium ≤4.43 — what `_subscribe_via_connection` needs to exist. + + It reaches through `driver.network.conn` because every selenium API that + takes a request or response handler registers an intercept, which pauses + each request until selenium continues it. On these versions there is no + observe-only alternative, so the private access is the price of not + changing the timing of the page under test. + + Gated to versions where this path actually runs. The console, preload and + driver-channel guards below are gated on nothing, because they hold on every + version and skipping them would drop coverage where a bump is most likely to + move something.""" def test_the_network_channel_still_carries_the_low_level_connection(self): from selenium.webdriver.common.bidi.network import Network From 18aee41236c6ee6702a74290f435e86ecd0468a4 Mon Sep 17 00:00:00 2001 From: Vishnu Vardhan Date: Wed, 19 Aug 2026 15:23:46 +0530 Subject: [PATCH 03/12] ci(selenium-devtools-py): exercise both selenium surfaces across the matrix --- .github/workflows/python.yml | 9 +++++---- packages/selenium-devtools-py/pyproject.toml | 16 ++++++---------- 2 files changed, 11 insertions(+), 14 deletions(-) diff --git a/.github/workflows/python.yml b/.github/workflows/python.yml index 0fe8f3cd..3cd31045 100644 --- a/.github/workflows/python.yml +++ b/.github/workflows/python.yml @@ -43,10 +43,11 @@ jobs: # so a bump that moved those internals used to pass CI green and degrade # at runtime. Installing the package is what makes those guards real. # - # `[test]`, not `[selenium]`: the test extra is the capped one. The user- - # facing extra is uncapped on purpose (see pyproject), so installing it - # here would resolve a selenium the adapter cannot fully capture on and - # fail the guards for a reason CI cannot fix. + # The matrix is load-bearing, not just breadth: network capture has two + # subscription paths, one per selenium BiDi surface, and the python version + # is what selects which selenium resolves. 3.9 caps at a pre-4.44 selenium + # and exercises the connection path; 3.12 resolves the latest and exercises + # the event-handler path. Dropping either job leaves one path untested. - name: Install the adapter and its runtime dependency run: pip install -e '.[test]' diff --git a/packages/selenium-devtools-py/pyproject.toml b/packages/selenium-devtools-py/pyproject.toml index 0268bf5d..9902fa07 100644 --- a/packages/selenium-devtools-py/pyproject.toml +++ b/packages/selenium-devtools-py/pyproject.toml @@ -17,17 +17,13 @@ keywords = ["selenium", "webdriver", "devtools", "pytest", "debugging"] dependencies = [] [project.optional-dependencies] -# Deliberately UNCAPPED, even though network capture needs selenium < 4.44 (that -# release regenerated the BiDi layer, moving the internals `bidi.py` subscribes -# through; issue #293). A cap here would downgrade anyone already on a newer -# selenium, and hard-fail the install for a project that requires one — a cost -# paid by every user to describe a partial degradation that only affects the -# Network tab. `bidi.py` reports it at runtime instead, naming the version. +# Uncapped on both sides. selenium 4.44 regenerated the BiDi layer and moved the +# internals network capture subscribes through; `bidi.py` now has a path for each +# surface, so there is no range to exclude. The CI matrix is what keeps that +# honest — its 3.9 job resolves a pre-4.44 selenium and its 3.12 job the latest, +# so both paths run on every PR. selenium = ["selenium>=4.6"] -# The TEST environment is capped, because the surface guards assert against -# whatever is installed and only mean something on a version the adapter fully -# supports. Constraining the dev environment costs no user anything. -test = ["pytest>=7", "selenium>=4.6,<4.44"] +test = ["pytest>=7", "selenium>=4.6"] # Auto-discovered by pytest; inert unless DEVTOOLS_ENABLE / DEVTOOLS_PORT is set. [project.entry-points.pytest11] From 2d0a38a52861188d1d1a17a3a162197de7d7e2d2 Mon Sep 17 00:00:00 2001 From: Vishnu Vardhan Date: Wed, 19 Aug 2026 15:46:03 +0530 Subject: [PATCH 04/12] fix(selenium-devtools-py): capture network on selenium 4.44+ --- .../src/selenium_devtools/bidi.py | 94 ++++++++---- .../selenium-devtools-py/tests/test_bidi.py | 134 +++++++++++++----- 2 files changed, 165 insertions(+), 63 deletions(-) diff --git a/packages/selenium-devtools-py/src/selenium_devtools/bidi.py b/packages/selenium-devtools-py/src/selenium_devtools/bidi.py index 2727d481..a8a9e1cf 100644 --- a/packages/selenium-devtools-py/src/selenium_devtools/bidi.py +++ b/packages/selenium-devtools-py/src/selenium_devtools/bidi.py @@ -24,8 +24,8 @@ generated event dataclasses are lossy (``BeforeRequestSentParameters`` declares only ``initiator``, and its deserializer DROPS every param not declared, taking the request, its id and the timestamp with it), so - ``register_raw_event_configs`` registers configs whose event class is ``dict`` - and the handlers receive raw params. + ``_add_raw_event_handler`` swaps in a pass-through deserializer for the + registration and puts selenium's own back immediately. * **≤4.43** — ``driver.network.conn`` plus ``NetworkEvent``, which is all those versions offer. @@ -390,8 +390,8 @@ def event_params(event: Any) -> Dict[str, Any]: Legacy selenium hands the callback a ``NetworkEvent`` carrying ``.params``; 4.44+ hands it whatever its deserializer produced, which is the raw params - dict for the configs registered by ``register_raw_event_configs``. Both - collapse here so the mapping helpers below only ever see a plain dict. + dict for the handlers registered by ``_add_raw_event_handler``. Both collapse + here so the mapping helpers below only ever see a plain dict. """ if isinstance(event, dict): return event @@ -399,32 +399,72 @@ def event_params(event: Any) -> Dict[str, Any]: return params if isinstance(params, dict) else {} -def register_raw_event_configs() -> bool: - """Register raw-params event configs on selenium 4.44+. False = legacy path. +_MISSING = object() - MUST run before anything touches ``driver.network``: the deserializers are - built once, in ``Network.__init__``, from whatever configs exist then. - Registered under the adapter's own keys so the result does not depend on - selenium's own duplicate config entries — it ships two keys mapping to - ``network.beforeRequestSent`` with different classes, and which one wins is - decided by dict iteration order. Ours are added last and win either way. +class _RawEvent: + """The deserializer selenium's dispatch expects, passing params through. - Side effect worth knowing: deserializers are keyed by BiDi event, not by - config key, so this also makes selenium's own ``response_completed`` handlers - receive the raw dict in this process. Nothing else in a test run subscribes - to it, and the dict carries strictly more than the dataclass it replaces. + Selenium identifies an event by ``event_class`` and deserializes it with + ``from_json``; its own wrapper builds a generated dataclass there, which for + these two events silently discards the request and the timestamp. """ + + def __init__(self, bidi_event: str) -> None: + self.event_class = bidi_event + + def from_json(self, params: Any) -> Any: + return params + + +def supports_event_handler_api() -> bool: + """True when selenium presents the 4.44+ event-handler API. Detection only — + nothing is mutated, so the caller can pick a path without side effects.""" try: from selenium.webdriver.common.bidi.network import EventConfig, Network except ImportError: return False # ≤4.43: no generated layer, use the connection path - configs = getattr(Network, "EVENT_CONFIGS", None) - if not isinstance(configs, dict) or not hasattr(Network, "add_event_handler"): - return False - for bidi_event, key in BIDI_NET_EVENT_KEYS.items(): - configs.setdefault(key, EventConfig(key, bidi_event, dict)) - return True + return bool( + EventConfig + and isinstance(getattr(Network, "EVENT_CONFIGS", None), dict) + and hasattr(Network, "add_event_handler") + ) + + +def _add_raw_event_handler(network: Any, bidi_event: str, callback: Any) -> None: + """Subscribe ``callback`` to ``bidi_event`` with the RAW params, leaving + selenium's shared state exactly as it was found. + + Selenium picks the deserializer out of a per-BiDi-event map, so receiving raw + params means putting ours in that map. It is swapped in only for the duration + of the registration and the ORIGINAL OBJECT is put back, because + ``add_callback`` closes over the deserializer it was given: our handler keeps + the raw one for the life of the session, while every other handler — before + or after, ours or the user's — keeps selenium's own. + + Restoring matters beyond tidiness. Left in place this would hand raw dicts to + any other subscriber of these events in the process, breaking attribute + access on the generated objects they expect, and it would outlive the adapter. + """ + from selenium.webdriver.common.bidi.network import EventConfig + + key = BIDI_NET_EVENT_KEYS[bidi_event] + configs = network.EVENT_CONFIGS + wrappers = network._event_manager._event_wrappers + + had_key = key in configs + saved = wrappers.get(bidi_event, _MISSING) + configs[key] = EventConfig(key, bidi_event, dict) + wrappers[bidi_event] = _RawEvent(bidi_event) + try: + network.add_event_handler(key, callback) + finally: + if not had_key: + configs.pop(key, None) + if saved is _MISSING: + wrappers.pop(bidi_event, None) + else: + wrappers[bidi_event] = saved _reported_incomplete: set = set() @@ -462,7 +502,7 @@ def _attach_network(driver: Any, capturer: SessionCapturer) -> bool: Returns False (and logs) on any failure — network BiDi is best-effort. """ - use_event_manager = register_raw_event_configs() + use_event_manager = supports_event_handler_api() pending: Dict[str, Dict[str, Any]] = {} @@ -517,11 +557,9 @@ def _subscribe_via_event_manager( """ try: network = driver.network - network.add_event_handler( - BIDI_NET_EVENT_KEYS[BIDI_NET_BEFORE_REQUEST], on_request_sent - ) - network.add_event_handler( - BIDI_NET_EVENT_KEYS[BIDI_NET_RESPONSE_COMPLETED], on_response_completed + _add_raw_event_handler(network, BIDI_NET_BEFORE_REQUEST, on_request_sent) + _add_raw_event_handler( + network, BIDI_NET_RESPONSE_COMPLETED, on_response_completed ) _log.info( "network capture subscribed via the event-handler API (selenium %s)", diff --git a/packages/selenium-devtools-py/tests/test_bidi.py b/packages/selenium-devtools-py/tests/test_bidi.py index b5933a61..651e83f6 100644 --- a/packages/selenium-devtools-py/tests/test_bidi.py +++ b/packages/selenium-devtools-py/tests/test_bidi.py @@ -388,30 +388,66 @@ def __init__(self, event_key, bidi_event, event_class): self.bidi_event = bidi_event self.event_class = event_class + class TypedWrapper: + """selenium's own deserializer: builds the generated dataclass, which + for these events keeps only its one declared field.""" + + def __init__(self, bidi_event, event_class): + self.event_class = bidi_event + self._python_class = event_class + + def from_json(self, params): + if self._python_class is dict: + return params + declared = getattr(self._python_class, "DECLARED", ()) + return self._python_class( + **{k: v for k, v in params.items() if k in declared} + ) + + class BeforeRequestSentParameters: + DECLARED = ("initiator",) + + def __init__(self, initiator=None): + self.initiator = initiator + class Network: - EVENT_CONFIGS = {} + EVENT_CONFIGS = { + "before_request_sent": EventConfig( + "before_request_sent", + "network.beforeRequestSent", + BeforeRequestSentParameters, + ), + "response_completed": EventConfig( + "response_completed", "network.responseCompleted", dict + ), + } def __init__(self): self.handlers = {} - # selenium builds its deserializers here, from the configs that - # exist at construction time. Mirrored because that timing is the - # whole reason registration has to come first. - self.deserializers = { - config.bidi_event: config.event_class - for config in self.EVENT_CONFIGS.values() - } + # selenium builds one deserializer per BiDi event here, and + # `add_callback` then CLOSES OVER the one it is handed — which is + # what makes restoring the map afterwards safe. + self._event_manager = types.SimpleNamespace( + _event_wrappers={ + config.bidi_event: TypedWrapper( + config.bidi_event, config.event_class + ) + for config in self.EVENT_CONFIGS.values() + } + ) def add_event_handler(self, event, callback, contexts=None): # selenium raises for an unregistered key rather than ignoring it. config = self.EVENT_CONFIGS.get(event) if config is None: raise ValueError(f"Event '{event}' not found") - self.handlers[event] = callback + wrapper = self._event_manager._event_wrappers[config.bidi_event] + self.handlers[event] = lambda params: callback(wrapper.from_json(params)) return len(self.handlers) if not with_event_manager: del Network.add_event_handler - del Network.EVENT_CONFIGS + Network.EVENT_CONFIGS = None module.EventConfig = EventConfig module.Network = Network @@ -470,50 +506,78 @@ def test_raw_params_reach_the_handlers_and_are_captured(self): self.assertEqual(frames[0]["url"], "https://x/a.js") self.assertEqual(frames[1]["status"], 200) # correlated with the request - def test_configs_are_registered_before_network_is_constructed(self): - """The ordering IS the mechanism. `Network.__init__` builds one - deserializer per event from the configs present at that moment, so - registering after the first `driver.network` silently keeps the lossy - typed path and every entry loses its request id.""" - log = [] - - class RecordingConfigs(dict): - def setdefault(self, *args, **kwargs): - log.append("registered") - return super().setdefault(*args, **kwargs) + def test_selenium_shared_state_is_left_exactly_as_found(self): + """The deserializer swap must not outlive the registration. + Left in place it would hand raw dicts to any other subscriber of these + events in the process — breaking attribute access on the generated + objects they expect — and would persist after the adapter is done. + """ module = fake_network_module() - module.Network.EVENT_CONFIGS = RecordingConfigs() + network = module.Network() + + configs_before = dict(module.Network.EVENT_CONFIGS) + wrappers = network._event_manager._event_wrappers + wrappers_before = dict(wrappers) with mock.patch.dict( sys.modules, {"selenium.webdriver.common.bidi.network": module} ): - bidi._attach_network( - NewSeleniumDriver(module.Network(), log), SessionCapturer(FakeTransport()) + self.assertTrue( + bidi._attach_network( + NewSeleniumDriver(network, []), SessionCapturer(FakeTransport()) + ) ) - self.assertEqual(log[0], "registered") - self.assertIn("network accessed", log) - - def test_both_events_register_a_raw_config(self): + self.assertEqual(module.Network.EVENT_CONFIGS, configs_before) + # Identity, not equality: selenium's own deserializer OBJECTS are back, + # so a handler registered later behaves exactly as it would have. + self.assertEqual(wrappers, wrappers_before) + for event, wrapper in wrappers_before.items(): + self.assertIs(wrappers[event], wrapper) + + def test_our_handler_keeps_raw_params_after_the_restore(self): + """`add_callback` closes over the deserializer it was handed, so putting + selenium's back does not reach into a handler already registered. This is + the assumption the whole isolation rests on.""" module = fake_network_module() + network = module.Network() + capturer = SessionCapturer(FakeTransport()) + with mock.patch.dict( sys.modules, {"selenium.webdriver.common.bidi.network": module} ): - self.assertTrue(bidi.register_raw_event_configs()) + bidi._attach_network(NewSeleniumDriver(network, []), capturer) - configs = module.Network.EVENT_CONFIGS - for bidi_event, key in bidi.BIDI_NET_EVENT_KEYS.items(): - self.assertEqual(configs[key].bidi_event, bidi_event) - # dict is what makes selenium pass the params through untouched. - self.assertIs(configs[key].event_class, dict) + # Dispatched AFTER the restore, through selenium's own registration. + network.handlers["devtools_before_request_sent"]( + {"request": {"request": "R1", "url": "https://x/a.js", "method": "GET"}, + "timestamp": 1000} + ) + + frames = [ + batch[0] for scope, batch in capturer._tx.sent + if scope == "networkRequests" + ] + self.assertEqual(len(frames), 1) + self.assertEqual(frames[0]["url"], "https://x/a.js") def test_legacy_selenium_keeps_the_connection_path(self): module = fake_network_module(with_event_manager=False) with mock.patch.dict( sys.modules, {"selenium.webdriver.common.bidi.network": module} ): - self.assertFalse(bidi.register_raw_event_configs()) + self.assertFalse(bidi.supports_event_handler_api()) + + def test_the_regenerated_layer_selects_the_event_handler_path(self): + module = fake_network_module() + with mock.patch.dict( + sys.modules, {"selenium.webdriver.common.bidi.network": module} + ): + self.assertTrue(bidi.supports_event_handler_api()) + + # Detection alone must not touch anything — the caller uses it to pick. + self.assertNotIn("devtools_before_request_sent", module.Network.EVENT_CONFIGS) class TestEventParams(unittest.TestCase): From 1ba656b712f319b6709d3b2f9ed701f01b0ff6eb Mon Sep 17 00:00:00 2001 From: Vishnu Vardhan Date: Wed, 19 Aug 2026 16:19:06 +0530 Subject: [PATCH 05/12] chore(selenium-devtools-py): require selenium 4.44+ and python 3.10+ --- .github/workflows/python.yml | 10 +++------- packages/selenium-devtools-py/README.md | 8 +++++++- packages/selenium-devtools-py/pyproject.toml | 18 ++++++++++-------- 3 files changed, 20 insertions(+), 16 deletions(-) diff --git a/.github/workflows/python.yml b/.github/workflows/python.yml index 3cd31045..e9b34981 100644 --- a/.github/workflows/python.yml +++ b/.github/workflows/python.yml @@ -32,7 +32,9 @@ jobs: runs-on: ubuntu-latest strategy: matrix: - python-version: ['3.9', '3.12'] + # The floor the package declares, and the newest — 3.10 because selenium + # 4.44 requires it, and network capture requires 4.44+. + python-version: ['3.10', '3.13'] steps: - uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 - uses: actions/setup-python@a26af69be951a213d495a4c3e4e4022e16d87065 # v5 @@ -42,12 +44,6 @@ jobs: # pin its BiDi surface (tests/test_selenium_surface.py) skip without it — # so a bump that moved those internals used to pass CI green and degrade # at runtime. Installing the package is what makes those guards real. - # - # The matrix is load-bearing, not just breadth: network capture has two - # subscription paths, one per selenium BiDi surface, and the python version - # is what selects which selenium resolves. 3.9 caps at a pre-4.44 selenium - # and exercises the connection path; 3.12 resolves the latest and exercises - # the event-handler path. Dropping either job leaves one path untested. - name: Install the adapter and its runtime dependency run: pip install -e '.[test]' diff --git a/packages/selenium-devtools-py/README.md b/packages/selenium-devtools-py/README.md index 8ad4f1cc..7ff1dbbd 100644 --- a/packages/selenium-devtools-py/README.md +++ b/packages/selenium-devtools-py/README.md @@ -19,6 +19,12 @@ pip install -e packages/selenium-devtools-py # or: pip install selenium-devtoo The transport is **dependency-free** (stdlib WebSocket client). The only thing on top of your own `selenium` install is this package; `pytest` is optional. +**Requires Python 3.10+ and selenium 4.44+.** Network capture subscribes through +the public BiDi event API that selenium regenerated in 4.44 — before that the +only way to observe requests without pausing them was a private connection, +which the same release removed. 4.44 requires Python 3.10, which sets the +Python floor too. + ## Use **With pytest (recommended) — no code changes to your tests:** @@ -159,7 +165,7 @@ DEVTOOLS_PORT=3000 PYTHONPATH=src pytest e2e/test_smoke.py -p selenium_devtools. Two workflows, mirroring the JS split (`ci.yml` tests / `release.yml` publish): - **`python.yml`** — runs on PRs + pushes touching this package or `shared`: - unit tests on Python 3.9 + 3.12, and a contract-drift check (regenerate + unit tests on Python 3.10 + 3.13, and a contract-drift check (regenerate `_contract.py`, fail on any diff). Zero repo config needed. - **`python-release.yml`** — **manual** (`workflow_dispatch`, like the JS "Manual NPM Publish"), target `pypi` or `testpypi`. Builds the sdist + wheel diff --git a/packages/selenium-devtools-py/pyproject.toml b/packages/selenium-devtools-py/pyproject.toml index 9902fa07..75752369 100644 --- a/packages/selenium-devtools-py/pyproject.toml +++ b/packages/selenium-devtools-py/pyproject.toml @@ -7,7 +7,9 @@ name = "selenium-devtools-py" version = "0.1.0" description = "Python Selenium adapter for the WebdriverIO DevTools dashboard" readme = "README.md" -requires-python = ">=3.9" +# >=3.10 because selenium 4.44 requires it, and network capture needs 4.44+. +# Not an independent choice: 3.9 left security support in October 2025. +requires-python = ">=3.10" license = { text = "MIT" } authors = [{ name = "WebdriverIO" }] keywords = ["selenium", "webdriver", "devtools", "pytest", "debugging"] @@ -17,13 +19,13 @@ keywords = ["selenium", "webdriver", "devtools", "pytest", "debugging"] dependencies = [] [project.optional-dependencies] -# Uncapped on both sides. selenium 4.44 regenerated the BiDi layer and moved the -# internals network capture subscribes through; `bidi.py` now has a path for each -# surface, so there is no range to exclude. The CI matrix is what keeps that -# honest — its 3.9 job resolves a pre-4.44 selenium and its 3.12 job the latest, -# so both paths run on every PR. -selenium = ["selenium>=4.6"] -test = ["pytest>=7", "selenium>=4.6"] +# 4.44 is where selenium regenerated the BiDi layer, and with it the public +# event API network capture subscribes through. Earlier versions offered only a +# private connection, which 4.44 removed — supporting both would mean carrying +# two subscription paths for releases that predate this package's own first one. +# Uncapped above: 4.44+ is a floor, not a range. +selenium = ["selenium>=4.44"] +test = ["pytest>=7", "selenium>=4.44"] # Auto-discovered by pytest; inert unless DEVTOOLS_ENABLE / DEVTOOLS_PORT is set. [project.entry-points.pytest11] From f09f287fdbdd4f73f80a342988904f4f82e20d2a Mon Sep 17 00:00:00 2001 From: Vishnu Vardhan Date: Wed, 19 Aug 2026 16:19:17 +0530 Subject: [PATCH 06/12] refactor(selenium-devtools-py): drop the pre-4.44 selenium path --- .../src/selenium_devtools/bidi.py | 145 +++++++----------- .../src/selenium_devtools/constants.py | 12 +- .../selenium-devtools-py/tests/test_bidi.py | 70 +++++---- .../tests/test_selenium_surface.py | 80 +++------- 4 files changed, 122 insertions(+), 185 deletions(-) diff --git a/packages/selenium-devtools-py/src/selenium_devtools/bidi.py b/packages/selenium-devtools-py/src/selenium_devtools/bidi.py index a8a9e1cf..6818c33e 100644 --- a/packages/selenium-devtools-py/src/selenium_devtools/bidi.py +++ b/packages/selenium-devtools-py/src/selenium_devtools/bidi.py @@ -17,21 +17,21 @@ request until selenium continues it; that would change the timing of the page under test. Both paths below subscribe instead. -There are two of those paths because selenium 4.44 regenerated the BiDi layer -from a schema, and the pre-4.44 one no longer exists there: - -* **4.44+** — ``Network.add_event_handler``, public and observe-only. Its - generated event dataclasses are lossy (``BeforeRequestSentParameters`` - declares only ``initiator``, and its deserializer DROPS every param not - declared, taking the request, its id and the timestamp with it), so - ``_add_raw_event_handler`` swaps in a pass-through deserializer for the - registration and puts selenium's own back immediately. -* **≤4.43** — ``driver.network.conn`` plus ``NetworkEvent``, which is all those - versions offer. - -``event_params`` is what lets one pair of handlers serve both, and the mapping -helpers below never learn which path ran. ``tests/test_selenium_surface.py`` -guards both surfaces against the installed selenium. +Network capture subscribes through ``Network.add_event_handler``, the public +observe-only API in the BiDi layer selenium regenerated in 4.44. Before that +release the only way to observe was a private connection plus ``NetworkEvent``, +both of which 4.44 removed; the adapter requires 4.44+ rather than carrying a +second path for versions predating its own first release. + +Its generated event dataclasses cannot be used as delivered: +``BeforeRequestSentParameters`` declares only ``initiator`` and the deserializer +DROPS every param not declared, taking the request, its id and the timestamp +with it — which leaves a response with nothing to correlate against. So +``_add_raw_event_handler`` swaps in a pass-through deserializer for the +registration and puts selenium's own back immediately. + +``tests/test_selenium_surface.py`` guards that surface against the installed +selenium. """ from __future__ import annotations @@ -47,7 +47,7 @@ BIDI_NET_EVENT_KEYS, BIDI_NET_RESPONSE_COMPLETED, LOGGER_NAME, - SELENIUM_NETWORK_SURFACE_MOVED_AT, + SELENIUM_MINIMUM_VERSION, ) from .utils import now_ms, selenium_version @@ -179,8 +179,8 @@ def _format_stacktrace(stacktrace: Any) -> str: def request_sent_kwargs(params: Dict[str, Any]) -> Optional[Dict[str, Any]]: """kwargs for the initial (pending) network frame, or None if unidentifiable. - ``params`` is the BiDi ``network.beforeRequestSent`` event params — the - ``.params`` dict on selenium's NetworkEvent. + ``params`` is the raw BiDi ``network.beforeRequestSent`` event params, as + delivered by ``event_params``. """ request = params.get("request") or {} request_id = str(request.get("request") or params.get("id") or "") @@ -362,41 +362,42 @@ def on_js_error(entry: Any) -> None: def network_unavailable_reason(exc: Exception) -> str: - """Why the connection path could not attach. - - Reached on 4.44+ only when ``register_raw_event_configs`` declined, i.e. the - regenerated layer is installed but did not present the API it is defined by. - The bare exception there is ``cannot import name 'NetworkEvent'``, which - reads like a broken install rather than a path that does not apply — so that - case says what it means and asks for a report, because it is a combination - the adapter does not know about. + """Why network capture could not attach, naming a too-old selenium as the + cause when it is one. + + The adapter needs the BiDi layer selenium regenerated in 4.44, and pyproject + requires it, but a user can still end up below that — an existing + environment, a transitive pin, a resolver that had to back off. The bare + exception is then an AttributeError about a generated attribute, which reads + like a broken install rather than a version floor. """ version = selenium_version() - if version >= SELENIUM_NETWORK_SURFACE_MOVED_AT: + required = ".".join(str(p) for p in SELENIUM_MINIMUM_VERSION) + if version < SELENIUM_MINIMUM_VERSION: installed = ".".join(str(part) for part in version) - moved_at = ".".join(str(p) for p in SELENIUM_NETWORK_SURFACE_MOVED_AT) return ( - f"network capture could not attach on selenium {installed}: its " - f"{moved_at}+ event-handler API was not usable, and the older " - f"connection path does not exist there ({exc}). Console, DOM and " - "command capture are unaffected. Please report this with your " - "selenium version: https://github.com/webdriverio/devtools/issues" + f"network capture needs selenium >= {required} and {installed} is " + "installed: the BiDi event API it subscribes through arrived in " + f"{required}. Console, DOM and command capture are unaffected. " + f"`pip install --upgrade 'selenium>={required}'` restores it." ) - return f"network channel unavailable: {exc}" + return ( + f"network capture could not attach on selenium {'.'.join(str(p) for p in version)}" + f" ({exc}). Console, DOM and command capture are unaffected. Please " + "report this: https://github.com/webdriverio/devtools/issues" + ) def event_params(event: Any) -> Dict[str, Any]: - """The BiDi event params dict, from either subscription path. + """The BiDi event params dict. - Legacy selenium hands the callback a ``NetworkEvent`` carrying ``.params``; - 4.44+ hands it whatever its deserializer produced, which is the raw params - dict for the handlers registered by ``_add_raw_event_handler``. Both collapse - here so the mapping helpers below only ever see a plain dict. + ``_RawEvent`` passes selenium's params straight through, so this is already a + dict on every healthy path. It stays as the boundary check because the value + comes from selenium's dispatch rather than from us: a future release that + hands the callback something else is then an empty dict and a warning from + ``_incomplete_event``, not an AttributeError inside the handler. """ - if isinstance(event, dict): - return event - params = getattr(event, "params", None) - return params if isinstance(params, dict) else {} + return event if isinstance(event, dict) else {} _MISSING = object() @@ -418,12 +419,12 @@ def from_json(self, params: Any) -> Any: def supports_event_handler_api() -> bool: - """True when selenium presents the 4.44+ event-handler API. Detection only — - nothing is mutated, so the caller can pick a path without side effects.""" + """True when selenium presents the event-handler API capture subscribes + through. Detection only — nothing is mutated.""" try: from selenium.webdriver.common.bidi.network import EventConfig, Network except ImportError: - return False # ≤4.43: no generated layer, use the connection path + return False return bool( EventConfig and isinstance(getattr(Network, "EVENT_CONFIGS", None), dict) @@ -502,8 +503,6 @@ def _attach_network(driver: Any, capturer: SessionCapturer) -> bool: Returns False (and logs) on any failure — network BiDi is best-effort. """ - use_event_manager = supports_event_handler_api() - pending: Dict[str, Dict[str, Any]] = {} def on_request_sent(event: Any) -> None: @@ -539,11 +538,9 @@ def on_response_completed(event: Any) -> None: except Exception as exc: # noqa: BLE001 _warn(f"responseCompleted handler threw: {exc}") - if use_event_manager: - return _subscribe_via_event_manager( - driver, on_request_sent, on_response_completed - ) - return _subscribe_via_connection(driver, on_request_sent, on_response_completed) + return _subscribe_via_event_manager( + driver, on_request_sent, on_response_completed + ) def _subscribe_via_event_manager( @@ -555,6 +552,11 @@ def _subscribe_via_event_manager( register an intercept even in their high-level form, which pauses every request until selenium continues it. This only observes. """ + if not supports_event_handler_api(): + # Checked rather than caught, so a too-old selenium is reported as a + # version floor instead of an AttributeError about a generated attribute. + _warn(network_unavailable_reason(AttributeError("no BiDi event API"))) + return False try: network = driver.network _add_raw_event_handler(network, BIDI_NET_BEFORE_REQUEST, on_request_sent) @@ -570,43 +572,6 @@ def _subscribe_via_event_manager( _warn(f"network subscribe failed: {exc}") return False -def _subscribe_via_connection( - driver: Any, on_request_sent: Any, on_response_completed: Any -) -> bool: - """selenium ≤4.43: subscribe over the low-level connection. - - Kept because it is the only path on those versions, not as a fallback for a - 4.44+ failure — there `driver.network.conn` and ``NetworkEvent`` do not - exist, so this cannot recover anything the path above could not do. - """ - try: - conn = driver.network.conn - from selenium.webdriver.common.bidi.network import NetworkEvent # lazy - from selenium.webdriver.common.bidi.session import Session # lazy - except Exception as exc: # noqa: BLE001 - _warn(network_unavailable_reason(exc)) - return False - - try: - conn.execute( - Session(conn).subscribe( - BIDI_NET_BEFORE_REQUEST, BIDI_NET_RESPONSE_COMPLETED - ) - ) - conn.add_callback(NetworkEvent(BIDI_NET_BEFORE_REQUEST), on_request_sent) - conn.add_callback( - NetworkEvent(BIDI_NET_RESPONSE_COMPLETED), on_response_completed - ) - _log.info( - "network capture subscribed via the connection (selenium %s)", - ".".join(str(p) for p in selenium_version()), - ) - return True - except Exception as exc: # noqa: BLE001 - _warn(f"network subscribe failed: {exc}") - return False - - def attach(driver: Any, capturer: SessionCapturer) -> bool: """Wire BiDi console + network capture onto ``driver``. diff --git a/packages/selenium-devtools-py/src/selenium_devtools/constants.py b/packages/selenium-devtools-py/src/selenium_devtools/constants.py index ba35627f..515fc7b8 100644 --- a/packages/selenium-devtools-py/src/selenium_devtools/constants.py +++ b/packages/selenium-devtools-py/src/selenium_devtools/constants.py @@ -100,11 +100,13 @@ # would stall the user's page loads if a callback failed to continue them). BIDI_NET_BEFORE_REQUEST = "network.beforeRequestSent" BIDI_NET_RESPONSE_COMPLETED = "network.responseCompleted" -# selenium 4.44 regenerated the BiDi layer from a schema: ``NetworkEvent`` left -# ``bidi.network`` and ``Network.conn`` became ``_conn``, so the subscribe above -# cannot be built there. One source of truth for that version, read by bidi.py to -# pick its subscription path and by the surface guards. -SELENIUM_NETWORK_SURFACE_MOVED_AT = (4, 44) +# The adapter targets the regenerated BiDi layer, which selenium 4.44 shipped — +# it is what ``Network.add_event_handler`` and ``EVENT_CONFIGS`` arrived in, and +# the pre-4.44 internals network capture used instead were removed in the same +# release. Declared in pyproject too; here so a wrong install is explained rather +# than raising an AttributeError, and so the surface guards know what they apply +# to. Also the reason ``requires-python`` is >=3.10: selenium 4.44 requires it. +SELENIUM_MINIMUM_VERSION = (4, 44) # Keys the adapter registers into ``Network.EVENT_CONFIGS`` on 4.44+, so its # handlers receive RAW event params. Registered under our own names rather than # reusing selenium's, because selenium's generated event dataclasses model only diff --git a/packages/selenium-devtools-py/tests/test_bidi.py b/packages/selenium-devtools-py/tests/test_bidi.py index 651e83f6..ab26b881 100644 --- a/packages/selenium-devtools-py/tests/test_bidi.py +++ b/packages/selenium-devtools-py/tests/test_bidi.py @@ -562,12 +562,24 @@ def test_our_handler_keeps_raw_params_after_the_restore(self): self.assertEqual(len(frames), 1) self.assertEqual(frames[0]["url"], "https://x/a.js") - def test_legacy_selenium_keeps_the_connection_path(self): + def test_a_selenium_without_the_event_api_is_reported_not_silent(self): + """There is no second path to fall back to, so a selenium that cannot + provide this has to say so. Silence here is an empty Network tab with no + stated cause, which is the failure this whole port exists to end.""" module = fake_network_module(with_event_manager=False) + with mock.patch.dict( sys.modules, {"selenium.webdriver.common.bidi.network": module} ): self.assertFalse(bidi.supports_event_handler_api()) + with self.assertLogs("selenium_devtools.bidi", level="WARNING") as logs: + # `.network` is never reached: the check happens before it. + attached = bidi._attach_network( + NewSeleniumDriver(None, []), SessionCapturer(FakeTransport()) + ) + + self.assertFalse(attached) + self.assertIn("network capture", "\n".join(logs.output)) def test_the_regenerated_layer_selects_the_event_handler_path(self): module = fake_network_module() @@ -584,12 +596,14 @@ class TestEventParams(unittest.TestCase): def test_a_raw_dict_is_its_own_params(self): self.assertEqual(bidi.event_params({"request": {}}), {"request": {}}) - def test_a_legacy_event_object_is_unwrapped(self): - event = types.SimpleNamespace(params={"request": {"url": "u"}}) - self.assertEqual(bidi.event_params(event), {"request": {"url": "u"}}) - def test_anything_else_degrades_to_empty(self): + # The value comes from selenium's dispatch, so a release that hands the + # callback an object must become a warning from _incomplete_event, not + # an AttributeError raised inside the handler. self.assertEqual(bidi.event_params(object()), {}) + self.assertEqual( + bidi.event_params(types.SimpleNamespace(params={"request": {}})), {} + ) class TestADegradedEventIsReported(unittest.TestCase): @@ -628,42 +642,34 @@ def test_a_complete_event_is_silent(self): class TestWhyNetworkCaptureIsOff(unittest.TestCase): - """The connection path is unreachable on 4.44+, so arriving there means the - regenerated layer is installed but did not present the API that defines it. - That is a combination the adapter does not know about, and the warning has to - say so rather than echo an ImportError that reads like a broken install.""" + """pyproject requires the selenium that carries the BiDi event API, but a + user can still be below it — an existing environment, a transitive pin, a + resolver that backed off. The bare failure is then an AttributeError about a + generated attribute, which reads like a broken install rather than a floor.""" - def test_a_moved_surface_is_reported_as_a_version_gap(self): - major, minor = bidi.SELENIUM_NETWORK_SURFACE_MOVED_AT + def test_a_selenium_below_the_floor_is_named_as_the_cause(self): + major, minor = bidi.SELENIUM_MINIMUM_VERSION with mock.patch.object( - bidi, "selenium_version", return_value=(major, minor + 1) + bidi, "selenium_version", return_value=(major, minor - 1) ): - reason = bidi.network_unavailable_reason( - ImportError("cannot import name 'NetworkEvent'") - ) + reason = bidi.network_unavailable_reason(AttributeError("no attribute")) - # BOTH versions, and they are different things: what the user has, and - # where the surface moved. Asserted on the ATTRIBUTION, not on the - # version appearing somewhere — the moved-at version is also named in - # the "selenium < X captures network" advice, so a bare assertIn passes - # even when the sentence blames the installed version for the move. - self.assertIn(f"on selenium {major}.{minor + 1}", reason) - self.assertIn(f"{major}.{minor}+ event-handler API", reason) - # The underlying error, and the half that still works — without it this - # reads as total loss. - self.assertIn("cannot import name", reason) + self.assertIn(f"{major}.{minor - 1} is installed", reason) # what they have + self.assertIn(f">= {major}.{minor}", reason) # what is needed + self.assertIn("pip install --upgrade", reason) # how to fix it + # The half that still works, or this reads as total loss. self.assertIn("Console", reason) - self.assertIn("report", reason) - def test_an_ordinary_failure_still_reports_the_exception(self): - # Below the moved version the connection path is the ONLY path, so a - # failure there is ordinary and blaming the selenium release would send - # the reader somewhere with no answer. - with mock.patch.object(bidi, "selenium_version", return_value=(4, 36)): + def test_a_failure_on_a_supported_selenium_reports_the_exception(self): + # At or above the floor the version is NOT the cause, so blaming it would + # send the reader somewhere with no answer. + with mock.patch.object( + bidi, "selenium_version", return_value=bidi.SELENIUM_MINIMUM_VERSION + ): reason = bidi.network_unavailable_reason(RuntimeError("no bidi socket")) self.assertIn("no bidi socket", reason) - self.assertNotIn("event-handler API", reason) + self.assertNotIn("pip install --upgrade", reason) if __name__ == "__main__": diff --git a/packages/selenium-devtools-py/tests/test_selenium_surface.py b/packages/selenium-devtools-py/tests/test_selenium_surface.py index d16666b2..b463f824 100644 --- a/packages/selenium-devtools-py/tests/test_selenium_surface.py +++ b/packages/selenium-devtools-py/tests/test_selenium_surface.py @@ -11,13 +11,15 @@ rename or relocation, not to test selenium's behaviour. That prediction was already history when these first ran: selenium 4.44 shipped -the regenerated layer, and CI (which resolves a newer selenium than a local 3.9 -can install) failed on the first run, on a breakage that had already shipped. +the regenerated layer, and CI (which resolves a newer selenium than the local +python could install) failed on the first run, on a breakage that had already +shipped. The adapter now targets that layer, and these guard it. -`bidi.py` now has a path for each surface, so BOTH are guarded here and which -class applies is decided by the installed version. Neither is optional: whichever -one the installed selenium presents is the only thing standing between a working -Network tab and an empty one. +Skipped below the version the package requires, rather than failing: the floor +is declared in `pyproject.toml`, and a developer whose environment predates it +should be told these did not run, not handed a wall of failures about attributes +their selenium was never going to have. CI installs from that floor, so they run +where they matter. Skipped when selenium is absent. That is not free: the CI job must install the adapter's own runtime dependency or these never run where they are meant to @@ -30,27 +32,28 @@ class applies is decided by the installed version. Neither is optional: whicheve import inspect import unittest -from selenium_devtools.constants import SELENIUM_NETWORK_SURFACE_MOVED_AT +from selenium_devtools.constants import SELENIUM_MINIMUM_VERSION from selenium_devtools.utils import selenium_version _HAS_SELENIUM = importlib.util.find_spec("selenium") is not None - -# The version fact itself lives in constants.py, because bidi.py needs it too — -# it is what turns the runtime degradation into a warning naming the version. -_NETWORK_SURFACE_MOVED = selenium_version() >= SELENIUM_NETWORK_SURFACE_MOVED_AT +_BELOW_MINIMUM = selenium_version() < SELENIUM_MINIMUM_VERSION +_TOO_OLD = ( + f"selenium is below the {'.'.join(str(p) for p in SELENIUM_MINIMUM_VERSION)} " + "this package requires" +) @unittest.skipUnless(_HAS_SELENIUM, "selenium is not installed") -@unittest.skipIf(not _NETWORK_SURFACE_MOVED, "selenium predates the regenerated layer") +@unittest.skipIf(_BELOW_MINIMUM, _TOO_OLD) class TestTheRegeneratedNetworkSurface(unittest.TestCase): - """selenium 4.44+ — what `_subscribe_via_event_manager` needs to exist. + """What `_subscribe_via_event_manager` needs to exist. `EVENT_CONFIGS` is a public class attribute and `add_event_handler` a public method, so this is a supported-API dependency rather than reaching inside. - What is NOT public is that one deserializer is built per BiDi event at - `Network.__init__`, which is why registering a `dict` config wins and why it - must happen before the first `driver.network`. That is the fragile part, and - the shape assertions below are what would catch it changing.""" + What is NOT public is that one deserializer is built per BiDi event and held + in a map, which is what `_add_raw_event_handler` swaps and restores. That is + the fragile part, and the shape assertions below are what would catch it + changing.""" def test_the_public_event_handler_api_is_present(self): from selenium.webdriver.common.bidi.network import Network @@ -100,50 +103,11 @@ def test_the_generated_event_classes_are_still_lossy(self): self.assertNotIn("timestamp", declared) -@unittest.skipUnless(_HAS_SELENIUM, "selenium is not installed") -@unittest.skipIf(_NETWORK_SURFACE_MOVED, "selenium uses the regenerated surface") -class TestThePreRegenerationNetworkInternals(unittest.TestCase): - """selenium ≤4.43 — what `_subscribe_via_connection` needs to exist. - - It reaches through `driver.network.conn` because every selenium API that - takes a request or response handler registers an intercept, which pauses - each request until selenium continues it. On these versions there is no - observe-only alternative, so the private access is the price of not - changing the timing of the page under test. - - Gated to versions where this path actually runs. The console, preload and - driver-channel guards below are gated on nothing, because they hold on every - version and skipping them would drop coverage where a bump is most likely to - move something.""" - - def test_the_network_channel_still_carries_the_low_level_connection(self): - from selenium.webdriver.common.bidi.network import Network - - # `driver.network.conn` is the whole reason this module reaches inside. - # Asserted against the constructed object rather than the source text: - # what the adapter depends on is that `.conn` is reachable and is the - # connection it was given, not how selenium happens to write the - # assignment. A sentinel stands in for the connection — no session is - # needed to answer the question. - self.assertIn("conn", inspect.signature(Network.__init__).parameters) - - sentinel = object() - self.assertIs(Network(sentinel).conn, sentinel) - - def test_the_event_and_session_types_are_where_the_adapter_imports_them(self): - from selenium.webdriver.common.bidi.network import NetworkEvent - from selenium.webdriver.common.bidi.session import Session - - # Constructed as `NetworkEvent(name)` and `Session(conn).subscribe(...)`. - self.assertTrue(callable(NetworkEvent)) - self.assertTrue(hasattr(Session, "subscribe")) - - @unittest.skipUnless(_HAS_SELENIUM, "selenium is not installed") class TestTheChannelsThatSurvivedTheBiDiRegeneration(unittest.TestCase): """Console capture and the document-start preload go through `driver.script`, - which 4.44's regeneration left alone. Deliberately NOT gated on the cap, so - these keep guarding on whatever selenium is installed.""" + which 4.44's regeneration left alone. Deliberately NOT gated on the version + floor, so these keep guarding on whatever selenium is installed.""" def test_the_driver_exposes_the_bidi_channels(self): from selenium.webdriver.remote.webdriver import WebDriver From 397376223ea109384f58edc71f269acafc28aa1d Mon Sep 17 00:00:00 2001 From: Vishnu Vardhan Date: Wed, 19 Aug 2026 16:32:01 +0530 Subject: [PATCH 07/12] docs: name the change that fixed nightwatch cucumber DOM capture --- CLAUDE.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CLAUDE.md b/CLAUDE.md index dd4e4f90..e364b8c8 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -286,7 +286,7 @@ Documented divergences from the conventions above. They exist today as debt to b - **Nightwatch cucumber: traces generate, capture asserts with real pass/fail, and carry DOM mutations** (via the `test/` harness, 2026-07; both remaining gaps closed and re-verified 2026-08-06). Three fixes made the cucumber runner emit a useful trace: **(1) build** — `packages/nightwatch-devtools/tsup.config.ts` now compiles `src/helpers/cucumberHooks.cts` → `dist/helpers/cucumberHooks.cjs` as a self-contained CJS bundle (`@cucumber/cucumber` external). Previously the build ran only `tsup src/index.ts --clean`, so that file never existed and Cucumber's `require:[cucumberHooksPath]` registered *no* hooks (glob matched nothing) → zero capture. `PLUGIN_GLOBAL_KEY` moved to a leaf module `plugin-global-key.ts` so the hooks bundle stays tiny and CJS-safe (importing it via `constants.ts` dragged in core → `createRequire(import.meta.url)`, which throws when bundled to CJS). **(2) capture ordering** — a pre-quit cucumber `After` hook (`order:1000`, `captureCucumberScenarioBeforeQuit`) runs the trace capture + slice flush while the per-scenario browser session is live (the `order:-1` finalize is post browser-quit, so the flush bailed on the absent `sessionId`). Requires `traceGranularity:'test'` (per-scenario slices). **(3) native asserts** — the same pre-quit hook drains `browserProxy.drainNativeAssertCalls()` and calls `captureNativeAssertions` (the `afterEach` path early-returns for cucumber), so `assert.*` rows now appear. Console/network (BiDi) + commands + asserts + frames + sources + transcript are captured. BDD and live mode are unaffected. - **Assert outcomes are correlated off the assertion's own promise, because the results bag does not exist.** Cucumber's Nightwatch client is built by `createClient` with **no reporter**, so `SimplifiedReporter.logAssertResult` no-ops and `results.assertions`/`results.testcases` are *never* populated — no scenario-level reconcile can recover them, which is why `currentTest: undefined` was a dead end. `nativeAssertions.ts` `observedAssertOutcome` reads the outcome from the returned promise instead (`lib/core/asynctree.js` `shouldRejectNodePromise`: a failing `assert.*` rejects its deferred, a failing `verify.*` resolves *with* the AssertionError, a pass resolves with the command value). A fulfilment of `undefined` stays **neutral** — that is an assertion enqueued but never executed after an earlier `assert.*` emptied the queue, and reading it as a pass would paint a never-run assertion green. The results bag still wins where it exists and the row's window comes from whichever source supplied the outcome, so the describe/it timeline is byte-stable. Measured: **4 of 4 assert rows with a real pass/fail** (was 2 of 4 rows, 0 correlated), spanning real 44–372 ms windows instead of a synthetic 1 ms. - Relatedly, cucumber's **per-step** `resetCommandTracking()` was wiping the native-assert buffer, so each scenario kept only its *last* step's assertions (measured 1 of 2). The buffer is per **test unit** — `resetTestTracking()` at `wrapBrowserOnce` now clears it. - - **DOM `mutations` ARE captured**, and the old `ECONNRESET` / "collector not found" attribution is **wrong** — neither appears any more. The gap closed itself with document-start injection (`core/bidi-preload.ts`), confirmed on a *baseline* build so the credit is A10's. Measured per scenario with `traceGranularity:'test'` + `webSocketUrl: true`: 39 and 19 mutation entries, 2 DOM anchors each, **0 of 19 rows on the wrong document**. What remains is noise, not a gap: the screencast poller issues `/screenshot` every 200 ms into the session Nightwatch quits per scenario, logging 9–26 `WARN webdriverHttp: … socket hang up` per run. Fix is to stop the recorder before the per-scenario quit, or suppress the warn for a session in teardown. + - **DOM `mutations` ARE captured**, and the old `ECONNRESET` / "collector not found" attribution is **wrong** — neither appears any more. The gap closed itself with document-start injection (`core/bidi-preload.ts`), confirmed on a *baseline* build so the credit belongs to that change and not to anything here. Measured per scenario with `traceGranularity:'test'` + `webSocketUrl: true`: 39 and 19 mutation entries, 2 DOM anchors each, **0 of 19 rows on the wrong document**. What remains is noise, not a gap: the screencast poller issues `/screenshot` every 200 ms into the session Nightwatch quits per scenario, logging 9–26 `WARN webdriverHttp: … socket hang up` per run. Fix is to stop the recorder before the per-scenario quit, or suppress the warn for a session in teardown. - **A tsup entry that another entry imports is not a leaf, so an `import.meta.url === process.argv[1]` self-start check inside it is dead code; a CLI has to be its own leaf entry.** tsup hoists a module body shared by two entries into `dist/chunk-*.js`, and there `import.meta.url` is the chunk's path, which can never equal `process.argv[1]`. `packages/backend/src/show-trace.ts` imports `start` from `index.ts`, which is what made index shared, so index's old "start if run directly" guard was dead in **every** build: `node dist/index.js` exited 0 without ever serving, while `dist/show-trace.js` self-started correctly for exactly the same reason inverted, being a leaf whose body stays in its own output file (it also compares realpaths, because the invoked path is the `node_modules/.bin` symlink). The live dashboard server is therefore its own leaf entry, `packages/backend/src/server.ts` (shebang, built to an executable `dist/server.js`, shipped as the `devtools-backend` bin, accepting `--port`, `--hostname`, `-h`/`--help`), and `index.ts` stays library-only for the three adapters' in-process `start`/`stop`. Same family as the `cucumberHooks.cts` entry above: the tsup entry list is part of the contract, and in both cases the symptom was silence rather than an error. - **A captured text locator is generated in the recording runner's dialect; every other branch is portable CSS.** `shared/locator-dialect.ts` `locatorDialect(runner)` is the one fact table — WDIO runners (`mocha`/`jasmine`/`cucumber`) get `a*=Logout`, `nightwatch*`/`selenium-webdriver` and an **unidentified recorder** get `//a[contains(., "Logout")]`. The id reaches the page script as `CaptureActionSnapshotInput.runner` and the zip as an extension field on `context-options`, read back through `isTestRunnerId` onto `Metadata.runner`; absent in older and foreign zips, where the player shows no hint. Under WDIO a text carrying a `"` still emits XPath — WDIO compiles `tag*=` to XPath with `"` quoting and would build a broken expression — and WDIO resolves `//` itself. `locatorsMatch` decomposes **both** sides from either dialect, so `after.point` survives whichever way round the two grammars fall; the `concat()`-stitched literal is still left to exact comparison. `@wdio/elements`' standalone `getSnapshot` deliberately keeps the portable XPath default (its output is pasted into arbitrary tools), so a WDIO run's `browser.getSnapshot()` and its trace A11y tab disagree on that one branch. - `Metadata.runner` (typed `TestRunnerId`) and `metadata.options.framework` (untyped `string`, read by the sidebar's `getFramework`) are two carriers of the same fact. All three adapters now set both; the next change to either should collapse `getFramework` onto `Metadata.runner`. From f7c94b93a86673917e0a81289e6ee6e0dc4c8ff6 Mon Sep 17 00:00:00 2001 From: Vishnu Vardhan Date: Wed, 19 Aug 2026 17:05:20 +0530 Subject: [PATCH 08/12] fix(selenium-devtools-py): stop publishing the raw deserializer to other subscribers --- .../src/selenium_devtools/bidi.py | 80 ++++----- .../src/selenium_devtools/constants.py | 10 -- .../selenium-devtools-py/tests/test_bidi.py | 163 ++++++++++++------ .../tests/test_selenium_surface.py | 44 ++--- 4 files changed, 167 insertions(+), 130 deletions(-) diff --git a/packages/selenium-devtools-py/src/selenium_devtools/bidi.py b/packages/selenium-devtools-py/src/selenium_devtools/bidi.py index 6818c33e..34630b34 100644 --- a/packages/selenium-devtools-py/src/selenium_devtools/bidi.py +++ b/packages/selenium-devtools-py/src/selenium_devtools/bidi.py @@ -27,8 +27,8 @@ ``BeforeRequestSentParameters`` declares only ``initiator`` and the deserializer DROPS every param not declared, taking the request, its id and the timestamp with it — which leaves a response with nothing to correlate against. So -``_add_raw_event_handler`` swaps in a pass-through deserializer for the -registration and puts selenium's own back immediately. +``_add_raw_event_handler`` registers the callback with a pass-through +deserializer of its own, writing nothing that another subscriber reads. ``tests/test_selenium_surface.py`` guards that surface against the installed selenium. @@ -44,7 +44,6 @@ BIDI_CAPABILITY, BIDI_LEVEL_MAP, BIDI_NET_BEFORE_REQUEST, - BIDI_NET_EVENT_KEYS, BIDI_NET_RESPONSE_COMPLETED, LOGGER_NAME, SELENIUM_MINIMUM_VERSION, @@ -400,9 +399,6 @@ def event_params(event: Any) -> Dict[str, Any]: return event if isinstance(event, dict) else {} -_MISSING = object() - - class _RawEvent: """The deserializer selenium's dispatch expects, passing params through. @@ -419,53 +415,45 @@ def from_json(self, params: Any) -> Any: def supports_event_handler_api() -> bool: - """True when selenium presents the event-handler API capture subscribes - through. Detection only — nothing is mutated.""" + """True when selenium presents the regenerated BiDi network layer. + + Detection only — nothing is mutated. ``add_event_handler`` is the marker + rather than something this calls: it is public, it arrived with the layer, + and the event manager behind it is per-instance so there is nothing to check + for on the class. A manager that is then missing its parts raises inside + ``_add_raw_event_handler`` and is reported there. + """ try: - from selenium.webdriver.common.bidi.network import EventConfig, Network + from selenium.webdriver.common.bidi.network import Network except ImportError: return False - return bool( - EventConfig - and isinstance(getattr(Network, "EVENT_CONFIGS", None), dict) - and hasattr(Network, "add_event_handler") - ) + return hasattr(Network, "add_event_handler") def _add_raw_event_handler(network: Any, bidi_event: str, callback: Any) -> None: - """Subscribe ``callback`` to ``bidi_event`` with the RAW params, leaving - selenium's shared state exactly as it was found. - - Selenium picks the deserializer out of a per-BiDi-event map, so receiving raw - params means putting ours in that map. It is swapped in only for the duration - of the registration and the ORIGINAL OBJECT is put back, because - ``add_callback`` closes over the deserializer it was given: our handler keeps - the raw one for the life of the session, while every other handler — before - or after, ours or the user's — keeps selenium's own. - - Restoring matters beyond tidiness. Left in place this would hand raw dicts to - any other subscriber of these events in the process, breaking attribute - access on the generated objects they expect, and it would outlive the adapter. + """Subscribe ``callback`` to ``bidi_event`` with the RAW params. + + This is selenium's own ``add_event_handler`` body with one substitution: it + looks its deserializer up in a per-event map shared by every subscriber, and + we hand ours in directly. ``add_callback`` closes over the deserializer it is + given, so ours holds for the life of the session. + + Writing ours into that map instead would keep the registration on the public + API, but there is no safe window in which to do it: the swap has to stay in + place across ``add_event_handler``, which subscribes over the websocket, and + any handler another thread registers for the same event during that round + trip closes over OUR deserializer and receives dicts where it expects + selenium's generated objects. Passing it in writes nothing shared, so no such + window exists. """ - from selenium.webdriver.common.bidi.network import EventConfig - - key = BIDI_NET_EVENT_KEYS[bidi_event] - configs = network.EVENT_CONFIGS - wrappers = network._event_manager._event_wrappers - - had_key = key in configs - saved = wrappers.get(bidi_event, _MISSING) - configs[key] = EventConfig(key, bidi_event, dict) - wrappers[bidi_event] = _RawEvent(bidi_event) - try: - network.add_event_handler(key, callback) - finally: - if not had_key: - configs.pop(key, None) - if saved is _MISSING: - wrappers.pop(bidi_event, None) - else: - wrappers[bidi_event] = saved + manager = network._event_manager + callback_id = manager.conn.add_callback(_RawEvent(bidi_event), callback) + manager.subscribe_to_event(bidi_event) + # Selenium counts callbacks per event to decide when a subscription is no + # longer needed. Ours is registered on the connection directly, so without + # this it is invisible to that count and another consumer removing their + # handler would unsubscribe the event out from under us. + manager.add_callback_to_tracking(bidi_event, callback_id) _reported_incomplete: set = set() diff --git a/packages/selenium-devtools-py/src/selenium_devtools/constants.py b/packages/selenium-devtools-py/src/selenium_devtools/constants.py index 515fc7b8..5e68ce2a 100644 --- a/packages/selenium-devtools-py/src/selenium_devtools/constants.py +++ b/packages/selenium-devtools-py/src/selenium_devtools/constants.py @@ -107,16 +107,6 @@ # than raising an AttributeError, and so the surface guards know what they apply # to. Also the reason ``requires-python`` is >=3.10: selenium 4.44 requires it. SELENIUM_MINIMUM_VERSION = (4, 44) -# Keys the adapter registers into ``Network.EVENT_CONFIGS`` on 4.44+, so its -# handlers receive RAW event params. Registered under our own names rather than -# reusing selenium's, because selenium's generated event dataclasses model only -# each event's own extension field — ``BeforeRequestSentParameters`` declares -# just ``initiator`` — and its deserializer DROPS every param not declared, so -# the typed path loses the request, its id and the timestamp. -BIDI_NET_EVENT_KEYS = { - BIDI_NET_BEFORE_REQUEST: "devtools_before_request_sent", - BIDI_NET_RESPONSE_COMPLETED: "devtools_response_completed", -} # selenium's BiDi log entries already carry lowercase levels; this normalizes # the stragglers to the shared LogLevel union. Unmapped levels fall back to log. BIDI_LEVEL_MAP = { diff --git a/packages/selenium-devtools-py/tests/test_bidi.py b/packages/selenium-devtools-py/tests/test_bidi.py index ab26b881..727c3385 100644 --- a/packages/selenium-devtools-py/tests/test_bidi.py +++ b/packages/selenium-devtools-py/tests/test_bidi.py @@ -410,6 +410,39 @@ class BeforeRequestSentParameters: def __init__(self, initiator=None): self.initiator = initiator + class Conn: + """The websocket connection. `add_callback` CLOSES OVER the deserializer + it is handed, which is what lets the adapter pass its own in rather than + publish it to a shared map.""" + + def __init__(self): + self.callbacks = {} + self.next_id = 0 + + def add_callback(self, event, callback): + self.next_id += 1 + self.callbacks.setdefault(event.event_class, []).append( + lambda params: callback(event.from_json(params)) + ) + return self.next_id + + class EventManager: + def __init__(self, configs): + self.conn = Conn() + self.subscribed = [] + self.tracked = [] + # One deserializer per BiDi event, shared by every subscriber. + self._event_wrappers = { + config.bidi_event: TypedWrapper(config.bidi_event, config.event_class) + for config in configs.values() + } + + def subscribe_to_event(self, bidi_event, contexts=None): + self.subscribed.append(bidi_event) + + def add_callback_to_tracking(self, bidi_event, callback_id): + self.tracked.append((bidi_event, callback_id)) + class Network: EVENT_CONFIGS = { "before_request_sent": EventConfig( @@ -423,27 +456,17 @@ class Network: } def __init__(self): - self.handlers = {} - # selenium builds one deserializer per BiDi event here, and - # `add_callback` then CLOSES OVER the one it is handed — which is - # what makes restoring the map afterwards safe. - self._event_manager = types.SimpleNamespace( - _event_wrappers={ - config.bidi_event: TypedWrapper( - config.bidi_event, config.event_class - ) - for config in self.EVENT_CONFIGS.values() - } - ) + self._event_manager = EventManager(self.EVENT_CONFIGS) def add_event_handler(self, event, callback, contexts=None): - # selenium raises for an unregistered key rather than ignoring it. + """Selenium's own registration, which the adapter does NOT use: it + takes the deserializer from the shared map. Kept so a test can + register through it and prove it still gets selenium's own.""" config = self.EVENT_CONFIGS.get(event) if config is None: raise ValueError(f"Event '{event}' not found") wrapper = self._event_manager._event_wrappers[config.bidi_event] - self.handlers[event] = lambda params: callback(wrapper.from_json(params)) - return len(self.handlers) + return self._event_manager.conn.add_callback(wrapper, callback) if not with_event_manager: del Network.add_event_handler @@ -451,6 +474,7 @@ def add_event_handler(self, event, callback, contexts=None): module.EventConfig = EventConfig module.Network = Network + module.BeforeRequestSentParameters = BeforeRequestSentParameters return module @@ -475,7 +499,13 @@ def script(self): class TestTheEventManagerPath(unittest.TestCase): - """selenium 4.44+ — subscribing through the public `add_event_handler`.""" + """selenium 4.44+ — registering against the regenerated BiDi layer.""" + + @staticmethod + def _dispatch(network, bidi_event, params): + """Deliver an event the way selenium's connection does.""" + for callback in network._event_manager.conn.callbacks[bidi_event]: + callback(params) def test_raw_params_reach_the_handlers_and_are_captured(self): module = fake_network_module() @@ -485,17 +515,19 @@ def test_raw_params_reach_the_handlers_and_are_captured(self): with mock.patch.dict( sys.modules, {"selenium.webdriver.common.bidi.network": module} ): - self.assertTrue(bidi._attach_network(NewSeleniumDriver(network, []), capturer)) - - sent = network.handlers["devtools_before_request_sent"] - done = network.handlers["devtools_response_completed"] + self.assertTrue( + bidi._attach_network(NewSeleniumDriver(network, []), capturer) + ) - # Raw params, exactly as selenium's dict-config deserializer delivers. - sent({"request": {"request": "R1", "url": "https://x/a.js", "method": "GET"}, - "timestamp": 1000}) - done({"request": {"request": "R1"}, "timestamp": 1200, - "response": {"status": 200, "statusText": "OK", - "mimeType": "text/javascript", "bytesReceived": 12}}) + self._dispatch(network, "network.beforeRequestSent", { + "request": {"request": "R1", "url": "https://x/a.js", "method": "GET"}, + "timestamp": 1000, + }) + self._dispatch(network, "network.responseCompleted", { + "request": {"request": "R1"}, "timestamp": 1200, + "response": {"status": 200, "statusText": "OK", + "mimeType": "text/javascript", "bytesReceived": 12}, + }) # capture_network sends one batch per call, each a list of one frame. frames = [ @@ -506,12 +538,39 @@ def test_raw_params_reach_the_handlers_and_are_captured(self): self.assertEqual(frames[0]["url"], "https://x/a.js") self.assertEqual(frames[1]["status"], 200) # correlated with the request - def test_selenium_shared_state_is_left_exactly_as_found(self): - """The deserializer swap must not outlive the registration. + def test_both_events_are_subscribed_and_counted(self): + # Registering on the connection directly bypasses selenium's own + # bookkeeping, so the subscribe and the callback count are done + # explicitly — without the count, another consumer removing their + # handler would unsubscribe the event out from under us. + module = fake_network_module() + network = module.Network() - Left in place it would hand raw dicts to any other subscriber of these - events in the process — breaking attribute access on the generated - objects they expect — and would persist after the adapter is done. + with mock.patch.dict( + sys.modules, {"selenium.webdriver.common.bidi.network": module} + ): + bidi._attach_network( + NewSeleniumDriver(network, []), SessionCapturer(FakeTransport()) + ) + + manager = network._event_manager + self.assertEqual( + sorted(manager.subscribed), + ["network.beforeRequestSent", "network.responseCompleted"], + ) + self.assertEqual( + sorted(event for event, _ in manager.tracked), + ["network.beforeRequestSent", "network.responseCompleted"], + ) + + def test_nothing_another_subscriber_reads_is_written(self): + """The adapter's deserializer must never be visible to anyone else. + + Publishing it into the shared per-event map would be the public-API + route, but there is no safe window: the swap has to span the websocket + subscribe inside `add_event_handler`, and any handler registered during + that round trip would close over ours and receive dicts where it expects + selenium's generated objects. """ module = fake_network_module() network = module.Network() @@ -530,37 +589,37 @@ def test_selenium_shared_state_is_left_exactly_as_found(self): ) self.assertEqual(module.Network.EVENT_CONFIGS, configs_before) - # Identity, not equality: selenium's own deserializer OBJECTS are back, - # so a handler registered later behaves exactly as it would have. + # Identity, not equality: the very objects are untouched, so a handler + # registered at ANY time behaves exactly as it would have. self.assertEqual(wrappers, wrappers_before) for event, wrapper in wrappers_before.items(): self.assertIs(wrappers[event], wrapper) - def test_our_handler_keeps_raw_params_after_the_restore(self): - """`add_callback` closes over the deserializer it was handed, so putting - selenium's back does not reach into a handler already registered. This is - the assumption the whole isolation rests on.""" + def test_a_concurrent_subscriber_still_gets_selenium_deserializer(self): + """The race Greptile raised, made concrete: someone else registering for + the SAME event still gets the generated object, not our dict.""" module = fake_network_module() network = module.Network() - capturer = SessionCapturer(FakeTransport()) + seen = [] with mock.patch.dict( sys.modules, {"selenium.webdriver.common.bidi.network": module} ): - bidi._attach_network(NewSeleniumDriver(network, []), capturer) - - # Dispatched AFTER the restore, through selenium's own registration. - network.handlers["devtools_before_request_sent"]( - {"request": {"request": "R1", "url": "https://x/a.js", "method": "GET"}, - "timestamp": 1000} - ) - - frames = [ - batch[0] for scope, batch in capturer._tx.sent - if scope == "networkRequests" - ] - self.assertEqual(len(frames), 1) - self.assertEqual(frames[0]["url"], "https://x/a.js") + bidi._attach_network( + NewSeleniumDriver(network, []), SessionCapturer(FakeTransport()) + ) + # Selenium's own registration, for an event the adapter also holds. + network.add_event_handler("before_request_sent", seen.append) + + self._dispatch(network, "network.beforeRequestSent", { + "request": {"request": "R1", "url": "https://x/a.js", "method": "GET"}, + "initiator": {"type": "script"}, "timestamp": 1000, + }) + + self.assertEqual(len(seen), 1) + # The generated dataclass, with attribute access intact — NOT a dict. + self.assertIsInstance(seen[0], module.BeforeRequestSentParameters) + self.assertEqual(seen[0].initiator, {"type": "script"}) def test_a_selenium_without_the_event_api_is_reported_not_silent(self): """There is no second path to fall back to, so a selenium that cannot diff --git a/packages/selenium-devtools-py/tests/test_selenium_surface.py b/packages/selenium-devtools-py/tests/test_selenium_surface.py index b463f824..3c3535da 100644 --- a/packages/selenium-devtools-py/tests/test_selenium_surface.py +++ b/packages/selenium-devtools-py/tests/test_selenium_surface.py @@ -61,28 +61,28 @@ def test_the_public_event_handler_api_is_present(self): self.assertTrue(callable(getattr(Network, "add_event_handler", None))) self.assertIsInstance(getattr(Network, "EVENT_CONFIGS", None), dict) - def test_event_configs_carry_the_two_events_capture_needs(self): - from selenium.webdriver.common.bidi.network import Network - - from selenium_devtools.constants import ( - BIDI_NET_BEFORE_REQUEST, - BIDI_NET_RESPONSE_COMPLETED, - ) - - # Registration reuses selenium's own EventConfig shape, so the names it - # subscribes by have to be the ones selenium routes on. - wired = {config.bidi_event for config in Network.EVENT_CONFIGS.values()} - self.assertIn(BIDI_NET_BEFORE_REQUEST, wired) - self.assertIn(BIDI_NET_RESPONSE_COMPLETED, wired) - - def test_event_config_takes_the_three_fields_registration_supplies(self): - from selenium.webdriver.common.bidi.network import EventConfig - - config = EventConfig("k", "network.responseCompleted", dict) - - self.assertEqual(config.event_key, "k") - self.assertEqual(config.bidi_event, "network.responseCompleted") - self.assertIs(config.event_class, dict) + def test_the_event_manager_carries_what_registration_calls(self): + from selenium.webdriver.common.bidi._event_manager import _EventManager + + # `_add_raw_event_handler` is this class's own add_event_handler body + # with the deserializer passed in rather than looked up, so it calls + # exactly these. Private, and pinned for that reason. + self.assertIn("conn", inspect.signature(_EventManager.__init__).parameters) + for method in ("subscribe_to_event", "add_callback_to_tracking"): + self.assertTrue(callable(getattr(_EventManager, method, None)), method) + + def test_the_connection_deserializes_per_callback(self): + """The property the whole design rests on: `add_callback` closes over the + deserializer it is HANDED. If it ever resolved one per event instead, the + adapter could no longer keep raw params to itself and would be back to + publishing its own into shared state.""" + from selenium.webdriver.remote.websocket_connection import WebSocketConnection + + source = inspect.getsource(WebSocketConnection.add_callback) + + # The callback body must call from_json on the passed-in event object. + self.assertIn("event.from_json", source) + self.assertIn("event.event_class", source) def test_the_generated_event_classes_are_still_lossy(self): """The reason raw `dict` configs are registered at all. From 596664c9503b3ff9ac583d2e2012a5f0985d63d8 Mon Sep 17 00:00:00 2001 From: Vishnu Vardhan Date: Wed, 19 Aug 2026 17:16:08 +0530 Subject: [PATCH 09/12] fix(selenium-devtools-py): capture nothing unless every subscription succeeded --- .../src/selenium_devtools/bidi.py | 48 ++++++++++++++----- .../selenium-devtools-py/tests/test_bidi.py | 43 +++++++++++++++++ 2 files changed, 78 insertions(+), 13 deletions(-) diff --git a/packages/selenium-devtools-py/src/selenium_devtools/bidi.py b/packages/selenium-devtools-py/src/selenium_devtools/bidi.py index 34630b34..47fb7946 100644 --- a/packages/selenium-devtools-py/src/selenium_devtools/bidi.py +++ b/packages/selenium-devtools-py/src/selenium_devtools/bidi.py @@ -492,8 +492,14 @@ def _attach_network(driver: Any, capturer: SessionCapturer) -> bool: Returns False (and logs) on any failure — network BiDi is best-effort. """ pending: Dict[str, Dict[str, Any]] = {} + # Both handlers are inert until every subscription is in place — see + # _subscribe_via_event_manager for why a half-subscribed pair is worse than + # no capture at all. + active = {"ok": False} def on_request_sent(event: Any) -> None: + if not active["ok"]: + return try: params = event_params(event) if _incomplete_event(params, BIDI_NET_BEFORE_REQUEST): @@ -508,6 +514,8 @@ def on_request_sent(event: Any) -> None: captured = {"n": 0} def on_response_completed(event: Any) -> None: + if not active["ok"]: + return try: params = event_params(event) if _incomplete_event(params, BIDI_NET_RESPONSE_COMPLETED): @@ -527,18 +535,32 @@ def on_response_completed(event: Any) -> None: _warn(f"responseCompleted handler threw: {exc}") return _subscribe_via_event_manager( - driver, on_request_sent, on_response_completed + driver, + { + BIDI_NET_BEFORE_REQUEST: on_request_sent, + BIDI_NET_RESPONSE_COMPLETED: on_response_completed, + }, + active, ) def _subscribe_via_event_manager( - driver: Any, on_request_sent: Any, on_response_completed: Any + driver: Any, handlers: Dict[str, Any], active: Dict[str, bool] ) -> bool: - """selenium 4.44+: subscribe through the public ``add_event_handler``. + """selenium 4.44+: subscribe each handler to its BiDi event. Deliberately not ``add_request_handler``/``add_response_handler``: both register an intercept even in their high-level form, which pauses every request until selenium continues it. This only observes. + + ``active`` is flipped once EVERY handler is registered, and the handlers + consult it, so capture runs only on the complete set. A half-subscribed pair + is worse than none: ``beforeRequestSent`` on its own emits a pending frame + per request that only ``responseCompleted`` finalizes, so the Network tab + fills with requests stuck pending and ``pending`` grows for the rest of the + session. That covers the gap between the two subscribes as well as an + outright failure, and it needs no unregister path — which would be more + private surface, and could fail in turn while handling a failure. """ if not supports_event_handler_api(): # Checked rather than caught, so a too-old selenium is reported as a @@ -547,18 +569,18 @@ def _subscribe_via_event_manager( return False try: network = driver.network - _add_raw_event_handler(network, BIDI_NET_BEFORE_REQUEST, on_request_sent) - _add_raw_event_handler( - network, BIDI_NET_RESPONSE_COMPLETED, on_response_completed - ) - _log.info( - "network capture subscribed via the event-handler API (selenium %s)", - ".".join(str(p) for p in selenium_version()), - ) - return True + for bidi_event, callback in handlers.items(): + _add_raw_event_handler(network, bidi_event, callback) except Exception as exc: # noqa: BLE001 - _warn(f"network subscribe failed: {exc}") + _warn(f"network subscribe failed, no network events captured: {exc}") return False + active["ok"] = True + _log.info( + "network capture subscribed via the event-handler API (selenium %s)", + ".".join(str(p) for p in selenium_version()), + ) + return True + def attach(driver: Any, capturer: SessionCapturer) -> bool: """Wire BiDi console + network capture onto ``driver``. diff --git a/packages/selenium-devtools-py/tests/test_bidi.py b/packages/selenium-devtools-py/tests/test_bidi.py index 727c3385..5b912187 100644 --- a/packages/selenium-devtools-py/tests/test_bidi.py +++ b/packages/selenium-devtools-py/tests/test_bidi.py @@ -621,6 +621,49 @@ def test_a_concurrent_subscriber_still_gets_selenium_deserializer(self): self.assertIsInstance(seen[0], module.BeforeRequestSentParameters) self.assertEqual(seen[0].initiator, {"type": "script"}) + def test_a_half_subscribed_pair_captures_nothing(self): + """The first registration succeeding and the second failing must not + leave capture half-on. + + beforeRequestSent alone emits a pending frame per request that only + responseCompleted finalizes, so the Network tab would fill with requests + stuck pending and the pending map would grow for the rest of the session + — while attach() reported failure and the caller believed nothing was + capturing. + """ + module = fake_network_module() + network = module.Network() + capturer = SessionCapturer(FakeTransport()) + + real_subscribe = network._event_manager.subscribe_to_event + + def failing_subscribe(bidi_event, contexts=None): + if bidi_event == "network.responseCompleted": + raise RuntimeError("websocket went away") + return real_subscribe(bidi_event, contexts) + + network._event_manager.subscribe_to_event = failing_subscribe + + with mock.patch.dict( + sys.modules, {"selenium.webdriver.common.bidi.network": module} + ): + with self.assertLogs("selenium_devtools.bidi", level="WARNING"): + attached = bidi._attach_network( + NewSeleniumDriver(network, []), capturer + ) + + self.assertFalse(attached) + + # The first callback IS still registered — there is no unregister path. + # It must simply do nothing. + self._dispatch(network, "network.beforeRequestSent", { + "request": {"request": "R1", "url": "https://x/a.js", "method": "GET"}, + "timestamp": 1000, + }) + + frames = [s for s, _ in capturer._tx.sent if s == "networkRequests"] + self.assertEqual(frames, []) + def test_a_selenium_without_the_event_api_is_reported_not_silent(self): """There is no second path to fall back to, so a selenium that cannot provide this has to say so. Silence here is an empty Network tab with no From 2b8b11050388ed341c4d7a62119c18894dc649c8 Mon Sep 17 00:00:00 2001 From: Vishnu Vardhan Date: Wed, 19 Aug 2026 17:33:06 +0530 Subject: [PATCH 10/12] fix(selenium-devtools-py): unwind registrations when attach fails part-way --- .../src/selenium_devtools/bidi.py | 60 +++++++++++++--- .../selenium-devtools-py/tests/test_bidi.py | 68 +++++++++++++++++-- .../tests/test_selenium_surface.py | 16 +++++ 3 files changed, 127 insertions(+), 17 deletions(-) diff --git a/packages/selenium-devtools-py/src/selenium_devtools/bidi.py b/packages/selenium-devtools-py/src/selenium_devtools/bidi.py index 47fb7946..dbe57559 100644 --- a/packages/selenium-devtools-py/src/selenium_devtools/bidi.py +++ b/packages/selenium-devtools-py/src/selenium_devtools/bidi.py @@ -447,13 +447,43 @@ def _add_raw_event_handler(network: Any, bidi_event: str, callback: Any) -> None window exists. """ manager = network._event_manager - callback_id = manager.conn.add_callback(_RawEvent(bidi_event), callback) + wrapper = _RawEvent(bidi_event) + callback_id = manager.conn.add_callback(wrapper, callback) manager.subscribe_to_event(bidi_event) # Selenium counts callbacks per event to decide when a subscription is no # longer needed. Ours is registered on the connection directly, so without # this it is invisible to that count and another consumer removing their # handler would unsubscribe the event out from under us. manager.add_callback_to_tracking(bidi_event, callback_id) + # Returned so a failed attach can undo exactly what it did. + return wrapper, callback_id + + +def _undo_registration( + manager: Any, bidi_event: str, wrapper: Any, callback_id: Any +) -> None: + """Best-effort unwind of one registration. + + Being counted in selenium's bookkeeping is what makes leaving one behind + harmful: an abandoned callback keeps the event's callback count above zero, + so a later consumer removing THEIR handler no longer unsubscribes, and the + browser keeps sending the event for the rest of the session. + + Each step is guarded on its own. The registration may have failed part-way, + and a step with nothing to undo must not stop the ones that do. The callback + and its count go first so ``unsubscribe_from_event`` sees an empty list — + it only unsubscribes when no callbacks remain, so this cannot take down a + subscription another consumer is still using. + """ + for label, undo in ( + ("callback", lambda: manager.conn.remove_callback(wrapper, callback_id)), + ("count", lambda: manager.remove_callback_from_tracking(bidi_event, callback_id)), + ("subscription", lambda: manager.unsubscribe_from_event(bidi_event)), + ): + try: + undo() + except Exception as exc: # noqa: BLE001 — unwinding must not raise + _log.debug("could not unwind the %s for %s: %s", label, bidi_event, exc) _reported_incomplete: set = set() @@ -553,26 +583,36 @@ def _subscribe_via_event_manager( register an intercept even in their high-level form, which pauses every request until selenium continues it. This only observes. - ``active`` is flipped once EVERY handler is registered, and the handlers - consult it, so capture runs only on the complete set. A half-subscribed pair - is worse than none: ``beforeRequestSent`` on its own emits a pending frame - per request that only ``responseCompleted`` finalizes, so the Network tab - fills with requests stuck pending and ``pending`` grows for the rest of the - session. That covers the gap between the two subscribes as well as an - outright failure, and it needs no unregister path — which would be more - private surface, and could fail in turn while handling a failure. + A failure part-way through is handled twice over, because the two problems + are different. ``active`` is flipped only once EVERY handler is registered, + and the handlers consult it, so no incomplete DATA is ever produced: + ``beforeRequestSent`` on its own emits a pending frame per request that only + ``responseCompleted`` finalizes, which would fill the Network tab with + requests stuck pending and grow ``pending`` for the rest of the session. That + also covers the gap between the two subscribes, not just an outright failure. + Then the registrations already made are unwound, so no STATE is left behind + either — see ``_undo_registration`` for why an abandoned one is not inert. + The flag is the guarantee; the unwind is best-effort and may find nothing. """ if not supports_event_handler_api(): # Checked rather than caught, so a too-old selenium is reported as a # version floor instead of an AttributeError about a generated attribute. _warn(network_unavailable_reason(AttributeError("no BiDi event API"))) return False + manager = None + registered = [] try: network = driver.network + manager = network._event_manager for bidi_event, callback in handlers.items(): - _add_raw_event_handler(network, bidi_event, callback) + wrapper, callback_id = _add_raw_event_handler( + network, bidi_event, callback + ) + registered.append((bidi_event, wrapper, callback_id)) except Exception as exc: # noqa: BLE001 _warn(f"network subscribe failed, no network events captured: {exc}") + for bidi_event, wrapper, callback_id in registered: + _undo_registration(manager, bidi_event, wrapper, callback_id) return False active["ok"] = True _log.info( diff --git a/packages/selenium-devtools-py/tests/test_bidi.py b/packages/selenium-devtools-py/tests/test_bidi.py index 5b912187..f28275bb 100644 --- a/packages/selenium-devtools-py/tests/test_bidi.py +++ b/packages/selenium-devtools-py/tests/test_bidi.py @@ -416,16 +416,22 @@ class Conn: publish it to a shared map.""" def __init__(self): - self.callbacks = {} + self.callbacks = {} # event name -> [(callback_id, fn)] self.next_id = 0 def add_callback(self, event, callback): self.next_id += 1 self.callbacks.setdefault(event.event_class, []).append( - lambda params: callback(event.from_json(params)) + (self.next_id, lambda params: callback(event.from_json(params))) ) return self.next_id + def remove_callback(self, event, callback_id): + entries = self.callbacks.get(event.event_class, []) + self.callbacks[event.event_class] = [ + entry for entry in entries if entry[0] != callback_id + ] + class EventManager: def __init__(self, configs): self.conn = Conn() @@ -443,6 +449,18 @@ def subscribe_to_event(self, bidi_event, contexts=None): def add_callback_to_tracking(self, bidi_event, callback_id): self.tracked.append((bidi_event, callback_id)) + def remove_callback_from_tracking(self, bidi_event, callback_id): + self.tracked.remove((bidi_event, callback_id)) + + def unsubscribe_from_event(self, bidi_event): + # Selenium only unsubscribes when no callbacks remain, so a live + # consumer's subscription cannot be taken down by someone else's + # unwind. Mirrored, or the test would pass on a wrong implementation. + if any(event == bidi_event for event, _ in self.tracked): + return + while bidi_event in self.subscribed: + self.subscribed.remove(bidi_event) + class Network: EVENT_CONFIGS = { "before_request_sent": EventConfig( @@ -504,8 +522,8 @@ class TestTheEventManagerPath(unittest.TestCase): @staticmethod def _dispatch(network, bidi_event, params): """Deliver an event the way selenium's connection does.""" - for callback in network._event_manager.conn.callbacks[bidi_event]: - callback(params) + for _callback_id, fn in network._event_manager.conn.callbacks[bidi_event]: + fn(params) def test_raw_params_reach_the_handlers_and_are_captured(self): module = fake_network_module() @@ -654,16 +672,52 @@ def failing_subscribe(bidi_event, contexts=None): self.assertFalse(attached) - # The first callback IS still registered — there is no unregister path. - # It must simply do nothing. + manager = network._event_manager + # Nothing left behind. An abandoned callback would keep the event's + # callback count above zero, so a later consumer removing THEIR handler + # would no longer unsubscribe and the browser would keep sending it. + self.assertEqual(manager.conn.callbacks.get("network.beforeRequestSent"), []) + self.assertEqual(manager.tracked, []) + self.assertNotIn("network.beforeRequestSent", manager.subscribed) + + # And no data even if something did survive: the flag is the guarantee, + # the unwind is best-effort. self._dispatch(network, "network.beforeRequestSent", { "request": {"request": "R1", "url": "https://x/a.js", "method": "GET"}, "timestamp": 1000, }) - frames = [s for s, _ in capturer._tx.sent if s == "networkRequests"] self.assertEqual(frames, []) + def test_an_unwind_never_takes_down_a_live_subscription(self): + """`unsubscribe_from_event` only fires with no callbacks left, so another + consumer already listening to the same event keeps theirs.""" + module = fake_network_module() + network = module.Network() + manager = network._event_manager + + # A consumer subscribed before the adapter attaches. + manager.subscribe_to_event("network.beforeRequestSent") + manager.add_callback_to_tracking("network.beforeRequestSent", 999) + + def failing_subscribe(bidi_event, contexts=None): + if bidi_event == "network.responseCompleted": + raise RuntimeError("websocket went away") + manager.subscribed.append(bidi_event) + + manager.subscribe_to_event = failing_subscribe + + with mock.patch.dict( + sys.modules, {"selenium.webdriver.common.bidi.network": module} + ): + with self.assertLogs("selenium_devtools.bidi", level="WARNING"): + bidi._attach_network( + NewSeleniumDriver(network, []), SessionCapturer(FakeTransport()) + ) + + self.assertIn("network.beforeRequestSent", manager.subscribed) + self.assertIn(("network.beforeRequestSent", 999), manager.tracked) + def test_a_selenium_without_the_event_api_is_reported_not_silent(self): """There is no second path to fall back to, so a selenium that cannot provide this has to say so. Silence here is an empty Network tab with no diff --git a/packages/selenium-devtools-py/tests/test_selenium_surface.py b/packages/selenium-devtools-py/tests/test_selenium_surface.py index 3c3535da..e36a5e4e 100644 --- a/packages/selenium-devtools-py/tests/test_selenium_surface.py +++ b/packages/selenium-devtools-py/tests/test_selenium_surface.py @@ -71,6 +71,22 @@ def test_the_event_manager_carries_what_registration_calls(self): for method in ("subscribe_to_event", "add_callback_to_tracking"): self.assertTrue(callable(getattr(_EventManager, method, None)), method) + def test_a_registration_can_be_unwound(self): + """A failed attach undoes what it did. Without these, an abandoned + callback keeps the event's count above zero and a later consumer can + never unsubscribe it.""" + from selenium.webdriver.common.bidi._event_manager import _EventManager + from selenium.webdriver.remote.websocket_connection import WebSocketConnection + + self.assertTrue(callable(getattr(WebSocketConnection, "remove_callback", None))) + for method in ("remove_callback_from_tracking", "unsubscribe_from_event"): + self.assertTrue(callable(getattr(_EventManager, method, None)), method) + + # The unwind relies on this staying conditional: it must not tear down a + # subscription another consumer still has callbacks on. + source = inspect.getsource(_EventManager.unsubscribe_from_event) + self.assertIn('entry["callbacks"]', source) + def test_the_connection_deserializes_per_callback(self): """The property the whole design rests on: `add_callback` closes over the deserializer it is HANDED. If it ever resolved one per event instead, the From 4aea111847fe08faa0c28b8960b41ab55fdf6b08 Mon Sep 17 00:00:00 2001 From: Vishnu Vardhan Date: Wed, 19 Aug 2026 18:13:28 +0530 Subject: [PATCH 11/12] fix(selenium-devtools-py): report network capture once per session, not per event --- .../src/selenium_devtools/bidi.py | 58 ++++++++++++++----- .../src/selenium_devtools/instrumentation.py | 12 +++- .../selenium-devtools-py/tests/test_bidi.py | 57 ++++++++++++++++++ .../tests/test_instrumentation.py | 6 +- 4 files changed, 114 insertions(+), 19 deletions(-) diff --git a/packages/selenium-devtools-py/src/selenium_devtools/bidi.py b/packages/selenium-devtools-py/src/selenium_devtools/bidi.py index dbe57559..4490d759 100644 --- a/packages/selenium-devtools-py/src/selenium_devtools/bidi.py +++ b/packages/selenium-devtools-py/src/selenium_devtools/bidi.py @@ -516,12 +516,23 @@ def _incomplete_event(params: Dict[str, Any], label: str) -> bool: return True -def _attach_network(driver: Any, capturer: SessionCapturer) -> bool: +def _attach_network( + driver: Any, capturer: SessionCapturer, stats: Optional[Dict[str, Any]] = None +) -> bool: """Subscribe to network events WITHOUT interception (see module docstring). Returns False (and logs) on any failure — network BiDi is best-effort. + + ``stats`` is filled in for the caller to report at teardown, rather than + logged per event: the dashboard's Network tab already lists every request, so + a running count is one Console line per request saying what the UI beside it + already shows. What the tab cannot show is a request whose response never + arrived, which is what ``pending`` still holding entries at the end means. """ + stats = stats if stats is not None else {} + stats["captured"] = 0 pending: Dict[str, Dict[str, Any]] = {} + stats["pending"] = pending # Both handlers are inert until every subscription is in place — see # _subscribe_via_event_manager for why a half-subscribed pair is worse than # no capture at all. @@ -541,8 +552,6 @@ def on_request_sent(event: Any) -> None: except Exception as exc: # noqa: BLE001 _warn(f"beforeRequestSent handler threw: {exc}") - captured = {"n": 0} - def on_response_completed(event: Any) -> None: if not active["ok"]: return @@ -554,13 +563,7 @@ def on_response_completed(event: Any) -> None: if kwargs is not None: pending.pop(kwargs["request_id"], None) capturer.capture_network(**kwargs) - captured["n"] += 1 - # The first one is the proof the subscription is live end to - # end; the rest are a count, because an empty Network tab and a - # tab nobody looked at are indistinguishable after the fact. - if captured["n"] == 1: - _log.info("network capture live, first response: %s", kwargs["url"]) - _log.debug("network entries captured: %d", captured["n"]) + stats["captured"] += 1 except Exception as exc: # noqa: BLE001 _warn(f"responseCompleted handler threw: {exc}") @@ -615,19 +618,42 @@ def _subscribe_via_event_manager( _undo_registration(manager, bidi_event, wrapper, callback_id) return False active["ok"] = True - _log.info( - "network capture subscribed via the event-handler API (selenium %s)", - ".".join(str(p) for p in selenium_version()), - ) return True -def attach(driver: Any, capturer: SessionCapturer) -> bool: +def network_summary(stats: Optional[Dict[str, Any]]) -> Optional[str]: + """One line describing what network capture actually did, or None when there + is nothing worth saying. + + Reported once at teardown rather than per event. The count is the cheap half; + the useful half is requests still pending, which means a response never + arrived and is the one thing the Network tab cannot show on its own. + """ + if not stats: + return None + captured = stats.get("captured", 0) + unanswered = len(stats.get("pending") or ()) + if not captured and not unanswered: + return None + if unanswered: + return ( + f"network: {captured} request(s) captured, " + f"{unanswered} still awaiting a response at teardown" + ) + return f"network: {captured} request(s) captured" + + +def attach( + driver: Any, capturer: SessionCapturer, stats: Optional[Dict[str, Any]] = None +) -> bool: """Wire BiDi console + network capture onto ``driver``. Returns True if at least one channel attached. A driver without the ``webSocketUrl`` capability (BiDi not enabled at build time) is skipped with a one-line warning — capture continues via the command stream only. + + ``stats`` is an optional bag the caller keeps, to be passed to + ``network_summary`` when the session ends. """ if not _bidi_enabled(driver): _warn( @@ -638,6 +664,6 @@ def attach(driver: Any, capturer: SessionCapturer) -> bool: attached = 0 if _attach_console(driver, capturer): attached += 1 - if _attach_network(driver, capturer): + if _attach_network(driver, capturer, stats): attached += 1 return attached > 0 diff --git a/packages/selenium-devtools-py/src/selenium_devtools/instrumentation.py b/packages/selenium-devtools-py/src/selenium_devtools/instrumentation.py index 477e0ed3..140d94b7 100644 --- a/packages/selenium-devtools-py/src/selenium_devtools/instrumentation.py +++ b/packages/selenium-devtools-py/src/selenium_devtools/instrumentation.py @@ -281,6 +281,9 @@ def _close_entry(capturer: SessionCapturer, entry: dict) -> None: _flush_mutations(capturer, entry) entry["snapshot"] = None _finalize_screencast(capturer, entry["session_id"], entry) + summary = bidi.network_summary(entry.get("network")) + if summary: + _log.info(summary) def _finalize_screencast( @@ -342,7 +345,10 @@ def _ensure_session_setup(driver: Any, capturer: SessionCapturer) -> Optional[di _log.info("session %s started", session_id) _send_default_suite(capturer, "running") # tree entry for plain-script runs try: - if bidi.attach(driver, capturer): + # Filled in as events arrive; reported once by _on_quit, because the + # Network tab already lists every request as it happens. + entry["network"] = {} + if bidi.attach(driver, capturer, entry["network"]): _log.info("BiDi attached — capturing console + network") except Exception as exc: # noqa: BLE001 — capture must never break the test _log.warning("BiDi attach threw: %s", exc) @@ -536,7 +542,9 @@ def patched_execute(self, driver_command: str, params: Any = None): # noqa: ANN call_source=src, screenshot=shot, ) - _log.debug("command: %s", driver_command) + # No per-command line here: the Actions timeline lists every command as + # it happens, and `_WATCH` puts this logger's debug records in the same + # Console the user is reading, so it was one duplicate line per command. # Keep the snapshot iframe current after every command (a click can # navigate too, not just get/back/…), re-injecting if the page changed. if entry is not None: diff --git a/packages/selenium-devtools-py/tests/test_bidi.py b/packages/selenium-devtools-py/tests/test_bidi.py index f28275bb..db7e271f 100644 --- a/packages/selenium-devtools-py/tests/test_bidi.py +++ b/packages/selenium-devtools-py/tests/test_bidi.py @@ -748,6 +748,63 @@ def test_the_regenerated_layer_selects_the_event_handler_path(self): self.assertNotIn("devtools_before_request_sent", module.Network.EVENT_CONFIGS) +class TestTheTeardownSummary(unittest.TestCase): + """One line at the end instead of one per request. + + `_WATCH` raises this package's logger to DEBUG so its records reach the + dashboard Console, which means a per-event debug line is a Console line per + request — beside a Network tab already listing every one of them.""" + + def test_a_clean_run_reports_only_the_count(self): + self.assertEqual( + bidi.network_summary({"captured": 13, "pending": {}}), + "network: 13 request(s) captured", + ) + + def test_requests_without_a_response_are_called_out(self): + # The count is the cheap half. This is the half the Network tab cannot + # show: a request whose response never arrived. + summary = bidi.network_summary({"captured": 5, "pending": {"R7": {}, "R8": {}}}) + + self.assertIn("5 request(s) captured", summary) + self.assertIn("2 still awaiting a response", summary) + + def test_nothing_captured_and_nothing_pending_says_nothing(self): + # A session that made no requests should not add a line to the Console. + self.assertIsNone(bidi.network_summary({"captured": 0, "pending": {}})) + self.assertIsNone(bidi.network_summary({})) + self.assertIsNone(bidi.network_summary(None)) + + def test_the_counts_come_from_real_capture(self): + module = fake_network_module() + network = module.Network() + stats = {} + + with mock.patch.dict( + sys.modules, {"selenium.webdriver.common.bidi.network": module} + ): + bidi._attach_network( + NewSeleniumDriver(network, []), SessionCapturer(FakeTransport()), stats + ) + + for request_id in ("R1", "R2"): + TestTheEventManagerPath._dispatch( + network, "network.beforeRequestSent", + {"request": {"request": request_id, "url": "https://x/a", "method": "GET"}, + "timestamp": 1000}, + ) + # Only one of the two answers. + TestTheEventManagerPath._dispatch( + network, "network.responseCompleted", + {"request": {"request": "R1"}, "timestamp": 1200, + "response": {"status": 200}}, + ) + + self.assertEqual(bidi.network_summary(stats), + "network: 1 request(s) captured, 1 still awaiting a response " + "at teardown") + + class TestEventParams(unittest.TestCase): def test_a_raw_dict_is_its_own_params(self): self.assertEqual(bidi.event_params({"request": {}}), {"request": {}}) diff --git a/packages/selenium-devtools-py/tests/test_instrumentation.py b/packages/selenium-devtools-py/tests/test_instrumentation.py index df503ed3..e820bc31 100644 --- a/packages/selenium-devtools-py/tests/test_instrumentation.py +++ b/packages/selenium-devtools-py/tests/test_instrumentation.py @@ -476,7 +476,11 @@ def setUp(self): self.attached = [] self._bidi = mock.patch.object( instrumentation.bidi, "attach", - side_effect=lambda d, c: self.attached.append(d.session_id) or True, + # `stats` is the bag attach fills for the teardown summary; accepted + # here so the stub keeps matching the real signature. + side_effect=lambda d, c, stats=None: ( + self.attached.append(d.session_id) or True + ), ) self._bidi.start() instrumentation.install(self.cap, MultiSessionDriver) From 47a6df98370bae07649e151e1c6aafd7fafdcf11 Mon Sep 17 00:00:00 2001 From: Vishnu Vardhan Date: Wed, 19 Aug 2026 18:33:00 +0530 Subject: [PATCH 12/12] fix(selenium-devtools-py): unwind the registration that was in flight --- .../src/selenium_devtools/bidi.py | 18 +++++----- .../selenium-devtools-py/tests/test_bidi.py | 35 +++++++++++++++++++ 2 files changed, 45 insertions(+), 8 deletions(-) diff --git a/packages/selenium-devtools-py/src/selenium_devtools/bidi.py b/packages/selenium-devtools-py/src/selenium_devtools/bidi.py index 4490d759..8ff550fc 100644 --- a/packages/selenium-devtools-py/src/selenium_devtools/bidi.py +++ b/packages/selenium-devtools-py/src/selenium_devtools/bidi.py @@ -430,8 +430,11 @@ def supports_event_handler_api() -> bool: return hasattr(Network, "add_event_handler") -def _add_raw_event_handler(network: Any, bidi_event: str, callback: Any) -> None: - """Subscribe ``callback`` to ``bidi_event`` with the RAW params. +def _add_raw_event_handler( + network: Any, bidi_event: str, callback: Any, registered: List[Any] +) -> None: + """Subscribe ``callback`` to ``bidi_event`` with the RAW params, appending + what it installed to ``registered`` so a failed attach can undo it. This is selenium's own ``add_event_handler`` body with one substitution: it looks its deserializer up in a per-event map shared by every subscriber, and @@ -449,14 +452,16 @@ def _add_raw_event_handler(network: Any, bidi_event: str, callback: Any) -> None manager = network._event_manager wrapper = _RawEvent(bidi_event) callback_id = manager.conn.add_callback(wrapper, callback) + # Recorded the moment the callback exists, BEFORE the two steps that can + # raise. Recording it after them instead leaves the one registration in + # flight invisible to the unwind — precisely the one that failed. + registered.append((bidi_event, wrapper, callback_id)) manager.subscribe_to_event(bidi_event) # Selenium counts callbacks per event to decide when a subscription is no # longer needed. Ours is registered on the connection directly, so without # this it is invisible to that count and another consumer removing their # handler would unsubscribe the event out from under us. manager.add_callback_to_tracking(bidi_event, callback_id) - # Returned so a failed attach can undo exactly what it did. - return wrapper, callback_id def _undo_registration( @@ -608,10 +613,7 @@ def _subscribe_via_event_manager( network = driver.network manager = network._event_manager for bidi_event, callback in handlers.items(): - wrapper, callback_id = _add_raw_event_handler( - network, bidi_event, callback - ) - registered.append((bidi_event, wrapper, callback_id)) + _add_raw_event_handler(network, bidi_event, callback, registered) except Exception as exc: # noqa: BLE001 _warn(f"network subscribe failed, no network events captured: {exc}") for bidi_event, wrapper, callback_id in registered: diff --git a/packages/selenium-devtools-py/tests/test_bidi.py b/packages/selenium-devtools-py/tests/test_bidi.py index db7e271f..bf152ee1 100644 --- a/packages/selenium-devtools-py/tests/test_bidi.py +++ b/packages/selenium-devtools-py/tests/test_bidi.py @@ -689,6 +689,41 @@ def failing_subscribe(bidi_event, contexts=None): frames = [s for s, _ in capturer._tx.sent if s == "networkRequests"] self.assertEqual(frames, []) + def test_a_failure_mid_registration_unwinds_the_in_flight_callback(self): + """The callback is installed before the steps that can raise, so the + registration that FAILED is the one most easily missed. + + Recording it only after subscribing leaves it on the connection with + nothing referencing it, and a session that re-attaches accumulates one + more on every failure. + """ + module = fake_network_module() + network = module.Network() + manager = network._event_manager + + # Fails on the FIRST event, i.e. part-way through its own registration + # rather than after an earlier one completed. + def failing_subscribe(bidi_event, contexts=None): + raise RuntimeError("websocket went away") + + manager.subscribe_to_event = failing_subscribe + + with mock.patch.dict( + sys.modules, {"selenium.webdriver.common.bidi.network": module} + ): + with self.assertLogs("selenium_devtools.bidi", level="WARNING"): + attached = bidi._attach_network( + NewSeleniumDriver(network, []), SessionCapturer(FakeTransport()) + ) + + self.assertFalse(attached) + self.assertEqual( + [entries for entries in manager.conn.callbacks.values() if entries], [] + ) + # Never tracked, because add_callback_to_tracking is after the failure — + # so the unwind's own steps must tolerate having nothing to remove. + self.assertEqual(manager.tracked, []) + def test_an_unwind_never_takes_down_a_live_subscription(self): """`unsubscribe_from_event` only fires with no callbacks left, so another consumer already listening to the same event keeps theirs."""