WebDriverError misses optional "data" property

NEW
Unassigned

Status

defect
P2
normal
11 months ago
7 months ago

People

(Reporter: whimboo, Unassigned)

Tracking

(Blocks 1 bug)

Firefox Tracking Flags

(Not tracked)

Details

As defined by the spec the WebDriverError contains an optional `data` property which can hold extra data to help with error diagnosis:

https://w3c.github.io/webdriver/#handling-errors

> Optionally "data", which is a JSON Object with additional error data helpful in diagnosing the error.
Hm, I have troubles in geckodriver and webdriver to store the data key. I tried the following:

> pub data: Option<BTreeMap<String, Json>>,

But the compiler complains that the required trait is not implemented:

> 1262 | #[derive(RustcEncodable, RustcDecodable)]
>      |                          ^^^^^^^^^^^^^^ the trait `rustc_serialize::Decodable` is not implemented for `rustc_serialize::json::Json`

I assume that I cannot use `Json` as value in the BTreeMap, but not sure what else I should choose.

Andreas, can you please help? Thanks.
Flags: needinfo?(ato)
Here the WebDriver spec definition of `error data`:

> An error data dictionary is a mapping of string keys to JSON serializable values that can optionally be included with error objects.
Also I wonder if we should use a builder API for WebDriverError so that it is easier to set the optional data and stacktrace properties.
I’m afraid I don’t know serde well enough to know what the appropriate
type to use here is.  I would assume rustc_serialize and serde are
mutually incompatible in the sense that serde won’t know how to
serialise a serialisation type from rustc_serialize.

Perhaps you want to use a serde_json::value::Value here?
https://docs.serde.rs/serde_json/value/enum.Value.html

With regards to the API, a builder immediately sounds overkill but
we should examine what the use cases are.  We currently don’t make
use of the "data" field at all so it is something we could approach
later.
Flags: needinfo?(ato)
(In reply to Andreas Tolfsen 「:ato」 from comment #4)
> I’m afraid I don’t know serde well enough to know what the appropriate
> type to use here is.  I would assume rustc_serialize and serde are
> mutually incompatible in the sense that serde won’t know how to
> serialise a serialisation type from rustc_serialize.

This bug has nothing to do with Serde, which is a different effort and might still take a while.

So my request from above is all about rustc_serialize/deserialize.

> With regards to the API, a builder immediately sounds overkill but
> we should examine what the use cases are.  We currently don’t make
> use of the "data" field at all so it is something we could approach
> later.

We have two implementations (new, and new_with_stack) in how to create a WebDriverError:

https://dxr.mozilla.org/mozilla-central/source/testing/webdriver/src/error.rs#265

Given that `data` is also optional we would probably need two more methods.
Flags: needinfo?(ato)
(In reply to Henrik Skupin (:whimboo) from comment #5)
> (In reply to Andreas Tolfsen 「:ato」 from comment #4)
> > I’m afraid I don’t know serde well enough to know what the appropriate
> > type to use here is.  I would assume rustc_serialize and serde are
> > mutually incompatible in the sense that serde won’t know how to
> > serialise a serialisation type from rustc_serialize.
> 
> This bug has nothing to do with Serde, which is a different effort and might
> still take a while.
> 
> So my request from above is all about rustc_serialize/deserialize.

Sorry, then I don’t know the answer to your question.
Flags: needinfo?(ato)
Then I think it makes more sense to wait for the Serde work.
Assignee: hskupin → nobody
No longer blocks: 1396821
Status: ASSIGNED → NEW
Depends on: 1396821
Priority: P1 → P3
No longer blocks: 1264259
Depends on: 1264259
Blocks: webdriver
Priority: P3 → P2
You need to log in before you can comment on or make changes to this bug.