search

EGA-archive / crypt4gh-rust

Rust implementation for the Crypt4GH encryption format
https://ega-archive.github.io/crypt4gh-rust
Apache License 2.0
5 stars 2 forks source link

Returning a `Result` rather than using `assert!` for error conditions #2

Closed mmalenic closed 9 months ago

mmalenic commented 1 year ago

There's a few places where an assert! is used to check for a condition instead of a Result returning an Err. It might be nice to have these assert!s converted to Crypt4GHErrors.

For example: https://github.com/EGA-archive/crypt4gh-rust/blob/ce0c33695aac6f9275654deb7a263f93cf2b6e1a/src/header.rs#L319-L336 could return something like a Crypt4GH::MagicStringError if the magic string is not correct.

This would give the opportunity for library consumers of the crate to choose what to do when encountering an invalid Crypt4GH file.

I'd be happy to fork/PR some changes for this.

/cc @brainstorm

brainstorm commented 1 year ago

Good catch! As you know I'm currently transitioning away from sodiumoxide towards RustCrypto in our fork here: https://github.com/umccr/crypt4gh-rust. So lets incorporate this change you propose as well, also fix #1 and also potentially introduce API-breaking changes on the public interface such as not having buffers as function arguments, but other builder-type constructs that are more idiomatic that we briefly discussed a couple of weeks ago.

brainstorm commented 1 year ago

I think I've taken care of most asserts, there's only a few remaining in #3.