Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve: serializer/deserializer #86

Open
eldesh opened this issue Apr 13, 2020 · 2 comments
Open

Improve: serializer/deserializer #86

eldesh opened this issue Apr 13, 2020 · 2 comments
Labels
enhancement New feature or request

Comments

@eldesh
Copy link
Collaborator

eldesh commented Apr 13, 2020

Moved from #84 (comment) (@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 eldesh added the enhancement New feature or request label Apr 13, 2020
@eldesh
Copy link
Collaborator Author

eldesh commented Apr 13, 2020

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

@eldesh
Copy link
Collaborator Author

eldesh commented Apr 13, 2020

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

1 participant