webdriver icon indicating copy to clipboard operation
webdriver copied to clipboard

Merging capabilities seems broken?

Open gsnedders opened this issue 8 months ago • 3 comments

Imagine trying to process the following:

{
    "alwaysMatch": {"browserName": "fake"},
    "firstMatch": [
        {"browserName": "real"},
        {}
    ]
}

Per step 7.1 of process capabilities:

Let merged be the result of trying to merge capabilities with required capabilities and first match capabilities as arguments.

"Trying" returns the error if it fails, so I think the merged capabilities ends up with:

[
    [abstract error],
    {
        "browserName": "real"
    },
]

This seems wrong, given we then assume the argument passed to "match capabilities" is a JSON object.

If we instead use the same JSON serialization of an error as send an error — though we don't define this outside of sending an error so this is definitely not what the spec currently says — then the merged capabilities would be:

[
    {
        "error": "invalid argument",
        "message": "[implementation defined]",
        "stacktrace": "[implementation defined]"
    },
    {
        "browserName": "real"
    },
]

This also doesn't seem particularly helpful.

From the look of our current tests in /webdriver/tests/classic/new_session/merge.py, we seem to expect the following to return to the client "invalid argument":

{
    "alwaysMatch": {
        "acceptInsecureCerts": True
    },
    "firstMatch": [
        {},
        {"acceptInsecureCerts": True}
    ]
}

Thus it's not that we're "trying to merge capabilities", it's that we need to process that error by then returning error.

gsnedders avatar Apr 10 '25 18:04 gsnedders

My understanding was that "trying" causes an early return of the algorithm where it is used. So if "merge capabilities" returns an error while "trying to merge capabilities", "process capabilities" returns with that error immediately (so "merged" is not getting defined and the algorithm does not continue) and I think this should propagate all the way to result in an invalid error?

OrKoN avatar Apr 10 '25 18:04 OrKoN

Oh, the return in https://w3c.github.io/webdriver/#dfn-try isn't from the "trying" steps, it's from the calling algorithm? That seems surprising, and I'm unaware of any other spec using a construct like that in their prose.

If we were following https://infra.spec.whatwg.org/#algorithm-control-flow, then we'd be talking about throwing it and eventually catching it.

gsnedders avatar Apr 10 '25 23:04 gsnedders

Yeah, I found it surprising too but other interpretations make many parts of the spec (and also the bidi spec) broken in the way you described. The definition mentions that "trying" is equivalent to doing defined steps so I think it means it's more like a shortcut to inline those steps without defining a new function.

OrKoN avatar Apr 11 '25 05:04 OrKoN