octocrab
octocrab copied to clipboard
fix(installation)!: Return Result instead of panicking
As #641 describes, returning a Result when calling Octocrab::installation is nicer than panicking outright.
I'm definitely not happy with how I build and return the Errors - is there a better way to build Errors directly?
This PR changes the public API behavior, since Octocrab::installation is exposed to the user.
@XAMPPRocky Sorry for pinging you 😀 Is there any better way to create custom errors in octocrab?
I'm not really happy with the way I did it as a first quick draft:
let source = std::io::Error::new(
std::io::ErrorKind::Other,
"Github App authorization is required to target an installation",
);
return Err(Error::Other {
backtrace: Backtrace::generate_with_source(&source),
source: Box::new(source),
});
@benpueschel We use the snafu crate currently for custom errors, so you can use that.
https://docs.rs/snafu/latest/snafu/
I may be missing something here - does snafu provide a way to conveniently build the Error::Other variant?
If it doesn't, I could maybe add a new Error type (though I don't really like that - it's both not elegant and would introduce another breaking change for anyone matching on the octocrab Error type).
If it doesn't, I could maybe add a new Error type (though I don't really like that - it's both not elegant and would introduce another breaking change for anyone matching on the octocrab Error type).
I would add an error variant. We should change the error enum to be #[non_exhaustive] to make that non breaking.
Sounds good.
Thank you for your PR!