Re: Response data should always be in a top-level "value" property.

On Wed, Oct 5, 2016 at 6:02 PM, Andreas Tolfsen <ato@mozilla.com> wrote:

> Sorry I’m late to the party (-;
>
> Simon Stewart <simon.m.stewart@gmail.com> writes:
>
> > Proposal: any response from an end node that contains data should put
> > that data in a "value" property of a JSON object.
>
> Most importantly, I think this is a very late and untimely change to
> make if we want to see the specification in CR-state by end of January.
>

It seems like a large change, but it's not massive when you audit the
actual commands. Indeed, most of the existing implementations already
always send the response data in a "value" property.


> I was under the impression that the fundamental design of the protocol
> would not change; especially not on a decision that was made—at a F2F
> none the less—over two years ago.
>

It'd be nice not to have to change anything, but in this case, as I
explained in a previous email, I think that this is a change worth making.
Again, there are currently 11 commands, mostly around window sizing, that
this impacts.


> Mozilla has put a lot of engineering resources into adapting our
> implementation to the current definition.  Changing it now will require
> a non-insignificant investment on our end.
>

I'd be happy to help.


> I’ve done the backwards compatibility dance around this before, and I’m
> hesitant to do it again for what I perceive as only aesthetic changes.


If this was merely aesthetics, I'd not propose the change.


>
> > 1) Processing the result cannot be a two stage process of dumbly
> > extracting the return value and then coercing to a type. Instead,
> > local ends must know the command being responded to (information not
> > included in the response) and switch behaviour as appropriate. This
> > additional complexity is needless.
>
> A client will always need to have some level of knowledge of the data
> structure that is returned.  If they blindly accept whatever the
> server sends, they may break the API contract they give to consumers.
>

I'm thinking of the two step process that most of the selenium local ends
do. They take the returned response, deserialise the data, add the session
id, figure out whether it's an exception, and then return the deserialised
response as something like a Map, Array, or primitive to the next level up.
That next level pulls out the required data from the response.

There's no "blindly" going on, but there is standard software layering.


> I buy the argument that it is easier for intermediaries to deal with an
> object where the return value is always encoded in the same place
> because they may not care about the data itself.  But on the other
> hand I think it is entirely reasonable for the local end to know more
> about the protocol it is implementing.
>

See above. The local ends know everything, but they still benefit from a
simpler processing model.


> For the ‘Set Timeouts’ and ‘Get Element Rect’ commands, any local end
> will inevitably have to look up specified keys in the returned JSON
> Object before it presents it to the user.  Whether you do this through
> some automatic JSON decoder codec (for which you could have one for W3C
> and one for Selenium) or manually is besides the point, because at some
> level it needs to know something about the protocol data structure.


See above.


>
> > 2) It's inconsistent. Generally, inconsistency is where bugs live.
>
> I think the idea of wrapping everything inside a ‘value’ field has a
> whiff of structured XMLness over it.
>

I don't understand. Also, XML was designed as a human readable data
interchange language. From a historical perspective, if hating XML wasn't
such a popular thing, I'd have used that for the json wire protocol.


> It might make the task of writing the W3C WebDriver-to-Selenium
> WebDriver shim easier, but at some level a client and a server needs to
> know certain details about what structures and types are returned so I
> don’t think it changes anything other than perhaps giving the code in
> Selenium a somewhat nicer appearance.
>

It also opens the door to us being able to extend the protocol if we need
to. If the response data can only be the value returned, without
decoration, we've shut the door on a path to move the spec forward in later
versions should we need to. Decoration also allows annotation of responses
with (e.g.) security data, or information about the customer (such as their
ID)


> My tl;dr on this is that I’m _not strongly opposed_ to the change as
> such, but I’d like us to think very carefully about whether we think it
> is necessary.  I agree with James that because it only creates another
> level of indirection, I too fail to see what it earns us apart from a
> certain aesthetic symmetry.
>

I strongly believe that this isn't just aesthetics.


> Moving a specification to Rec requires at least two conforming
> implementations, and it will take time to change geckodriver and
> potentially the Marionette automation protocol.  Because of this I think
> there is a distinct chance that the January CR deadline will slip.
>

geckodriver is most of the way there already, because of the way that the
spec suggests most commands send responses attached to a "value" property.
I'm happy to help throw some engineering effort into modifying the
remaining commands in geckodriver.

Simon