CHANGE: Add error handling for propogating failures

[lsampras]

A change request is considered a (small) Request-For-Comment, and requires a concrete proposal and motivation.

Summary

Add error handling so that functions can return errors whenever something goes wrong e.g incorrect inputs

Details

Currently this package uses asserts & unwrap's liberally, I want functions to instead return Results so that the callers can call the underlying functions and handle any errors that they receive.

I'd like to start with a few set of commonly used public functions and add validations there which can return errors when failed...
We could later use thiserror+errorstack to build out a more graceful error handling & error stack

Motivation

What is the motivation for this change?

While unwrap's & asserts might suffice for CLI tools, when using this crate in Web applications or deployed services, especially where the input to these functions would be externallly provided... It is imperative that the application does not crash due to any weird user input...

One such case could be that we take in a lo & hi number from user & generate a random number between them using

gen_range(lo..hi)
# but this will crash if lo >= hi
due to the following assertion in library
assert!(!range.is_empty(), "cannot sample empty range");

In such scenarios it isn't the right expectation to ask the callers to validate the input for all such variants

Alternatives

Which alternatives might be considered, and why or why not?

Depend on the caller to validate all such constraints, but keeping a track of our invariants on which we assert is also tricky

[vks]

Could you please be more specific which functions you would like to return a Result? In rand_distr, most distributions use results, and in Rand 0.9, Uniform will as well, so you will be able to use Uniform::new or Uniform::try_from if you want a Result.

gen_range is supposed to be "quick and easy", which is why it can panic, similar to array indexing.

[lsampras]

oh cool, makes sense,
I was following the rust cookbook examples.
Did not know there was an alternative to doing it this way...

I'll do an evaluation of the functions I'm using & point out if there are missing safe abstractions for any...

Is there an way where I can reason about which functions are safe for production & which are meant for "quick & easy" use ?

[dhardy]

To be fair, the last 0.8.x release is about 17 months old. We've made a bunch of changes since then for the 0.9 release but haven't given the project sufficient focus to get that done yet.

Is there an way where I can reason about which functions are safe for production & which are meant for "quick & easy" use ?

All the public API (at least, if not documented otherwise) should be safe for production use.

The Rng trait provides "quick and easy" functions which may have less performance with repeated sampling, but is still fully usable (e.g. we use Rng::gen_range in the shuffling algorithms if I recall correctly; these need a new range each time so don't benefit from constructing a Uniform distibution first anyway).

[lsampras]

Hey,
using Uniform::try_from also causes panics (sample code in playground ),

Can we fix this?
I can help with a couple of PR's, Although I don't know the extent of changes here...

[vks]

@lsampras I think the Playground uses Rand 0.8, so Uniform::try_from is using the default impl, which falls back to Uniform::try. Therefore, the behavior you are observing is expected.

If you try the current master of Rand, your example will work as expected. Since we did not have explicit tests for this, I added some in PR #1331.

[vks]

I believe everything mentioned here was addressed. Please feel free to reopen if you think gen_range or some variant of it should not panic.