[docs] decision: network responses expose their original body and request timing#17674
[docs] decision: network responses expose their original body and request timing#17674AutomatedTester wants to merge 2 commits into
Conversation
883901e to
9114a9a
Compare
titusfortner
left a comment
There was a problem hiding this comment.
Based on this, I think we're in agreement with my point 8 in #17685
This is essentially the specifics that I deferred in that ADR
In addition to the specific comments below, this ADR should include disabling body collection by default and allow users to enable it if desired. Enabling a collector that isn’t used can be a lot of unnecessary overhead.
| not support data collectors, these methods raise a **clear, typed "unsupported" | ||
| error** — never a silent empty body. Bindings SHOULD expose a capability check so users | ||
| can branch. | ||
| - **Fetch-then-patch**: a response handler can read the original body, modify it, and |
There was a problem hiding this comment.
This can work for requests, but not for responses as the spec is currently written.
network.responseStarted is emitted after response headers are received but before the body is complete. If the event is blocked, the remote end stores the request in the blocked request map and then awaits "continue request". The response cannot reach responseCompleted while responseStarted remains blocked by the interception, so the callable can never access the body.
Do we currently have a way to differentiate add_response_handler for whether the phase is started or complete? Do we need separate methods for this?
| data collector (`addDataCollector` + `getData`). The binding manages collector | ||
| lifecycle; users do not handle collector ids for the common case. Where the browser does | ||
| not support data collectors, these methods raise a **clear, typed "unsupported" | ||
| error** — never a silent empty body. Bindings SHOULD expose a capability check so users |
There was a problem hiding this comment.
Selenium should not proactively couple itself to transient browser implementation matrices. Selenium does need an error handling policy (see point 4 in #17685), but it should be passing through what the browser responds with rather than requiring temporary gating logic.
|
|
||
| Bindings expose, on their existing `Response`/`Request` wrappers: | ||
|
|
||
| - **`body()` / `text()` / `json()`** on a response, reading the **original** payload via a |
There was a problem hiding this comment.
This ADR should not require text() or json() helpers at this time. If we are intentionally deferring larger functional concerns like observation-vs-interception, we should also defer these kinds of helper methods.
The cross-binding contract should be limited to exposing the collected body in a binary-safe form. Text decoding and JSON parsing would add an unnecessary surface area around content-type parsing, charset handling, invalid encodings, binary payloads, compressed bodies, and language-specific JSON return types that are not a good priority.
Users can do these conversions themselves with the libraries and assumptions appropriate for their application. We can add convenience helpers later if there is clear demand.
Rename 0004 -> 17674 to match the PR number.
💥 What does this PR do?
Proposes a design decision record: network responses expose their original body and request timing.
The high-level network handlers can mutate/stub traffic but can't read the original response body or expose request timing — the two most common monitoring needs. Both are now supported by BiDi:
network.addDataCollector+network.getDatacapture response (and increasingly request) bodies, andFetchTimingInfo(13 fields) already rides on every request. This record proposesbody()/text()/json()and fetch-then-patch on theResponsewrapper (via managed data collectors) plustimingon theRequestwrapper. Because data collectors are newer and unevenly implemented across browsers, body-read raises a typed "unsupported" error with a capability gate rather than failing silently; timing is always available.🔧 Implementation Notes
docs/decisions/process from [docs] add design decision record process and template #17665 — depends on [docs] add design decision record process and template #17665 landing (adds onlydocs/decisions/0004-*.md).response, addedrequest; Chromium supported, consumed by Puppeteer; known FF redirect-timing edge cases). The binding manages collector lifecycle so users don't juggle collector ids.🤖 AI assistance
💡 Additional Considerations
One of a set of BiDi ergonomics decision records proposed together. Cross-binding convergence is tracked in the binding-status table.
🔄 Types of changes