1410838 - Error message from capabilities parsing lacks necessary granularity

Status: NEW

(Testing :: geckodriver, defect, P3)

Description

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Currently the following capabilities will result in a failure message which is totally useless:

{"capabilities": {"alwaysMatch": {"moz:firefoxOptions":{"binary":"path"}}}}

The failure as shown is:

1508752193486 webdriver::server DEBUG <- 500 Internal Server Error {"value":{"error":"session not created","message":"Unable to find a matching set of capabilities","stacktrace":"stack backtrace:\n 0: 0x47d2a4 - \n 1: 0x45d174 - \n 2: 0x427701 - \n 3: 0x42ac5a - \n 4: 0x45ff5e - \n 5: 0x45581f - \n 6: 0x4437a6 - \n 7: 0x443aa6 - \n 8: 0x6de879 - \n 9: 0x4439e9 - \n 10: 0x44385b - \n 11: 0x4575c8 - \n 12: 0x6da071 - \n 13: 0x74f38744 - BaseThreadInitThunk\n 14: 0x776a582d - RtlGetAppContainerNamedObjectPath"}}

It should really mention that the binary cannot be found.

Comment 1

[Andreas Tolfsen ❲:ato❳]

> <jgraham> ato: We would change the return type of validation from Bool to Result<(), String> and create a Vec of errors from each capability set we try to match

Comment 2

[Henrik Skupin [:whimboo][⌚️UTC+1]]

This issue has actually been original filed as https://github.com/mozilla/geckodriver/issues/832. Also see some discussion over there.

Comment 3

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Btw. it's not only necessary to check the binary location as given by the capability, but also the path as returned by mozrunner when trying to find the default system installation. If we don't do that we could see issues like https://github.com/mozilla/geckodriver/issues/940#issuecomment-362306534.

Comment 4

[Andreas Tolfsen ❲:ato❳]

This bug was originally filed for
https://github.com/mozilla/geckodriver/issues/832, although it talks
only about the specific case of the binary capability not being
valid. In the original issue Simon requested better details in
general for capabilities parsing, so I will repurpose this bug to
change the way we collate errors as I described in
https://bugzilla.mozilla.org/show_bug.cgi?id=1410838#c1.

I haven’t decided if this will fix
https://bugzilla.mozilla.org/show_bug.cgi?id=1524584 or if I will
address the binary check later.