w3c / webdriver-bidi

Bidirectional WebDriver protocol for browser automation
https://w3c.github.io/webdriver-bidi/
353 stars 39 forks source link

Extensible and Type Discrimination #480

Closed jrandolf-2 closed 1 year ago

jrandolf-2 commented 1 year ago

While working on Puppeteer, we've noticed it's very difficult to discriminate response types because response types have extensibility. This causes a reduction problem because, e.g., an EmptyResponse can in theory contain the keys of ErrorResponse, but the values are something completely different.

I propose instead of spreading extensibility, we go along with one of the following standard options:

CC: @jgraham @whimboo

jgraham commented 1 year ago

Isn't this a fundamental problem if you just look at the CDDL? Per the conversation about intersection types, I think something that matches ErrorResponse would also match EmptyResponse, and so if the assumption is that implementations will in fact add arbitrary extra data to the response guided only by the CDDL requirements, then we can't spec this away.

On the other hand, nothing in the spec currently allows browsers to actually add any extra data to responses. The point of Extensible was "clients have to accept payloads with extra fields in these places". But if that's always true, then it isn't adding anything.

I think what's actually needed is some constraints on how implementations can represent unilateral extensions of the protocol (which are hopefully rare, but for which there is precedent in e.g. capabilities). In that case we require that any extra fields have a : in the name, and commit to never specifying a field with such a name.

jgraham commented 1 year ago

For the specific case of Message types, putting an explicit type field on seems like the obvious thing that we should have done (and still can do), i.e. type: "success", type: "error", and type: "event".

jrandolf-2 commented 1 year ago

For the specific case of Message types, putting an explicit type field on seems like the obvious thing that we should have done (and still can do), i.e. type: "success", type: "error", and type: "event".

Perhaps we should try this then.