[adr] Behavioral contract for the low-level WebDriver BiDi layer - #17786
[adr] Behavioral contract for the low-level WebDriver BiDi layer#17786titusfortner wants to merge 9 commits into
Conversation
PR Summary by QodoAdd ADR defining behavioral contract for low-level WebDriver BiDi layer
AI Description
Diagram
High-Level Assessment
Files changed (1)
|
Code Review by Qodo
1.
|
|
Code review by qodo was updated up to the latest commit 98e1858 |
There was a problem hiding this comment.
Pull request overview
Adds a new ADR to the cross-binding decision log that defines a behavioral contract for Selenium’s internal, low-level WebDriver BiDi protocol layer (validation, extensibility, parsing strictness, and surface shape), intended to keep binding implementations aligned on wire-observable behavior.
Changes:
- Introduces a decision record enumerating required outbound/inbound behaviors for the BiDi low-level layer.
- Defines a two-tier taxonomy (Compliance vs Decision) to separate spec-forced requirements from Selenium-standardized choices.
- Records rationale, alternatives, and consequences to support consistent adoption across bindings.
|
Code review by qodo was updated up to the latest commit dfe1c07 |
|
Code review by qodo was updated up to the latest commit 5b77f31 |
|
Code review by qodo was updated up to the latest commit e965829 |
|
Code review by qodo was updated up to the latest commit 1b0e91a |
…uire only that malformed input raises
Code Review by Qodo
1. Missing selector behavior undefined
|
| strict default (a Decision, item 9), not part of this compliance floor; for a union whose arms are | ||
| all objects, a non-object payload has no variant to select and raises. | ||
|
|
||
| 6. **Inbound fields are validated strictly against the resolved type.** *(Decision.)* Once item 5 has |
There was a problem hiding this comment.
This may lead to bad situation. Let's image one required (non-nullable) field is added into the spec. Selenium added it accordingly. Users will get serialization error because this field is missing in payload... because... some vendor still didn't implement it.
IMHO serialization process should be relaxed.
Real example: w3c/webdriver-bidi#1117
There was a problem hiding this comment.
So you know, this comment resulted in my spending the past 5 days overhauling the ADR. Not sure it was worth that much time investment, but I'm much happier with it now. The TL/DR is that I now think inbound should be strictly validated except for missing required parameters.
… split, relaxation scope, consistency pass
|
Code review by qodo was updated up to the latest commit 541031b |
…n, inbound validation & retention, typed surface
|
Code review by qodo was updated up to the latest commit deb870f |
|
I just pushed a substantial rewrite of both the ADR and updated the PR description to match it. The main behavioral change is inbound required-field absence: the proposal now leaves a missing required inbound field omitted and warns, rather than rejecting the whole message by default. That will likely end up being the main policy question for review: should Selenium tolerate that incorrectness in order to be more flexible to browser/schema drift. The ADR is now framed around representability: outbound stays strict because Selenium controls what it sends; inbound tolerates only departures that can be represented without inventing data; corrupt present values and unknown closed-vocabulary members still error. The implementation comparison in the PR body is there to show which current differences are implementation cost/adoption gaps and which ones are real policy choices. |
📄 The decision, rationale, considered options, and consequences are in the record file this PR adds —
read it there. Below are proposal notes and review logistics.
🔗 Related
📝 Proposal notes
shared schema makes some behavior cheap to derive, but the contract is still the behavior every binding
exhibits, regardless of how it is implemented.
messages back into typed objects. Transport behavior is out of scope and should be decided separately if
needed.
decision list. It separates three things that are easy to conflate: behavior that is already agreed,
behavior that is unfinished or not-yet-adopted, and behavior where there is a real policy choice.
from generation are the pressure points: closed-type rejection, vendor extras on extensible types,
omitted-vs-null preservation, extra-property retention, and typed value objects. Selenium can choose a
simpler common standard that is easier for handwritten implementations, but that permits lossy behavior. Or
it can require the more precise per-type behavior everywhere. The table matters here because it shows
handwritten does not automatically mean "cannot do this"; some handwritten implementations pay the per-type
cost where they consider it correct.
tolerate only what can be represented without inventing data; reject what would produce a false typed
object.
and may be retained where useful.
resolved type. The corrupt-value case is the hard floor, since accepting it means abandoning runtime
validation entirely.
remain omitted and warn so Selenium does not block users when its schema is ahead of a browser
implementation, accepting that this also tolerates a genuine defect because the layer cannot tell the two
apart?
📊 Implementation comparison
Where each implementation (including current, proposed, and external) currently stands against the ADR
The record states three behavioral decisions (D1–D3); the table expands them into the comparison points
that best distinguish current implementations.
Per-implementation through-lines
time, while inbound is whatever falls out of casting the payload onto the object — no runtime checks, so
2.1/2.2 are ❌ and 2.3a passes only incidentally.
data is carried on spec-extensible types, and corrupt or malformed inbound values error. Its intentional
split from the ADR is policy, not cost: a missing required inbound field is an error because the payload is
objectively invalid against the schema.
bag instead of being modeled per type, and missing fields get defaults instead of being tracked as absent.
Those choices avoid expensive per-type work, but they are lossy, which is why its misses line up with the
rows that are costly by hand and cheap to generate.
adoption gaps.
Notes
① A malformed outbound message fails wdio's TypeScript compilation and never reaches the wire, which the
ADR accepts as local validation for a statically-typed binding.
② .NET trunk holds vendor extras only in a blanket
additionalDatasink on the message envelope, with noper-type extension on the spec-extensible types themselves — so a vendor capability (
goog:*/moz:*) orcookie attribute can't attach to the type that carries it, and never reaches the wire or survives a
round-trip. (bidi-net has the same envelope hatch but also carries per-type extension data on the
extensible types, so its extras do reach the wire — which is why it passes 1.5 and 2.6.)
③ wdio tolerates a missing required field because it does not validate inbound payloads at runtime; absence
stays absent, though there is no field-specific warning path.
④ This binding errors on a missing required field instead of tolerating it as absent, so the whole message
is rejected where the ADR would keep going.
⑤ .NET trunk and Python trunk don't error, but instead of leaving the field absent they fill it with a
default (
0/null/false) a caller can't distinguish from a genuine value — a missing size reads as 0, amissing flag as false, and downstream code acts on fabricated data with nothing to signal the field never
arrived.
⑥ Cheaper for static bindings: an attribute-driven deserializer rejects nulls and wrong types (2.1–2.2)
from the type system, even un-generated.
⑦ Cheaper for dynamic bindings: a sentinel keeps omitted distinct from null (2.3b) without a static
Optional/wrapper.🗣 Discussion
Notes from Slack and TLC minutes are recorded here as the proposal is discussed.
📌 Tracking
Tracking issue: (linked on acceptance)