[adr] network handler behavior proposal

[titusfortner]

📄 The decision, its rationale, considered options, and consequences are in the record file
this PR adds (docs/decisions/17685-network-handler-behavior.md);
read it there. The sections below are proposal notes and review logistics.

🔗 Related

• Counter-proposal to @p0deje's BiDi Protocol Design Document. Last Fall @nvborisenko and I raised concerns and improvements on that doc; meetings stopped before anything was resolved, so this record settles the behaviors instead.
• #17671 — proposes observation as a separate API; this record folds it in as a handler mode instead (see Proposal notes).
• Reference implementation: the behaviors are sketched end-to-end in Ruby (see Proposal notes).

📝 Proposal notes

• One record, deliberately bundled. Independent adoptability is not grounds to split (per the ADR scoping guidance). Observe-vs-intercept and body collection could each be their own record. Observe/intercept is here because elsewhere — Playwright's separate on/route, and [docs] decision: BiDi events are awaited with expect_* context managers #17671 — observation and interception are treated as mutually exclusive, requiring separate APIs; a separate record would concede that framing, which is what's contested. Body collection is here because it's part of the same handler registration, not a separable concern.
• Deliberately out of scope: the shape of authentication handlers (auth should not use a callable), and a complete/capture primitive for saving an event for later assertion (considered but deferred — not worth adding right now).
• Why settle this early: the behaviors are not backwards compatible. The bindings already diverge (see the Context table in the record), and every binding that ships more of its handler API first is more to unwind later.
• Implementation is intentionally unspecified. Bindings may satisfy these behaviors however fits the language. For illustration only, this Ruby sketch — state stored on the request wrapper and evaluated after each handler runs — satisfies all of them:

def process_request(request)
  @handlers.reverse_each do |h|
    h.call(request)
    if request.failed?
      return fail_request(request)
    elsif request.response?
      return provide_response(request)
    elsif request.submit?
      return continue_request(request)
    end
  end
  continue_request(request)
end

🗣 Discussion

For reviewers and the TLC (input wanted, not a defect):

• Observe vs. intercept as one API. Treating observation and interception as two modes of one handler — observation non-blocking — is the open design question between this record and [docs] decision: BiDi events are awaited with expect_* context managers #17671; flagging it for the TLC to land.
• TLC meeting outcome: (to be recorded after discussion)

📌 Tracking

Tracking issue: (linked on acceptance)

[qodo-code-review]

PR Summary by Qodo

ADR: Define network handler disposition, ordering, and failure semantics
📝 Documentation 🕐 20-40 Minutes

Description

• Proposes user-facing behaviors for resolving conflicting network handlers.
• Defines handler disposition, default chaining, LIFO ordering, and exception handling.
• Specifies completion capture and body collection responsibilities for handlers.

Diagram

graph TD
U(["User/Test"]) --> API["Network API"] --> HS["Handler stack"] --> HC["Handler callable"] --> D{"Disposition?"}
D --> F["Fail request"] --> B["BiDi command"]
D --> R["Provide response"] --> B
D --> S["Continue/Submit"] --> B
HC --> E["Log & skip"]
HC --> C[("Completed event")]

subgraph Legend
  direction LR
  _actor(["Actor"]) ~~~ _proc["Process"] ~~~ _dec{"Decision"} ~~~ _store[("Stored state")]
end

Loading

High-Level Assessment

The following are alternative approaches to this PR:

1. Require explicit disposition (Playwright-style fallback)

• ➕ Avoids ambiguity: every handler must declare whether it stops or continues processing
• ➕ Reduces surprises when handlers accidentally “chain” due to omitted disposition
• ➖ More verbose for common “mutate and continue” cases (headers, URL rewrites, etc.)
• ➖ Makes composition of multiple small handlers more cumbersome

2. Run all handlers then reconcile by fixed priority

• ➕ Deterministic resolution even if multiple handlers set conflicting dispositions
• ➕ Potentially simpler mental model for some users (priority table)
• ➖ Prevents a specific handler from intentionally short-circuiting others
• ➖ Can unintentionally apply mutations from handlers that “should not have run” under a user-chosen stop condition

3. FIFO ordering (first registered wins)

• ➕ Matches many event-listener models where registration order implies precedence
• ➕ Potentially easier to reason about for simple setups
• ➖ Makes local overrides difficult (tests can’t reliably override shared/global handlers)
• ➖ Encourages brittle global configuration and test coupling

Recommendation: The ADR’s proposed model (explicit disposition when needed, default chaining, LIFO precedence, and exception isolation) is the most composable for real test suites where local overrides must trump shared defaults. The main point to validate with stakeholders is whether the default “continue to next handler” is acceptable ergonomically and safety-wise; if not, consider the explicit-fallback alternative, but expect materially more boilerplate across bindings.

Files changed (1) +230 / -0

Documentation (1) +230 / -0

network-handler-behavior.mdAdd ADR defining network handler behavior and precedence rules +230/-0

Add ADR defining network handler behavior and precedence rules

• Introduces a proposed ADR that specifies how request/response handlers compose and resolve conflicts. Defines disposition verbs, default chaining behavior, LIFO ordering, exception handling, ignoring return values, access to original vs mutated event state, completion capture, and body-collection responsibilities.

docs/decisions/network-handler-behavior.md

[qodo-code-review]

Code Review by Qodo

🐞 Bugs (7) 📘 Rule violations (1) 📜 Skill insights (0)

Context used

✅ Compliance rules (platform): 11 rules

1. Java order nondeterministic 🐞 Bug ≡ Correctness

Description

The ADR table says Java runs “only the first matching handler”, but the current Java implementation
selects a matching handler from a ConcurrentHashMap stream with no defined encounter order, so the
chosen handler is effectively arbitrary and can vary between runs. This can mislead
readers/implementers about Java’s current precedence semantics.

Code

docs/decisions/17685-network-handler-behavior.md[18]

+| Java       | Only the first matching handler runs; disposition is always continue; a throwing handler propagates and leaves the request blocked; return-value driven; no response handler or managed body collection. |

Evidence

The ADR claims a “first” handler semantics for Java, but Java stores handlers in a ConcurrentHashMap
and selects via values().stream()...findFirst(), which does not guarantee ordering; therefore the
selected handler is not consistently the first-registered or otherwise deterministic handler.

docs/decisions/17685-network-handler-behavior.md[16-22]
java/src/org/openqa/selenium/remote/RemoteNetwork.java[47-50]
java/src/org/openqa/selenium/remote/RemoteNetwork.java[143-148]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
The ADR’s “Current behavior” row for Java states “Only the first matching handler runs”, which implies a deterministic ordering. In the current Java implementation, handler selection is performed by streaming `ConcurrentHashMap.values()` and taking `findFirst()`, which has no guaranteed iteration/encounter order.

### Issue Context
This is in the ADR’s binding comparison table; the goal is to accurately describe current behavior so readers understand what is changing.

### Fix Focus Areas
- docs/decisions/17685-network-handler-behavior.md[16-22]

### Suggested change
Reword the Java row to avoid implying deterministic precedence (e.g., “Only one matching handler runs (selection order is unspecified) ...”), or explicitly state the current selection is not ordered by registration.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

2. No cross-binding source references 📘 Rule violation ≡ Correctness

Description

The ADR describes binding differences and proposes user-visible handler behavior changes but does
not provide any concrete cross-binding source references (e.g., rg results or links to the
Java/Python/Ruby/.NET/JS implementations) demonstrating the comparison was actually performed. This
risks inconsistent implementation across bindings because reviewers/implementers cannot verify the
stated current behaviors or the proposed alignment points.

Code

docs/decisions/17685-network-handler-behavior.md[R13-22]

+The bindings diverge today: each grew its handler API independently, so dispatch order,
+multi-handler resolution, error handling, and what an event exposes are all inconsistent.
+
+| Binding    | Current behavior |
+|------------|------------------|
+| Java       | Only the first matching handler runs; disposition is always continue; a throwing handler propagates and leaves the request blocked; return-value driven; no response handler or managed body collection. |
+| Python     | An explicit `continue` in a handler fires immediately and wins; otherwise staged outcomes reconcile by `fail` > `provide_response` > `continue`; response handlers have no `fail`; dispatch is FIFO; a throwing handler's staged mutations are still sent; only the mutated event is visible; body is not collected behind the handler. |
+| Ruby       | Handlers run in parallel threads, so multi-handler disposition races; exceptions are logged; dispatch is FIFO with no default-continue; only the mutated event is visible; body collection is user-managed. |
+| .NET       | No request or response handler API. |
+| JavaScript | No request or response handler API. |

Evidence

PR Compliance ID 389265 requires evidence of cross-binding comparison (e.g., project-wide search
output or references to compared files). The ADR includes a cross-binding behavior table but
provides no file references or search evidence to substantiate the claims, making the required
comparison non-auditable.

Rule 389265: Compare cross-language bindings when changing user-visible behavior
docs/decisions/17685-network-handler-behavior.md[13-22]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The ADR asserts current cross-binding behaviors and proposes new user-visible behavior, but it lacks verifiable evidence of cross-binding comparison (project-wide search output and/or direct links to the analogous implementations in other bindings).

## Issue Context
Compliance requires that behavior-changing proposals include evidence of cross-language comparison so implementers can validate baseline behavior and keep bindings logically consistent.

## Fix Focus Areas
- docs/decisions/17685-network-handler-behavior.md[13-22]

## Suggested remediation
- Add a short section (e.g., "Cross-binding references") that lists at least one concrete file/link per compared binding (Java/Python/Ruby at minimum) that supports the "Current behavior" table entries.
- Optionally include a small snippet of the exact `rg ...` commands used (and the key hits) to demonstrate the repo-wide search comparison was performed.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

3. Auth callable contradiction 🐞 Bug ≡ Correctness

Description

The ADR excludes authentication handlers because “authentication should not use a callable,” but
Selenium Ruby’s add_authentication_handler explicitly accepts a block and existing tests use
block-based auth handlers (&:skip, &:cancel). This makes the ADR’s rationale inaccurate for at
least one binding and risks confusing implementers about the scope/baseline.

Code

docs/decisions/17685-network-handler-behavior.md[R20-21]

+This applies to request and response handlers, but not authentication handlers, since
+authentication should not use a callable.

Evidence

Ruby’s Network API accepts an optional block for add_authentication_handler and uses it when
credentials aren’t provided, and Ruby integration tests exercise callable auth handlers via &:skip
and &:cancel, contradicting the ADR’s claim that authentication should not use a callable.

rb/lib/selenium/webdriver/common/network.rb[47-63]
rb/spec/integration/selenium/webdriver/network_spec.rb[86-101]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The ADR states authentication handlers are out of scope because authentication should not use a callable. In current Selenium Ruby, authentication handlers can be provided as a callable/block, so the statement is not universally true across bindings.

## Issue Context
The ADR is meant to be cross-binding guidance; blanket statements that contradict an existing binding’s public API are likely to cause confusion.

## Fix Focus Areas
- docs/decisions/17685-network-handler-behavior.md[20-21]

## Suggested change
Reword to scope the statement as a proposal or explicitly note that auth handler behavior is out of scope and that existing bindings differ (e.g., “This ADR does not cover authentication handlers; bindings currently vary, and we may revisit whether auth should be callable-based separately.”).

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

View more (4)

4. Wrong Python behavior claim 🐞 Bug ≡ Correctness

Description

The ADR claims current Python behavior lets continueRequest override failures and stubs, but
Python’s BiDi network handler registry resolves outcomes with fixed precedence where any fail()
wins and provide_response() wins over mutations. This misstates the existing semantics and can
mislead readers about what is actually changing.

Code

docs/decisions/17685-network-handler-behavior.md[R158-159]

+  - We could run every handler but have `continueRequest` override failures and stubs (current
+    Python behavior), but it is not obvious why that command should have precedence.

Evidence

Python’s handler registry documents and implements reconciliation where any handler calling fail()
causes the request to be failed during _resolve(), and the integration tests assert that fail
wins when handlers disagree—so continueRequest does not override failures/stubs in current Python
behavior.

py/private/_network_handlers.py[28-37]
py/private/_network_handlers.py[428-437]
py/test/selenium/webdriver/common/bidi_network_tests.py[251-266]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The ADR’s “Considered options → Reconciliation” section states that in current Python behavior `continueRequest` overrides failures and stubs. The Python implementation instead reconciles handler actions with a strict priority (fail > provide_response > continue-with-mutations/default continue), so the ADR’s comparison baseline is inaccurate.

## Issue Context
This ADR is intended to guide cross-binding behavior decisions; incorrect statements about existing behavior can skew review/consensus and lead to incorrect implementations.

## Fix Focus Areas
- docs/decisions/17685-network-handler-behavior.md[154-160]

## Suggested change
Reword the parenthetical to reflect the actual Python semantics (e.g., “current Python behavior reconciles outcomes with fixed priority (fail > provide_response > continue) after running all matching handlers”).

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

5. Overgeneralized exception behavior 🐞 Bug ⚙ Maintainability

Description

The ADR states (in a comparative bullet) that Selenium logs uncaught handler exceptions instead of
ending the session, but current behavior is binding-specific (e.g., Ruby handler blocks are executed
without rescue/logging, while Python does catch/log). This should be reworded as a proposed
cross-binding behavior or explicitly scoped to the bindings that implement it today.

Code

docs/decisions/network-handler-behavior.md[R77-80]

+   * In Playwright, uncaught exceptions propagate to end the session, which causes problems when
+     something unrelated to the test's intent goes wrong.
+   * Selenium is more lenient and only logs the error to the console.
+

Evidence

Ruby currently invokes handler blocks without rescue/logging, while Python explicitly catches and
logs exceptions, so the ADR’s statement is not uniformly true across Selenium bindings today.

docs/decisions/network-handler-behavior.md[77-80]
rb/lib/selenium/webdriver/common/network.rb[89-96]
py/private/_network_handlers.py[746-756]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
The ADR text implies a universal Selenium behavior (“only logs the error to the console”) for handler exceptions, but current bindings differ.

### Issue Context
Ruby’s handler dispatch yields directly to user code without rescue; Python catches/logs and continues.

### Fix Focus Areas
- docs/decisions/network-handler-behavior.md[72-80]

### What to change
- Rephrase the bullet to one of:
 - “In some Selenium bindings (e.g., Python), uncaught handler exceptions are logged and processing continues…”
 - or “Proposed: Selenium should log uncaught handler exceptions and continue processing…”
- If desired, add a short note indicating current binding differences to avoid readers assuming uniform behavior today.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

6. Wrong handler verb names 🐞 Bug ≡ Correctness

Description

The ADR states Selenium request/response handlers use respond/submit (and response fail), but
existing bindings expose different verbs (e.g., Python provide_response/continue_request, Ruby
provide_response/continue) and do not offer a response-level fail. This mismatch will mislead
implementers/readers about current APIs and what exactly is being proposed to change.

Code

docs/decisions/network-handler-behavior.md[R31-43]

+   * Selenium supports:
+     * Request: `fail` (Playwright's `abort`, BiDi's `FailRequest`), `respond` (Playwright's `fulfill`, BiDi's `ProvideResponse`), and `submit` (Playwright's `continue`, BiDi's `ContinueRequest`).
+     * Response: `fail` (BiDi's `FailRequest`), and `submit`: note that since we don't need to prevent a round trip from a request, whether this is a BiDi `ContinueResponse` or `ProvideResponse` can be an implementation detail based on whether a replacement body value is provided.
+
+```ruby
+# Specifics of parameters and names can match spec details
+network.add_request_handler { |r| r.fail if something }
+network.add_request_handler { |r| r.respond(content: mocked_response) if something }
+network.add_request_handler { |r| r.add_header("X-Test", true) && r.submit if something }
+
+network.add_response_handler { |r| r.fail if something }
+network.add_response_handler { |r| r.submit(content: mocked_response) if something }
+network.add_response_handler { |r| r.add_header("X-Test", true) && r.submit if something }

Evidence

The ADR’s listed verbs don’t exist in current binding implementations, which use different method
names and capabilities for request/response interception.

docs/decisions/network-handler-behavior.md[31-43]
py/private/_network_handlers.py[325-438]
py/private/_network_handlers.py[440-581]
rb/lib/selenium/webdriver/bidi/network/intercepted_request.rb[30-58]
rb/lib/selenium/webdriver/bidi/network/intercepted_response.rb[31-68]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
The ADR uses handler disposition verbs (`respond`, `submit`, and response `fail`) that don’t match the verbs exposed by current Selenium bindings, which makes the document confusing as a proposal baseline.

### Issue Context
Current Python and Ruby bindings use `provide_response`/`continue_request` (Python) and `provide_response`/`continue` (Ruby), and do not expose a response-level `fail`.

### Fix Focus Areas
- docs/decisions/network-handler-behavior.md[26-44]

### What to change
- Replace “Selenium supports:” wording with either:
 - a binding-neutral set of verbs (and explicitly state they are *proposed* names), or
 - the currently used names (and optionally add a mapping table: proposed ↔ existing per binding).
- Update the Ruby/Python code examples accordingly (e.g., `provide_response`/`continue_request`/`continue`) and remove or clearly label response `fail` as a proposed addition if that’s intended.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

7. Example violates stated behavior 🐞 Bug ≡ Correctness

Description

The Appendix claims the sample process_request implementation satisfies the ADR behaviors, but it
does not handle handler exceptions (behavior 4) and would propagate exceptions from
h.call(request) instead of logging and continuing. As written, it also cannot discard per-handler
staged mutations on exception because it mutates a shared request object.

Code

docs/decisions/network-handler-behavior.md[R208-226]

+The behaviors in this ADR explicitly do not specify an implementation. For illustrative
+purposes, this code — with state stored in the request wrapper object and evaluated after
+execution inside the loop — will satisfy the above behaviors:
+
+```ruby
+def process_request(request)
+  @handlers.reverse_each do |h|
+    h.call(request)
+    if request.complete?
+      h.request = request
+      remove_handler(h)
+    end
+    if request.failed?
+      return fail_request(request)
+    elsif request.response?
+      return provide_response(request)
+    elsif request.submit? || request.complete?
+      return continue_request(request)
+    end

Evidence

The ADR requires log-and-continue on handler exceptions, but the appendix code calls handlers
without any rescue/try-catch while still claiming it satisfies the behaviors.

docs/decisions/network-handler-behavior.md[72-79]
docs/decisions/network-handler-behavior.md[208-226]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
The appendix implementation is presented as satisfying the ADR behaviors, but it lacks exception handling around handler invocation and does not show how staged mutations are discarded when a handler errors.

### Issue Context
Behavior 4 requires uncaught exceptions to be logged, staged changes discarded, and processing to continue as if the handler was not registered.

### Fix Focus Areas
- docs/decisions/network-handler-behavior.md[72-85]
- docs/decisions/network-handler-behavior.md[208-229]

### What to change
- Wrap `h.call(request)` in a begin/rescue (or language-appropriate equivalent) and explicitly note logging + continuing.
- Either:
 - update the pseudocode to show per-handler isolation/rollback of staged mutations (e.g., snapshot/clone before calling handler and restore on exception), or
 - amend the text to state the pseudocode is simplified and *does not* model rollback semantics (so it’s not claimed to fully satisfy behavior 4).

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

8. Exception propagation unclear 🐞 Bug ⚙ Maintainability ⭐ New

Description

Behavior 5 says intercept-handler exceptions “propagate” while also requiring the event to “keep
flowing as if that handler had not run,” but it doesn’t explicitly state how propagation should be
surfaced without interrupting dispatch/request resolution. This ambiguity can lead to inconsistent
cross-binding implementations of the same proposed behavior.

Code

docs/decisions/17685-network-handler-behavior.md[R104-106]

+5. **An uncaught exception discards the handler's staged changes; it propagates for an intercept
+   handler and is logged for an observe handler.** Either way the event keeps flowing as if that
+   handler had not run, so one broken handler cannot corrupt live traffic or stall the page. The

Evidence

The ADR simultaneously requires exception “propagation” for intercept handlers and continued event
flow, and the provided example implies remaining handlers still take effect; existing handler
dispatch code in-repo shows continued flow is typically implemented by catching exceptions and
continuing, which highlights why the ADR should be explicit about how “propagate” is achieved
without interrupting dispatch.

docs/decisions/17685-network-handler-behavior.md[104-106]
docs/decisions/17685-network-handler-behavior.md[114-120]
py/private/_network_handlers.py[746-766]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
Behavior 5 mixes two distinct requirements (error visibility and continued network flow) without stating the intended mechanism for “propagate” in intercept mode. Readers may interpret “propagate” as letting the exception abort handler dispatch, which conflicts with the “event keeps flowing” requirement.

## Issue Context
The ADR’s example text also implies that other handlers’ effects still apply even when one handler errors.

## Fix Focus Areas
- docs/decisions/17685-network-handler-behavior.md[104-111]
- docs/decisions/17685-network-handler-behavior.md[114-120]

## Suggested direction
Adjust behavior 5 wording to explicitly separate:
1) **Network disposition guarantee:** Selenium must still resolve/unblock the event as if the failing handler did not run (discarding its staged mutations).
2) **Error visibility guarantee:** For intercept handlers, the error is surfaced to the user (e.g., recorded and raised at a defined observation point / next driver call), while dispatch continues; for observe handlers, it is logged.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

9. Markdown nesting broken ✓ Resolved 🐞 Bug ⚙ Maintainability

Description

Under decision #7, the first bullet line is indented less than the preceding continuation lines of
the same list item, which can cause CommonMark/GitHub rendering to treat it as a separate top-level
list instead of nested under item 7. This makes the ADR harder to read and can change the visual
structure of the decision.

Code

docs/decisions/17685-network-handler-behavior.md[R129-134]

+7. **Body data is collected only when the handler opts in at registration.** A body is not
+   available by default; the handler declares that it needs the body when it is registered — not
+   from inside the callback, since the collector must be in place before the event — and Selenium
+   then owns the collector's lifecycle, size cap, and browser-support quirks. The body is readable
+   on the event inside that handler.
+  * The user never calls `addDataCollector` / `getData` or tears a collector down.

Evidence

Within list item 7, the continuation lines are indented deeper than the bullet that follows,
indicating inconsistent nesting that can alter Markdown rendering.

docs/decisions/17685-network-handler-behavior.md[129-134]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
The nested bullet list under item 7 is not consistently indented, which can break Markdown nesting and render incorrectly.

### Issue Context
Under ordered list item `7.`, the paragraph continuation lines are indented more than the subsequent `*` bullet line, so the bullets may not be considered part of item 7.

### Fix Focus Areas
- docs/decisions/17685-network-handler-behavior.md[129-134]

### Suggested change
Indent the `*` bullet lines to the same level as the continuation lines for item 7 (typically 3–4 spaces after the margin for CommonMark nested content under an ordered list item).

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

Previous review results

Review updated until commit 895d532

Results up to commit b09a2bc

🐞 Bugs (3) 📘 Rule violations (0) 📎 Requirement gaps (0) 🎨 UX issues (0) 🔗 Cross-repo conflicts (0) 📜 Skill insights (0)

1. Example violates stated behavior 🐞 Bug ≡ Correctness

Description

The Appendix claims the sample process_request implementation satisfies the ADR behaviors, but it
does not handle handler exceptions (behavior 4) and would propagate exceptions from
h.call(request) instead of logging and continuing. As written, it also cannot discard per-handler
staged mutations on exception because it mutates a shared request object.

Code

docs/decisions/network-handler-behavior.md[R208-226]

+The behaviors in this ADR explicitly do not specify an implementation. For illustrative
+purposes, this code — with state stored in the request wrapper object and evaluated after
+execution inside the loop — will satisfy the above behaviors:
+
+```ruby
+def process_request(request)
+  @handlers.reverse_each do |h|
+    h.call(request)
+    if request.complete?
+      h.request = request
+      remove_handler(h)
+    end
+    if request.failed?
+      return fail_request(request)
+    elsif request.response?
+      return provide_response(request)
+    elsif request.submit? || request.complete?
+      return continue_request(request)
+    end

Evidence

The ADR requires log-and-continue on handler exceptions, but the appendix code calls handlers
without any rescue/try-catch while still claiming it satisfies the behaviors.

docs/decisions/network-handler-behavior.md[72-79]
docs/decisions/network-handler-behavior.md[208-226]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
The appendix implementation is presented as satisfying the ADR behaviors, but it lacks exception handling around handler invocation and does not show how staged mutations are discarded when a handler errors.

### Issue Context
Behavior 4 requires uncaught exceptions to be logged, staged changes discarded, and processing to continue as if the handler was not registered.

### Fix Focus Areas
- docs/decisions/network-handler-behavior.md[72-85]
- docs/decisions/network-handler-behavior.md[208-229]

### What to change
- Wrap `h.call(request)` in a begin/rescue (or language-appropriate equivalent) and explicitly note logging + continuing.
- Either:
 - update the pseudocode to show per-handler isolation/rollback of staged mutations (e.g., snapshot/clone before calling handler and restore on exception), or
 - amend the text to state the pseudocode is simplified and *does not* model rollback semantics (so it’s not claimed to fully satisfy behavior 4).

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

2. Overgeneralized exception behavior 🐞 Bug ⚙ Maintainability

Description

The ADR states (in a comparative bullet) that Selenium logs uncaught handler exceptions instead of
ending the session, but current behavior is binding-specific (e.g., Ruby handler blocks are executed
without rescue/logging, while Python does catch/log). This should be reworded as a proposed
cross-binding behavior or explicitly scoped to the bindings that implement it today.

Code

docs/decisions/network-handler-behavior.md[R77-80]

+   * In Playwright, uncaught exceptions propagate to end the session, which causes problems when
+     something unrelated to the test's intent goes wrong.
+   * Selenium is more lenient and only logs the error to the console.
+

Evidence

Ruby currently invokes handler blocks without rescue/logging, while Python explicitly catches and
logs exceptions, so the ADR’s statement is not uniformly true across Selenium bindings today.

docs/decisions/network-handler-behavior.md[77-80]
rb/lib/selenium/webdriver/common/network.rb[89-96]
py/private/_network_handlers.py[746-756]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
The ADR text implies a universal Selenium behavior (“only logs the error to the console”) for handler exceptions, but current bindings differ.

### Issue Context
Ruby’s handler dispatch yields directly to user code without rescue; Python catches/logs and continues.

### Fix Focus Areas
- docs/decisions/network-handler-behavior.md[72-80]

### What to change
- Rephrase the bullet to one of:
 - “In some Selenium bindings (e.g., Python), uncaught handler exceptions are logged and processing continues…”
 - or “Proposed: Selenium should log uncaught handler exceptions and continue processing…”
- If desired, add a short note indicating current binding differences to avoid readers assuming uniform behavior today.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

3. Wrong handler verb names 🐞 Bug ≡ Correctness

Description

The ADR states Selenium request/response handlers use respond/submit (and response fail), but
existing bindings expose different verbs (e.g., Python provide_response/continue_request, Ruby
provide_response/continue) and do not offer a response-level fail. This mismatch will mislead
implementers/readers about current APIs and what exactly is being proposed to change.

Code

docs/decisions/network-handler-behavior.md[R31-43]

+   * Selenium supports:
+     * Request: `fail` (Playwright's `abort`, BiDi's `FailRequest`), `respond` (Playwright's `fulfill`, BiDi's `ProvideResponse`), and `submit` (Playwright's `continue`, BiDi's `ContinueRequest`).
+     * Response: `fail` (BiDi's `FailRequest`), and `submit`: note that since we don't need to prevent a round trip from a request, whether this is a BiDi `ContinueResponse` or `ProvideResponse` can be an implementation detail based on whether a replacement body value is provided.
+
+```ruby
+# Specifics of parameters and names can match spec details
+network.add_request_handler { |r| r.fail if something }
+network.add_request_handler { |r| r.respond(content: mocked_response) if something }
+network.add_request_handler { |r| r.add_header("X-Test", true) && r.submit if something }
+
+network.add_response_handler { |r| r.fail if something }
+network.add_response_handler { |r| r.submit(content: mocked_response) if something }
+network.add_response_handler { |r| r.add_header("X-Test", true) && r.submit if something }

Evidence

The ADR’s listed verbs don’t exist in current binding implementations, which use different method
names and capabilities for request/response interception.

docs/decisions/network-handler-behavior.md[31-43]
py/private/_network_handlers.py[325-438]
py/private/_network_handlers.py[440-581]
rb/lib/selenium/webdriver/bidi/network/intercepted_request.rb[30-58]
rb/lib/selenium/webdriver/bidi/network/intercepted_response.rb[31-68]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
The ADR uses handler disposition verbs (`respond`, `submit`, and response `fail`) that don’t match the verbs exposed by current Selenium bindings, which makes the document confusing as a proposal baseline.

### Issue Context
Current Python and Ruby bindings use `provide_response`/`continue_request` (Python) and `provide_response`/`continue` (Ruby), and do not expose a response-level `fail`.

### Fix Focus Areas
- docs/decisions/network-handler-behavior.md[26-44]

### What to change
- Replace “Selenium supports:” wording with either:
 - a binding-neutral set of verbs (and explicitly state they are *proposed* names), or
 - the currently used names (and optionally add a mapping table: proposed ↔ existing per binding).
- Update the Ruby/Python code examples accordingly (e.g., `provide_response`/`continue_request`/`continue`) and remove or clearly label response `fail` as a proposed addition if that’s intended.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

Results up to commit 18895d1

🐞 Bugs (2) 📘 Rule violations (0) 📎 Requirement gaps (0) 🎨 UX issues (0) 🔗 Cross-repo conflicts (0) 📜 Skill insights (0)

1. Wrong Python behavior claim 🐞 Bug ≡ Correctness

Description

The ADR claims current Python behavior lets continueRequest override failures and stubs, but
Python’s BiDi network handler registry resolves outcomes with fixed precedence where any fail()
wins and provide_response() wins over mutations. This misstates the existing semantics and can
mislead readers about what is actually changing.

Code

docs/decisions/17685-network-handler-behavior.md[R158-159]

+  - We could run every handler but have `continueRequest` override failures and stubs (current
+    Python behavior), but it is not obvious why that command should have precedence.

Evidence

Python’s handler registry documents and implements reconciliation where any handler calling fail()
causes the request to be failed during _resolve(), and the integration tests assert that fail
wins when handlers disagree—so continueRequest does not override failures/stubs in current Python
behavior.

py/private/_network_handlers.py[28-37]
py/private/_network_handlers.py[428-437]
py/test/selenium/webdriver/common/bidi_network_tests.py[251-266]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The ADR’s “Considered options → Reconciliation” section states that in current Python behavior `continueRequest` overrides failures and stubs. The Python implementation instead reconciles handler actions with a strict priority (fail > provide_response > continue-with-mutations/default continue), so the ADR’s comparison baseline is inaccurate.

## Issue Context
This ADR is intended to guide cross-binding behavior decisions; incorrect statements about existing behavior can skew review/consensus and lead to incorrect implementations.

## Fix Focus Areas
- docs/decisions/17685-network-handler-behavior.md[154-160]

## Suggested change
Reword the parenthetical to reflect the actual Python semantics (e.g., “current Python behavior reconciles outcomes with fixed priority (fail > provide_response > continue) after running all matching handlers”).

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

2. Auth callable contradiction 🐞 Bug ≡ Correctness

Description

The ADR excludes authentication handlers because “authentication should not use a callable,” but
Selenium Ruby’s add_authentication_handler explicitly accepts a block and existing tests use
block-based auth handlers (&:skip, &:cancel). This makes the ADR’s rationale inaccurate for at
least one binding and risks confusing implementers about the scope/baseline.

Code

docs/decisions/17685-network-handler-behavior.md[R20-21]

+This applies to request and response handlers, but not authentication handlers, since
+authentication should not use a callable.

Evidence

Ruby’s Network API accepts an optional block for add_authentication_handler and uses it when
credentials aren’t provided, and Ruby integration tests exercise callable auth handlers via &:skip
and &:cancel, contradicting the ADR’s claim that authentication should not use a callable.

rb/lib/selenium/webdriver/common/network.rb[47-63]
rb/spec/integration/selenium/webdriver/network_spec.rb[86-101]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The ADR states authentication handlers are out of scope because authentication should not use a callable. In current Selenium Ruby, authentication handlers can be provided as a callable/block, so the statement is not universally true across bindings.

## Issue Context
The ADR is meant to be cross-binding guidance; blanket statements that contradict an existing binding’s public API are likely to cause confusion.

## Fix Focus Areas
- docs/decisions/17685-network-handler-behavior.md[20-21]

## Suggested change
Reword to scope the statement as a proposal or explicitly note that auth handler behavior is out of scope and that existing bindings differ (e.g., “This ADR does not cover authentication handlers; bindings currently vary, and we may revisit whether auth should be callable-based separately.”).

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

Results up to commit 281e9bb

🐞 Bugs (1) 📘 Rule violations (1) 📎 Requirement gaps (0) 🎨 UX issues (0) 🔗 Cross-repo conflicts (0) 📜 Skill insights (0)

1. No cross-binding source references 📘 Rule violation ≡ Correctness

Description

The ADR describes binding differences and proposes user-visible handler behavior changes but does
not provide any concrete cross-binding source references (e.g., rg results or links to the
Java/Python/Ruby/.NET/JS implementations) demonstrating the comparison was actually performed. This
risks inconsistent implementation across bindings because reviewers/implementers cannot verify the
stated current behaviors or the proposed alignment points.

Code

docs/decisions/17685-network-handler-behavior.md[R13-22]

+The bindings diverge today: each grew its handler API independently, so dispatch order,
+multi-handler resolution, error handling, and what an event exposes are all inconsistent.
+
+| Binding    | Current behavior |
+|------------|------------------|
+| Java       | Only the first matching handler runs; disposition is always continue; a throwing handler propagates and leaves the request blocked; return-value driven; no response handler or managed body collection. |
+| Python     | An explicit `continue` in a handler fires immediately and wins; otherwise staged outcomes reconcile by `fail` > `provide_response` > `continue`; response handlers have no `fail`; dispatch is FIFO; a throwing handler's staged mutations are still sent; only the mutated event is visible; body is not collected behind the handler. |
+| Ruby       | Handlers run in parallel threads, so multi-handler disposition races; exceptions are logged; dispatch is FIFO with no default-continue; only the mutated event is visible; body collection is user-managed. |
+| .NET       | No request or response handler API. |
+| JavaScript | No request or response handler API. |

Evidence

PR Compliance ID 389265 requires evidence of cross-binding comparison (e.g., project-wide search
output or references to compared files). The ADR includes a cross-binding behavior table but
provides no file references or search evidence to substantiate the claims, making the required
comparison non-auditable.

Rule 389265: Compare cross-language bindings when changing user-visible behavior
docs/decisions/17685-network-handler-behavior.md[13-22]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The ADR asserts current cross-binding behaviors and proposes new user-visible behavior, but it lacks verifiable evidence of cross-binding comparison (project-wide search output and/or direct links to the analogous implementations in other bindings).

## Issue Context
Compliance requires that behavior-changing proposals include evidence of cross-language comparison so implementers can validate baseline behavior and keep bindings logically consistent.

## Fix Focus Areas
- docs/decisions/17685-network-handler-behavior.md[13-22]

## Suggested remediation
- Add a short section (e.g., "Cross-binding references") that lists at least one concrete file/link per compared binding (Java/Python/Ruby at minimum) that supports the "Current behavior" table entries.
- Optionally include a small snippet of the exact `rg ...` commands used (and the key hits) to demonstrate the repo-wide search comparison was performed.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

2. Java order nondeterministic 🐞 Bug ≡ Correctness

Description

The ADR table says Java runs “only the first matching handler”, but the current Java implementation
selects a matching handler from a ConcurrentHashMap stream with no defined encounter order, so the
chosen handler is effectively arbitrary and can vary between runs. This can mislead
readers/implementers about Java’s current precedence semantics.

Code

docs/decisions/17685-network-handler-behavior.md[18]

+| Java       | Only the first matching handler runs; disposition is always continue; a throwing handler propagates and leaves the request blocked; return-value driven; no response handler or managed body collection. |

Evidence

The ADR claims a “first” handler semantics for Java, but Java stores handlers in a ConcurrentHashMap
and selects via values().stream()...findFirst(), which does not guarantee ordering; therefore the
selected handler is not consistently the first-registered or otherwise deterministic handler.

docs/decisions/17685-network-handler-behavior.md[16-22]
java/src/org/openqa/selenium/remote/RemoteNetwork.java[47-50]
java/src/org/openqa/selenium/remote/RemoteNetwork.java[143-148]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
The ADR’s “Current behavior” row for Java states “Only the first matching handler runs”, which implies a deterministic ordering. In the current Java implementation, handler selection is performed by streaming `ConcurrentHashMap.values()` and taking `findFirst()`, which has no guaranteed iteration/encounter order.

### Issue Context
This is in the ADR’s binding comparison table; the goal is to accurately describe current behavior so readers understand what is changing.

### Fix Focus Areas
- docs/decisions/17685-network-handler-behavior.md[16-22]

### Suggested change
Reword the Java row to avoid implying deterministic precedence (e.g., “Only one matching handler runs (selection order is unspecified) ...”), or explicitly state the current selection is not ordered by registration.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

3. Markdown nesting broken ✓ Resolved 🐞 Bug ⚙ Maintainability

Description

Under decision #7, the first bullet line is indented less than the preceding continuation lines of
the same list item, which can cause CommonMark/GitHub rendering to treat it as a separate top-level
list instead of nested under item 7. This makes the ADR harder to read and can change the visual
structure of the decision.

Code

docs/decisions/17685-network-handler-behavior.md[R129-134]

+7. **Body data is collected only when the handler opts in at registration.** A body is not
+   available by default; the handler declares that it needs the body when it is registered — not
+   from inside the callback, since the collector must be in place before the event — and Selenium
+   then owns the collector's lifecycle, size cap, and browser-support quirks. The body is readable
+   on the event inside that handler.
+  * The user never calls `addDataCollector` / `getData` or tears a collector down.

Evidence

Within list item 7, the continuation lines are indented deeper than the bullet that follows,
indicating inconsistent nesting that can alter Markdown rendering.

docs/decisions/17685-network-handler-behavior.md[129-134]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
The nested bullet list under item 7 is not consistently indented, which can break Markdown nesting and render incorrectly.

### Issue Context
Under ordered list item `7.`, the paragraph continuation lines are indented more than the subsequent `*` bullet line, so the bullets may not be considered part of item 7.

### Fix Focus Areas
- docs/decisions/17685-network-handler-behavior.md[129-134]

### Suggested change
Indent the `*` bullet lines to the same level as the continuation lines for item 7 (typically 3–4 spaces after the margin for CommonMark nested content under an ordered list item).

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

[qodo-code-review]

Code review by qodo was updated up to the latest commit 18895d1

[titusfortner]

Actually, maybe the right answer here is to make this user-toggled with the default being to raise. I didn't consider this when suggesting alternatives below. Perhaps we do want to error-by-default but provide a global escape hatch while things are still in draft.

[AutomatedTester]

I feel fail fast should always be the right approach for people

[titusfortner]

First, to clarify, Python as written does the error behavior in this ADR (except for it doesn't discard staged changes).

As for fast-fail, that's correct for synchronous code evaluating the behavior. In network handlers, the failures will come from things the author didn't write like third-party scripts, analytics beacons, favicon retries, an auth challenge from an unrelated CDN. We don't want to necessarily couple the test's pass/fail to the open internet via a still-under-development BiDi protocol.

The reason I don't love this proposal as written is because when a user wants to mutate a specific thing as the intent of the test, silently ignoring it is a real problem, but I also don't want to de facto require users to always wrap everything in a try/catch which seems like a bad experience.

Honestly the desired default behavior is going to be different if a user wants to observe vs mutate, which might be a good reason to support the differentiation earlier. The alternative would be an error handling toggle for propagate vs log

[nvborisenko]

+1 to fail fast, notifying user your handler is bad. In low-level BiDi/.NET the default behavior is to break entire pipeline:

Example (pseudo code):

var handler = using network.intercept(r => raise exception)
network.navigate("selenium.dev")

navigate fails, swallowed. User sees exception at disposal. Dear user, investigate.

[AutomatedTester]

I wasn't suggesting my way was correct, I followed what other docs suggested which is why I did it.

[titusfortner]

As the ADR says though, I'd prefer if we could limit propagating failures to the tests/intentions not any potential malformed network call that the code or BiDi spec doesn't properly parse.

@nvborisenko yeah intercepts should propagate errors because they likely get into the intent of the test and are likely filtered in one fashion or another. My concern is when you create a handler to "log everything" that you're going to end up with a ton of unexpected errors because the internet is complicated and our spec is new. My concern goes away if we differentiate behavior between observers and intercepts, but if we continue with the "everything is an intercept for now" I see potential problems.

[nvborisenko]

Intercepting and just listening are different cases. BiDi provides different capabilities. I feel it should be different api methods. Argue: to listen to network traffic, we just subscribe. To intercept it: we have to add interception and react accordingly. Seems unnecessary client/server roundtrip just for listening.

[diemol]

Yes, I see it how @nvborisenko sees it. That is why I think this ADR and #17671 complement each other.

[diemol]

I think this ADR will help us unify handler behavior across all language bindings, which is currently handled differently in each language.

If I understand this correctly, @titusfortner suggests this ADR is an alternative to #17671. But what I see is that this ADR covers network interception, which blocks because it waits for a handler disposition. Whereas #17671 is about observation, which should not block, and only subscribes to events (did I get that right, @AutomatedTester?).

That is why I believe the two ADRs complement each other.

In addition, this ADR, like #17671, seems to me to be a middle-layer API. One that users would use for specific use cases that the high-level API won't cover.

[diemol]

I don't think this part is necessary unless you want to give more context by linking to the document. Which I think is not really necessary.

[titusfortner]

Yes, this more accurately belongs in the PR body not the ADR itself.

After a few ADRs I think I better understand what the split should look like and I tried to clarify here: #17697

[diemol]

I am confused. What does this mean? Is this "A user can register more than one handler for the same network phase, and matching handlers can disagree", the decision?

I am just looking for a paragraph that describes the decision.

[titusfortner]

"This" == "This ADR"

Yeah, let me update this ADR

[titusfortner]

@diemol

Whereas #17671 is about observation, which should not block, and only subscribes to events

In the last TLC meeting people were pushing for the idea that for now everything that can block will block in our implementations and we won't handle it any differently.

I'm saying they are in opposition because if you adopt this ADR, you don't need the extra methods proposed by the other ADR.

Also, I think it is wrong to characterize these as "middle-layer API." Anything that affects the code the user needs to write is user-facing and needs to be part of the high-level API contract we are creating that we want to maintain.

[diemol]

@diemol

Whereas #17671 is about observation, which should not block, and only subscribes to events

In the last TLC meeting people were pushing for the idea that for now everything that can block will block in our implementations and we won't handle it any differently.

I'm saying they are in opposition because if you adopt this ADR, you don't need the extra methods proposed by the other ADR.

Also, I think it is wrong to characterize these as "middle-layer API." Anything that affects the code the user needs to write is user-facing and needs to be part of the high-level API contract we are creating that we want to maintain.

The way I see these two ADRs:

• This one is to actually manipulate the network call.
• The other one is for observation, because if you want to do observation and interception, then this could actually slow down tests, because you're intercepting and manipulating the call. I don't think that's what we want to do.

That's why I think there are two other options: either observe or intercept here.

I still think that this is a middle-layer API. I don't want the user to have to handle all these callbacks and other things to manipulate their code in BiDi. I want this API to land and be used in a higher-level API to do network routing, mocking, and method interception, covering all of this implementation.

In my head, we should have:

1. The CDDL parsing to generate the low-level stuff
2. The part where we map commands and do the instantiation of the driver
3. This current ADR, which is like the middle level
4. Some high-level features that make it very easy for users to actually interact with BiDi

If this current ADR is what we're doing at the high level, it is way too complex.

[qodo-code-review]

Code review by qodo was updated up to the latest commit 281e9bb

[qodo-code-review]

Code review by qodo was updated up to the latest commit 895d532

[titusfortner]

1. I removed the requirement for a "complete" resolution method because it was confusing things and also because I no longer think it is necessary right now
2. I added the requirement for toggling observation vs interception to minimize the confusion around this one vs [docs] decision: BiDi events are awaited with expect_* context managers #17671. Our normal handler method needs to support observation directly either now or in the future, we don't need (and shouldn't have) a separate method just because of that.
3. I want to push back on calling this middle layer. This is what we agreed to as the high layer at the TLC summit in SF 2 years ago. To clarify, what I think you are suggesting is methods that wrap the handler so users don't need to work with lambdas for common cases? If so, that represents a major expansion of the API across all the bindings for no functional gain when we don't even have the underlying behavior we want sorted out.

Can we get the functionality implemented across the bindings then figure out which wrappers and helpers and fixtures to add later?

I feel like we may need to step back from the ADRs and agree to the milestones and requirements for Selenium 5 again.