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.

Sign up for GitHub

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

Jump to bottom

Use error-chain library #163

Closed
wants to merge 8 commits into from
Closed

Use error-chain library #163

wants to merge 8 commits into from

Conversation

mulkieran
Copy link
Member

Signed-off-by: mulhern amulhern@redhat.com

@mulkieran mulkieran force-pushed the master-error-chain branch 6 times, most recently from baa34ff to a052702 Compare September 13, 2017 13:05
@mulkieran mulkieran force-pushed the master-error-chain branch 2 times, most recently from d0a68c1 to 3f650bd Compare September 13, 2017 14:11
@mulkieran
Copy link
Member Author

mulkieran commented Sep 13, 2017 •
edited
Loading

See Stratis usage at: stratis-storage/stratisd#564.

Edit: That pull no longer contains the changes. I'll be doing another pull tomorrow.

@mulkieran mulkieran changed the title Begin work on using error-chain Use error-chain library Sep 14, 2017
@mulkieran mulkieran requested review from agrover and trgill September 14, 2017 14:21
@mulkieran mulkieran force-pushed the master-error-chain branch 3 times, most recently from 7eacb53 to 34fe428 Compare September 15, 2017 15:00
@agrover
Copy link
Contributor

agrover commented Sep 15, 2017

How about renaming devicemapper::Result back to DmResult using #[error_chain(result = "DmResult")]? When I see Result I think std::result::Result.

@mulkieran
Copy link
Member Author

I think that would be an unusual thing to do. even the standard library has fmt::Result, thread::Result, io::Result, alloc::fmt::Result, collections::fmt::Result and in my sample of (1), that's how dependent crates behave.

@agrover
Copy link
Contributor

agrover commented Sep 15, 2017

This isn't how we're doing it right now, either here or in stratisd. It would be best to introduce error chain in this PR without tying it to the larger style issue of how we name our result types, please.

This was referenced Sep 18, 2017
Closed
Closed
@mulkieran mulkieran force-pushed the master-error-chain branch 6 times, most recently from b22032a to e692c7b Compare September 19, 2017 13:17
@mulkieran
Copy link
Member Author

@agrover @trgill Here's an example backtrace that we get from an unwrap() when the error is an error-chain error and BACKTRACE=1. https://travis-ci.org/stratis-storage/devicemapper-rs/jobs/277303412. Note that it contains the full backtrace of the actual error, although there is also a lot of other probably unavoidable stuff mixed in with it.

@mulkieran mulkieran force-pushed the master-error-chain branch 3 times, most recently from 8d9145b to 51a242c Compare September 20, 2017 18:23
@mulkieran
Copy link
Member Author

Taking advantage of #171 .

@mulkieran
Copy link
Member Author

rebased.

@mulkieran mulkieran force-pushed the master-error-chain branch 5 times, most recently from 3bd4e47 to e3afab7 Compare September 21, 2017 12:32
This was referenced Sep 21, 2017
Merged
Merged
@mulkieran
Unify handling of setup across devices
d59467f 
For code reuse, clarity, uniformity with device creation.

Signed-off-by: mulhern <amulhern@redhat.com>
@mulkieran
Refactor to use error-chain module for errors
ed19894 
To gain the benefits of the library.

* Add a new errors.rs file to contain the macro invocations for error_chain.
* Remove result.rs file, now obsoleted.
* Substitute as appropriate.
* Add some error chains to handle higher level errors.

Signed-off-by: mulhern <amulhern@redhat.com>
@mulkieran
Chain all errors at end of method
c80f78f 
When summarizing the meaning of failures, gather up the first error and then
chain.

Signed-off-by: mulhern <amulhern@redhat.com>
@mulkieran
Add a chain_error() method and use it to simplify the error chaining
8cd499e 
There's a little less obvious function origami and the code is slightly
easier to read.

Signed-off-by: mulhern <amulhern@redhat.com>
@mulkieran
Make dm tests match error kinds
affeaa4 
This makes the tests a bit more precise.

Signed-off-by: mulhern <amulhern@redhat.com>
@mulkieran
Make two error heirarchies
f351de9 
One is for devicemapper core and one is for the outer layer.

Signed-off-by: mulhern <amulhern@redhat.com>
@mulkieran
stash
e58e538
@mulkieran
Simplify error chaining in ThinPoolDev
11a0a88 
Signed-off-by: mulhern <amulhern@redhat.com>
@mulkieran
Copy link
Member Author

rebased.

@mulkieran mulkieran mentioned this pull request Sep 22, 2017
Merged
@mulkieran mulkieran mentioned this pull request Oct 2, 2017
Closed
@agrover
Copy link
Contributor

agrover commented Nov 2, 2017

@mulkieran this PR appears to be obsoleted, if so please close?

@mulkieran
Copy link
Member Author

It needs to be broken out into issues.

@mulkieran
Copy link
Member Author

Likely we will be using faillure crate instead of error-chain, so probably this PR is no longer useful.

@mulkieran mulkieran closed this Nov 27, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@agrover agrover Awaiting requested review from agrover

@trgill trgill Awaiting requested review from trgill

Assignees
No one assigned
Labels
in progress
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@mulkieran @agrover