search

Idein / dockworker

Docker daemon API client in Rust
61 stars 35 forks source link

Improve: serializer/deserializer #86

Open eldesh opened 4 years ago

eldesh commented 4 years ago

Moved from https://github.com/eldesh/dockworker/issues/84#issuecomment-611449910 (@emk)

Thank you for maintaining dockworker!

I'm currently trying to replace boondock with dockworker in cage, and I'm hitting this same error.

I fought with quite a few of these errors back in the day when working on boondock, and here's the strategy I used:

  1. Assume that basically all fields in all structures returned by docker should have type Option<T>, until proven otherwise. There's no internal documentation in the docker source about what can be null and what can't, and sooner or later, it feels like almost anything can be null.
  2. Centralize all JSON decoding. There should only be one call to serde_json::from_slice in the library. And it should be serde_json::from_slice, unfortunately, and not serde_json::from_reader, because...
  3. There needs to be some mechanism for automatically logging JSON parse errors with the surrounding context, so that you can see what broke serde_json. In boondock, I included the entire JSON blob in the error, but that could be cut down to just show the 30 characters on either side.

I would actually love to completely deprecate boondock in favor of dockworker, and just submit occasional PRs to dockworker. But to do that, there would need to be some kind of mutually agreed-upon solution for (1-3). Would that be of interest to you, if I did the refactoring?

eldesh commented 4 years ago

Making the decoding errors richer (solution 3) seems like a good idea.

eldesh commented 4 years ago

Solution 1 seems a bit over the top. I would like to fix the bugs that have occurred, in turn.