[dev/1.0.0] ZBytes::iter stop to the first invalid item instead of raising the error

[wyfo]

Describe the release item

zenoh/zenoh/src/api/bytes.rs

Line 235 in 9bcfdc7

let t = ZSerde.deserialize(&kpld).ok()?;

Iterator should yield Result<T <ZSerde as Deserialize<'a, T>>::Error> instead, and first yield the error, then yield None.

@Mallets @milyin

[milyin]

Dmitry @yellowhatter pointed that this ZResult is quite large (24 bytes) so returning it on each step is not an optimal solution.
Maybe it's better to just provide a way to allow user to get the error stored in the iterator?
Another solution could be to return error on each step, but use some lightweight error type. @yellowhatter thinks that it would be good to optimize ZResult using only error codes, since this type appears in hot path all the time. So if we principally decide to reduce ZResult size later, we could use this approach for iterator

[wyfo]

About the result size, the iterator item type is generic, so the size of the ZResult<T> depends only of the size of the type T, and may use niche optimization; we also already box the error type, so I don't see how there could be an issue regarding the size of ZResult<T>.

Dmitry @yellowhatter pointed that this ZResult is quite large (24 bytes) so returning it on each step is not an optimal solution.

I disagree with the conclusion of this sentence. If the function is inlined, the result will not be "returned" at each step. This function is generic, so it is inlinable cross-crate, and because it's in a iterator, I believe it will mostly be inlined.
And again, I don't see any memory size issue.

[wyfo]

Another solution could be to return error on each step, but use some lightweight error type.

That's already the case because we use boxed error. Maybe there is a confusion with ZError, which is pretty heavy, but is still boxed anyway.

[Mallets]

If I've understood it correctly, you are proposing for the iterator to return Result<T, <ZSerde as Deserialize<'a, T>>::Error>.

User code will be something like:

while let Ok(Some(t)) = sample.payload().iter::<String>().next() {
    // Do stuff
}

If that's the case I'm fine with the proposal and @wyfo 's argument.

[wyfo]

Small correction, the pattern matching will be Some(Ok(t)).
But the user can also just use sample.payload().iter::<String>().collect::<Result<Vec<_>, _>>(), this is a common pattern, supported by the stdlib.

[Mallets]

Correct with the pattern matching Some(Ok(t)).

[wyfo]

Anyway, a simpler solution could be to simply to let the user do the deserialization himself, and just return an iterator of ZBytes.

[Mallets]

I think it's not nice to let the user do it while we can do it with improved ergonomics.

[Mallets]

Closed by #1120