Errors are not properly propagated

[maxammann]

I forgot to implement visit_seq for deserializing a struct. That caused postcard to fail deserialize it but json-serde succeeded as it users visit_map. The problem is that for any error which happens during serialization postcard returns the error SerdeDeCustom.

called `Result::unwrap()` on an `Err` value: SerdeDeCustom
thread 'tests::integration::test_serialisation_postcard' panicked at 'called `Result::unwrap()` on an `Err` value: SerdeDeCustom', src/tests.rs:74:88

If I do not implement visit_seq this code is called by serde:

    fn visit_seq<A>(self, seq: A) -> Result<Self::Value, A::Error>
    where
        A: SeqAccess<'de>,
    {
        let _ = seq;
        Err(Error::invalid_type(Unexpected::Seq, &self))
    }

The error Err(Error::invalid_type(Unexpected::Seq, &self)) just vanishes and becomes a SerdeDeCustom. This is very hard to debug.

[jamesmunns]

Hey! Thanks for reporting this. I ran into a similar problem recently.

The challenge is that I have three choices that I can see:

1. Discard the error context
   • doesn't require a heap (for dyn Error) or a generic type (for A::Error)
2. Make it generic
   • doesn't require a heap, does require a generic type
3. Make it dynamic
   • does require a heap, doesn't require a generic type

Since postcard is embedded focused, and already uses a LOT of generic types, I went with option 1 to "keep it simple". But perhaps I made it too simple.

If you're interested in implementing option 2 in a PR, I'd be interested in seeing what the net difference is. This would require adding a variant to the Error struct that contains the relevant (generic) Serde error.

Option 3 is probably not acceptable, though if it could be done optionally (e.g. only when use-std is enabled), that might be enough for debugging PC-side.

[maxammann]

Thanks for the detailed response, I'm just starting to learn Rust, but actually implemented proper error handling in a project last week. So I think I understand the options you proposed.

With a generic type in option 2 you mean en enum, right? So a 'static enum which implements the std:error:Error trait, right?

[HalfVoxel]

I'll add in another vote for this. It's really tough to debug any deserialization errors since they are completely discarded.

I'd go for option 3, but only enabled when use-std is enabled. The common case is that deserialization succeeds, after all. And then the error type does not have to allocate anything Option<Box<Something>>.