Return Result from Server methods
#25
Conversation
Deletes a useless dummy test, created by `cargo new`.
|
I'm excited about this change! It looks like it incorporates moving a big chunk of code besides the change from |
Yes, good idea! |
tgeoghegan
force-pushed
the
timg/return-result-2
branch
from
00db7ce
to
6adcf33
Compare
|
There you are @jsha: I will move the structs and functions from |
src/util.rs
Outdated
| /// Input sizes do not match | ||
| #[error("input sizes do not match")] | ||
| InputSizeMismatch, | ||
| } |
There was a problem hiding this comment.
The name "ProofError" sound as if this might be returned if the input-validation protocol failed. Looking at the error that's propagated, this seems to happen only when "unpacking" a proof. Perhaps this is better cast as another "SerializeError" type error? Something like "SerializeError::UnpackInputSizeMismatch"?
There was a problem hiding this comment.
That's a good point. The tricky thing is that I want to make a follow up PR where I move this enum, struct UnpackedProof, struct UnpackedProofMut, unpack_proof and unpack_proof_mut to a new module proof.rs (the idea is to remove stuff from util.rs with the eventual goal of deleting that module). But arguably, all the stuff here in util.rs has to do with serialization, so maybe instead of proof.rs my later PR should just rename util.rs to serialize.rs...
OK, given that we want to revisit serialization in the near future anyway (#24), I think I'm just going to punt on moving anything from util.rs into a new module, and for now I will add a case to SerializeError as you suggest.
There was a problem hiding this comment.
IMO serialize is a better name for the stuff in this package than proof.
Various methods on `Server` used to return `Option<T>`, making it possible for them to fail but not to indicate why. This means `prio-server` can't tell the difference between a packet being rejected due to a decryption error or because the decrypted payload was invalid, causing frustrating situations as described in [1]. We now return `Result<T, ServerError>` from `Server`'s methods. In support of this, this commit also includes some other cleanups: - the functions `unpack_proof` and `unpack_proof_mut` now also return errors - a number of `use module::*` style statements are rewritten to be more explicit. [1] divviup/prio-server#550
tgeoghegan
force-pushed
the
timg/return-result-2
branch
from
0d1bad9
to
24ac8dd
Compare
The remaining methods on
Serverwhich returnedOption<Something>now returnResult<Something, ServerError>, to make it easier forprio-serverto tell errors apart (#550).