Error messages are incomplete

[steven-joruk]

thread 'main' panicked at 'failed to read settings: Error { inner: ErrorImpl { kind: UnexpectedEventType { expected: StartDictionary, found: String }, file_position: None } }', examples/settings/src/main.rs:50:30

It would be much more useful if it said which key it was trying to deserialize, and if the file_position was set.

Using plist v1.0.0 and serde v1.0.112

[ebarnard]

At the moment error messages only report file position when the error is during parsing rather than deserialisation. It probably would be possible to include file potisions alongside the event stream that is passed to the deserialiser.

[ebarnard]

How would you envisage the error presenting the item being deserialised - something like JSONPath?

[probablykasper]

Would be really great to have this

[jcgruenhage]

I'm stumbling over this as well right now. Something like JSONPath for showing what it's failing at would be very nice indeed, but I think that's a lot harder and a lot less important than the file position. With the file position known, the rest can be determined by the developer in some way anyway.

[ebarnard]

There are currently a lot of places where file position and path information is lost. The solution to keep track of position would be very similar to keeping track of path so we may as well do both. What we really want to do is pass something like the following around in the place of a plain Event.

enum PathSegment<'a> {
    Array(usize),
    Dictionary(Cow<'a, str>),
}

struct EventWithContext<'a> {
    event: Event<'a>,
    position: Option<FilePosition>,
    path: Option<&'a [PathSegment<'a>]>,
}

[jcgruenhage]

As the event type comes from xml-rs afaict, this probably needs to touch similar code paths to the quick-xml migration, right? Is this work that's already planned more concretely, than the WIP PR over at #68?

[ebarnard]

I mean the internal plist Event type https://github.com/ebarnard/rust-plist/blob/32103bf0c6d612c0efaa27038f9183d96ad5d953/src/stream/mod.rs#L52

[jcgruenhage]

I've started toying around with this, and I can really see now why it hadn't been done previously. Things that make this unpleasant:

• #4: This is a pretty big one. It makes handling keys vs values in dicts a PITA in the binary reader, which makes keeping path info more complicated than it otherwise would be.
• Having path as Option<&'a [PathSegment<'a>]> doesn't work, it needs to be owned, because we need to change it for each event, which we can't do if we've given back a reference with the previous event. Something like a path node where each node keeps a reference to it's parent node as well as the segment info above, that could work, but that's also not pretty.
• Tracking path/pos through everything is quite a big change, and should there ever be a change like this again, all of these code paths need to change again. Maybe it'd make sense to base this on a trait instead, which is responsible for giving human readable position information about where the error occurred? The different event sources have different pieces of information available after all. Getting events from a Value doesn't have a byte offset, line/column info doesn't apply for the binary format, the path segments as described above wouldn't work with binary formats either, if keys that aren't string pop up, lots of things like this.

[magebeans]

Would it be possible to use something like https://docs.rs/serde_path_to_error/latest/serde_path_to_error/ to get this for free instead? I've been trying to patch together a plist equivalent of the json example in that crate, but I can't seem to be able to pull out the right Deserializer to feed it when working with plists.

[jcgruenhage]

You want something like this:

    let cursor = Cursor::new(<input here>);
    let reader = plist::stream::Reader::new(cursor);
    let mut deserializer = plist::Deserializer::new(reader);
    let result : YourType = serde_path_to_error::deserialize(&mut deserializer).unwrap();

Good find! Nice to know that serde_path_to_error exists, it solves this IMO.

[failable]

thread 'main' panicked at 'called `Result::unwrap()` on an `Err` value: Initialze("UnexpectedEventType { expected: ValueOrStartCollection, found: EndCollection }")', src/safari.rs:81:14

Have similar issue when trying to deserialize from the "Bookmarks.plist" of Safari. Not sure how to debug this.

[ebarnard]

@liebkne If this is still a problem can you open a new issue and provide the plist file you are trying to deserialise.