Skip to content

[py] Align driver.network handlers with ADR 17685 - #18051

Open
AutomatedTester wants to merge 1 commit into
trunkfrom
copse/look-at-18019-and-create-a-plan-on-what-we-7b5007
Open

AutomatedTester wants to merge 1 commit into
trunkfrom
copse/look-at-18019-and-create-a-plan-on-what-we-7b5007

Conversation

@AutomatedTester

@AutomatedTester AutomatedTester commented Sep 18, 2026 •

Copy link
Copy Markdown
Member

🔗 Related Issues

Part of #18019 — this is the Python row only.

⚠️ #18019 must stay open after this merges — it is the cross-binding tracking issue covering the Java, Ruby, .NET and JavaScript rows, which are still outstanding. Maintainers: please unlink it from this PR's Development sidebar before merging if GitHub has attached it.

Implements ADR 17685 — network handler behavior (Accepted) for Python.

💥 What does this PR do?

Brings driver.network in line with the eleven decisions in the ADR, across all three handler families (request, response, authentication).

Decision Change
1 add/remove/clear for each family. add returns a HandlerHandle. Adds add_authentication(username, password).
2 URL patterns are validated locally and evaluated by the browser. Selenium no longer expands globs or matches URLs itself.
3, 4 Dispositions fail / respond / submit; settling twice within one handler raises AlreadySettledError.
5, 6 Last-registered-first (LIFO); the first handler to settle resolves the event and stops the chain; an unsettled event proceeds with whatever was staged.
7 An uncaught handler exception fails the event and is re-raised from the next call into driver.network.
9 request.original / response.original expose the event as it arrived, read-only.
10 Opt-in body collection via collect_body=True at registration; Selenium owns the collector's lifecycle and size cap.
11 Handlers scope to a window_handle or a user_context, never both; passing both raises.

Previously handlers were FIFO with priority-based reconciliation, and the response family had no fail.

🔧 Implementation Notes

Backwards compatibility. The ADR's Consequences say the change "is not backwards compatible", but AGENTS.md and CONTRIBUTING.md require deprecate-then-remove, so I kept the old surfaces working:

  • No public method was removed. add_auth_handler / remove_auth_handler still work and now emit DeprecationWarning pointing at add_authentication.
  • The phase-based add_request_handler("before_request", cb) form still works and warns.
  • Every new argument is keyword-only after *, so positional order is unchanged on all three add_* methods.
  • HandlerHandle subclasses str, so decision 1's "handle object" stays usable everywhere the plain string IDs were.
  • Network(conn, driver=None) keeps the new driver argument optional.

I verified this by diffing the generated public surface against trunk rather than by inspection.

The one genuine break is decision 2: a caller passing a glob like "**/api/**" previously got client-side matching. Rather than fail obscurely at the browser, _url_pattern_from_string detects */? and logs a warning explaining that finer matching now belongs inside the handler. This is the item I'd most like a second opinion on — a warning-only migration for a behavior change is a judgment call, and the TLC may prefer a full deprecation cycle.

Two bugs the browser tests caught, both fixed here:

  1. Error-reporting race. Failing an event unblocks the browser, which lets the user's navigation raise and call back into driver.network. Recording the handler's exception after failing raced that very call, so decision 7's "surfaces to the user" was non-deterministic. Now recorded before failing. The regression test fails if the two lines are swapped back — I checked.
  2. Bodyless requests stalled the page. While a request is blocked, network.getData for a body that will never exist never answers at all — it does not error, it hangs, so the request stays blocked until the command times out. Any collect_body=True handler seeing a GET hit this. Now guarded on bodySize.

On (2), the unit fixture had been hiding the problem: it modelled a GET with no bodySize while asserting body collection succeeded. make_before_request_event now takes method and body_size. Confirmed against a real browser that a POST body reads correctly inside a blocked handler.

🤖 AI assistance

  • No substantial AI assistance used
  • AI assisted (complete below)
    • Tool(s): Claude Code (Opus 5), via Copse
    • What was generated: the handler-registry rewrite in py/private/_network_handlers.py, the generator manifest entries in py/private/bidi_enhancements_manifest.py, and the unit and browser tests. Design decisions come from ADR 17685; the two bugs above were found by running the browser suites and diagnosed by probing a real browser rather than by assumption.
    • I reviewed all AI output and can explain the change

💡 Additional Considerations

Needs a decision before merge:

  1. Is the warning-only migration for glob patterns (decision 2) acceptable, or should globs keep working for a deprecation cycle?
  2. Decision 11 scoping interacts with network.addIntercept, which cannot scope to a user context. This PR intercepts everything and continues out-of-scope events untouched. Worth confirming that is the intended reading.

Follow-up work, not in this PR:

  • Java, Ruby, .NET and JavaScript rows of #18019. Java is the largest rewrite and its scoping work depends on #17925; JS should track #17973.
  • A shared cross-binding conformance matrix keyed to the eleven decisions, so five bindings do not re-derive five APIs.
  • Documentation on seleniumhq.github.io for the new handler families.

🔄 Types of changes

  • New feature (non-breaking change which adds functionality and tests!)
  • Bug fix (backwards compatible)
  • Breaking change — limited to glob URL patterns no longer being expanded client-side (decision 2); see Implementation Notes

Testing

Suite Result
//py:test/unit/selenium/webdriver/common/bidi_network_tests-unit 95 passed
//py:unit 32/32 targets pass
network_tests-chrome-bidi 42 passed, 1 skipped
network_tests-firefox-bidi 41 passed, 2 xfailed
ruff check / ruff format --check clean

The Chrome skip and both Firefox xfails are pre-existing, documented browser limitations, not introduced here. Note that py/test/selenium/webdriver/common/bidi/network_tests.py sits under the **/bidi/** path that scripts/format.sh excludes from ruff, so I linted and formatted it explicitly.

Co-Authored-By: Copse noreply@copse.dev
Copse-Models: acp:claude-agent-acp#opus[1m]

@selenium-ci selenium-ci added C-py Python Bindings B-devtools Includes everything BiDi or Chrome DevTools related labels Sep 18, 2026
@AutomatedTester
AutomatedTester force-pushed the copse/look-at-18019-and-create-a-plan-on-what-we-7b5007 branch from 13cc000 to 1c500a6 Compare September 28, 2026 09:14
Implements the network handler behavior ADR for the Python bindings,
covering the request, response and authentication handler families.

- add/remove/clear for each family; add returns a HandlerHandle
- URL patterns are validated locally and evaluated by the browser;
  Selenium no longer expands globs or matches URLs itself
- handlers are consulted last-registered first, and the first one to
  settle a disposition resolves the event and stops the chain
- an uncaught handler exception fails the event and is re-raised from
  the next call into driver.network
- opt-in request body collection, with Selenium owning the collector's
  lifecycle and size cap
- handlers are scoped to a window handle or a user context, never both

add_auth_handler, remove_auth_handler and the phase-based
add_request_handler(event, callback) form still work and now warn. No
public method was removed, and every new argument is keyword-only, so
existing calls are unaffected.
@AutomatedTester
AutomatedTester force-pushed the copse/look-at-18019-and-create-a-plan-on-what-we-7b5007 branch from 1c500a6 to b57eb6e Compare September 28, 2026 14:02

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

B-devtools Includes everything BiDi or Chrome DevTools related C-py Python Bindings

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants