Use TryRngCore for random_from_rng

[thomaseizinger]

With the update to rand 0.9, the Rng and CryptoRng traits gained fallible counter-parts: TryRng and TryCryptoRng. Randomess generation from sources such as the OS-based rng but also hardware rngs involve IO and can thus fail.

It appears to me that the APIs of EphemeralSecret, ReusableSecret as well as StaticSecret could be improved by relying on TryCryptoRng instead of of CryptoRng. This allows users of the random_from_rng function to gracefully handle this error (instead of panicking). The convenience random function retains its API; we simply now unwrap ourselves instead of using the unwrap_err adapter from rand.

[tarcieri]

The general rule I’ve used elsewhere is:

• if an API is already fallible, use fallible RNG
• if an API is otherwise infallible, use infallible RNG

The reason is, in practice, the RNG will never fail on most platforms. The potential for failures are a small corner case of fallback implementations on legacy platforms.

I don’t think it makes sense to turn every RNG API under the sun to be fallible just for the sake of those exotic platforms.

But if an API is already fallible, in such cases you don’t want to panic if the API could propagate the error instead.

Another option is to add a parallel set of try_ APIs.

[burdges]

Also, exotic situations could've wrappers that impl Rng traits and handle the failures, but pass the failures in the try form, so high up code could add panics for no reason.

[thomaseizinger]

Another option is to add a parallel set of try_ APIs.

That seems a bit redundant to me, given that we already have the random convenience function.

The reason is, in practice, the RNG will never fail on most platforms. The potential for failures are a small corner case of fallback implementations on legacy platforms.

Fair. To be honest, I don't know the likelihood of failure here. It just occurred to me that the APIs could be more composable when dealing with the dependency upgrade.

I don't feel strongly at this stage. Time will tell if we receive any crash reports from users!

[tarcieri]

IMO it’s actually more “composable” to use unwrap_err to convert a TryCryptoRng into a CryptoRng than it is to expect everyone who currently has the convenience of an infallible API currently to have to add an expect to handle potential errors that, on most mainstream platforms, will never actually happen in practice

[thomaseizinger]

IMO it’s actually more “composable” to use unwrap_err to convert a TryCryptoRng into a CryptoRng than it is to expect everyone who currently has the convenience of an infallible API currently to have to add an expect to handle potential errors that, on most mainstream platforms, will never actually happen in practice

Wouldn't users who are fine with panicking or are on such mainstream platforms not just use the random function?

[tarcieri]

They can, yes.

(Really at the end of the day I think the whole addition of fallible RNGs is a misadventure that’s added undue complexity)