Conversation
Also restrict to prevent user implementation
| Error::new_with_cause( | ||
| ErrorKind::Unavailable, | ||
| "seeding a new RNG failed: both OS and Jitter entropy sources failed", | ||
| e1) |
There was a problem hiding this comment.
Why does error handling seem to be to most difficult part again and again ;-)?
Good idea to report e1 from OsRng as the error. That seems to be the most important one.
But not reporting the error cause from JitterRng is not great...
One option is to implement a struct TwoCauses and implement a custom stdError::description and other machinery for that. Maybe overkill though.
There was a problem hiding this comment.
If Error::cause returned an iterator instead of an Option it would be easier :(
A custom description implementation doesn't cover it very well either, because (a) it requires formatting two messages into one (long) string somehow and (b) if both causes have sub-causes there are even more problems.
Easier to log/print something directly.
|
I think |
'error: An unknown error occurred'
I changed my mind about renaming as mentioned in #17: having "new" in the name is important to clarify that this provides a "new" function.
To prevent confusion, this completely prevents any user implementation of the
NewSeededtrait, by making it only applicable to types implementingSeedFromRnganyway (from_rngmay migrate toSeedableRngbut that's a separate issue).As @pitdicker put it elsewhere, this is supposed to "make it easy to do the right thing". If users or PRNG authors want a different seeding function, they can use a different trait/function (there's no reason another trait can't also provide a
newfunction).Since @nagisa @newpavlov @burdges commented on #17 I'll give them a chance to disagree here, but I don't find any of the arguments against this I've seen so far convincing.
(Note: the TODO comments are intended to stay in the source for now; #43 can implement one of them.)