Some type checks in capabilities.rs for geckodriver raise UnknownError instead of InvalidArgument
Categories
(Testing :: geckodriver, defect, P1)
Tracking
(firefox71 fixed)
Tracking | Status | |
---|---|---|
firefox71 | --- | fixed |
People
(Reporter: whimboo, Assigned: whimboo)
References
Details
Attachments
(1 file)
(deleted),
text/x-phabricator-request
|
Details |
Not sure if that is expected but for me it looks like a bug in various places in capabilities.rs of geckodriver. Several type checks raise an UnknownError
instead of InvalidArgument
like that one:
Andreas, I assume that was an oversight and as such collides with the WebDriver spec.
Comment 1•5 years ago
|
||
I think the answer is it depends on a case-by-case basis.
When an error is caused by invalid input, i.e. it breaches with the
types and bounds checks described in WebDriver, it should generally
be coerced to ErrorStatus::InvalidArgument
. This is the case in
the concrete example you linked to because we expect profile
to
take one particular shape and the mistake is clearly on the user’s
side.
There are other cases where the input is not at fault, e.g.
hypothetically imagining that we were unable to write the provided
Base64-encoded profile to disk, and where it is appropriate to use
ErrorStatus::UnknownError
for the lack of a more specific error
code in WebDriver.
Does that make sense?
Assignee | ||
Comment 2•5 years ago
|
||
Yes, that is exactly what I was expecting. I will get this fixed.
Assignee | ||
Comment 3•5 years ago
|
||
Comment 5•5 years ago
|
||
In hindsight, this change should’ve been noted in the changelog.
Leaving a comment here so we remember to do so when making the release.
Assignee | ||
Comment 6•5 years ago
|
||
I prefer to make changes to the changelog when we do the release work. Think about possible backouts, follow-ups which would cause further changes. We could add a whiteboard entry for those bugs where we think it makes sense to add a changelog entry.
Comment 7•5 years ago
|
||
bugherder |
Description
•