Thoroughly fix the error handling #22
Conversation
* Remove references to the old approach. * Define the outline of the new approach. * Apply it everywhere. * Move some error-handling code from futures to error. * Define an injection for FFI NulError. * Define WriteType as a nice modern enum. * Enhance the enhance_nullary_enum macro to cope with omitted variants. Signed-off-by: Keith Wansbrough <Keith.Wansbrough@metaswitch.com>
Signed-off-by: Keith Wansbrough <Keith.Wansbrough@metaswitch.com>
This has new names for the missing symbols in error-result. It also removes the to_result / error-chain handling; that rightfully belongs in this crate not there, and it was polluting the namespace. Signed-off-by: Keith Wansbrough <Keith.Wansbrough@metaswitch.com>
Signed-off-by: Keith Wansbrough <Keith.Wansbrough@metaswitch.com>
Cassandra errors are now just reported as such; they don't get string wrappers. Signed-off-by: Keith Wansbrough <Keith.Wansbrough@metaswitch.com>
Signed-off-by: Keith Wansbrough <Keith.Wansbrough@metaswitch.com> * Remove almost all uses of expect, unwrap, panic, unimplemented, etc. * Correct a few cases where the wrong error was being reported. All errors should now be cassandra::Errors. * Improvements to error construction helpers. * Wrap the ValueType type.
Signed-off-by: Keith Wansbrough <Keith.Wansbrough@metaswitch.com>
Signed-off-by: Keith Wansbrough <Keith.Wansbrough@metaswitch.com>
|
(now rebased with correct signed-off-by lines!) |
| @@ -124,22 +119,37 @@ impl Debug for Column { | |||
| CASS_VALUE_TYPE_TIMEUUID => write!(f, "TIMEUUID Cassandra type"), | |||
| CASS_VALUE_TYPE_INET => write!(f, "INET Cassandra type"), | |||
| CASS_VALUE_TYPE_LIST => { | |||
| for item in self.set_iter().expect("a list should always be able to return a set iterator") { | |||
| write!(f, "LIST {:?}", item)? | |||
| match self.set_iter() { | |||
There was a problem hiding this comment.
Don't worry too much about these - I make a better fix to them in the next PR, #23
| cass_result_column_name(self.0, index, name, name_length); | ||
| let slice = slice::from_raw_parts(name as *const u8, name_length as usize); | ||
| str::from_utf8(slice).expect("must be utf8").to_owned() | ||
| cass_result_column_name(self.0, index, name, name_length).to_result(()) |
There was a problem hiding this comment.
Spot the bug - fixed in #23
There was a problem hiding this comment.
Surely the real solution is to move everything into Rust and stop using Unsafe code? 😈
|
Note to self - fix Cargo.toml to point to the new 0.10 library once it's merged. |
|
Oh - do we also need a version bump, if we've an API breaking change? |
|
I've not yet released the previous breaking change. I'm holding off so I can release them all together, as v0.9.0. (hence this is -pre1). I guess technically I could have bumped to -pre2? But it doesn't really matter at this stage, especially since versions are ignored by Cargo on git dependencies. |
|
Thanks Angus. TODO: depend on updated cassandra-sys crate, then merge. |
Signed-off-by: Keith Wansbrough <Keith.Wansbrough@metaswitch.com>
This is a thorough root-and-branch replacement of all the error handling with something sane.
There is now a single error type defined in errors.rs, which has variants for plain Cassandra errors (just a return code), detailed errors (with additional info), internal errors (currently only "unexpected type") or various foreign errors (in particular, problems converting strings to/from C).
This error type is used appropriately everywhere, and has appropriate conversions as necessary.
In addition I've audited and removed all (well, almost all) uses of
unwrap,expect,panic!,unimplemented, and friends, replacing them with principled use of?andResult. There are a few that remain where they are appropriate, and one (Statement::new) I'm not 100% happy with but think it best to leave.This is very much a breaking API change - several return types have been changed (from
TtoResult<T, Error>, or fromResult<T, WrongError>toResult<T, Error>, usually, though some other changes have been made too).Some other choice details:
enhance_nullary_enummacro to cope with omitted variants. We don't wantCASS_OKorCASS_ERROR_LAST_ENTRYto count as errors which you need to consider inmatch; we should ensure these just never happen.futurestoerrormodule. Add some other nice error building helpers. Remove direct tests ofCASS_OKelsewhere and depend on helpers inerrormodule.to_result). Also add some missing symbols while we're at it..chain_err(|| "pointless text"); the Cassandra errors speak for themselves and don't need the addition of an extra wrapper.Enjoy!