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

Implicit error handling #25

Closed
1 of 3 tasks
dhardy opened this issue Oct 30, 2017 · 8 comments
Closed
1 of 3 tasks

Implicit error handling #25

dhardy opened this issue Oct 30, 2017 · 8 comments

Comments

@dhardy
Copy link
Owner

dhardy commented Oct 30, 2017

We currently have these error kinds (#9):

/// Permanent failure: likely not recoverable without user action.
Unavailable,
/// Temporary failure: recommended to retry a few times, but may also be
/// irrecoverable.
Transient,
/// Not ready yet: recommended to try again a little later.
NotReady,
/// Uncategorised error
Other,

next_u32 etc. and fill_bytes cannot report an error except via panic. Instead, these functions should probably:

  • block if NotReady
  • retry a few times if Transient

Additionally,


There is some room for discussion here (especially how to handle Transient), but mostly this issue is to track implementation (PRs welcome).

@dhardy
Copy link
Owner Author

dhardy commented Oct 30, 2017

Specifically, see the impl of Rng for ReadRng and OsRng which both currently unwrap errors.

@burdges
Copy link

burdges commented Oct 30, 2017

I think our lacking an associated error type will hamper error propagation in many situations where you'd actually care about the error. I asked a summary so far question to help document this and other compromises #29 but I'm kinda guessing there.

@dhardy
Copy link
Owner Author

dhardy commented Oct 30, 2017

@burdges (1) this point is about handling specific errors, not about propagating errors (#8 and #10 are more on topic for that), and (2) if we had an associated error type then we can have no generic handling (e.g. from_rng must either use fill_bytes for implicit error handling but panic, or try_fill_bytes to allow error propagation but do no implicit handling), so associated types make the situation worse.

@burdges
Copy link

burdges commented Nov 8, 2017

It's worth glancing at the boats' new failure crate (via), but it fails no_std projects immediately so not ideal here.

@dhardy
Copy link
Owner Author

dhardy commented Nov 9, 2017

@burdges you're off topic in this issue for the second time. See also this comment; failure doesn't help much with this problem.

@dhardy
Copy link
Owner Author

dhardy commented Nov 9, 2017

#35 implements this for reseeding

@dhardy
Copy link
Owner Author

dhardy commented Nov 9, 2017

Note: interrupt handling is only required for std::io::Read; both implementations use read_exact which already retries on interrupt.

dhardy added a commit that referenced this issue Nov 9, 2017
Add rand_core::impls::fill_via_try_fill with handling for errors.
dhardy added a commit that referenced this issue Nov 10, 2017
Add rand_core::impls::fill_via_try_fill with handling for errors.
@dhardy
Copy link
Owner Author

dhardy commented Jan 22, 2018

It seems any implicit handling is being done locally (context-dependent), so no need for this issue.

@dhardy dhardy closed this as completed Jan 22, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants