Throw NotFound and CannotDereference Exceptions - Reborn by nulltoken · Pull Request #842 · libgit2/libgit2sharp

nulltoken · 2014-10-08T22:13:45Z

This change is to throw more specific exceptions in several cases when
possible. If an object cannot be found, throw a more specific
NotFoundException. If an object exists but cannot be dereferenced to its
desired type, then throw a CannotDereferenceException.

This is the rebirth of #839 that has been too hastily merged (then reverted). 😊

nulltoken · 2014-11-10T11:02:55Z

Depends on libgit2/libgit2#2627

nulltoken · 2014-11-23T22:22:18Z

Now that libgit2/libgit2#2718 has been merged, we should be able to more easily distinguish root cause of peeling issues.

Therzok · 2014-12-08T03:01:27Z

Does this need to go on top of #870 ?

jamill · 2014-12-27T23:30:22Z

Looking more closely at this - I don't think the second commit (a75c2c2e) is needed anymore. What we probably need to do instead is update the documentation around InvalidSpecificationException to properly reflect the current meaning of the underlying libgit2 error and the corresponding libgit2sharp exception (either as part of this PR or separately).

nulltoken · 2015-01-06T21:26:23Z

@jamill I hear you and I agree, however I'm a bit "dry" when it comes to enhancing the exception xml doc. Any idea?

FWIW, I've added some code coverage regarding the lookup of wrong revparse expression. Does it feel ok?

nulltoken · 2015-01-07T09:28:49Z

LibGit2Sharp.Tests/RepositoryFixture.cs

I still feel a little bothered that those two above throw different exceptions.

FWIW:

master^{commit}^{blob} throws InvalidSpecificationException

tags/e90810b^{commit}^{blob} throws InvalidSpecificationException

/cc @carlosmn @jamill Thoughts?

Seems fine to me. If an app cares about the difference between the errors, it can tell the user what went wrong more specifically.

I don't know if the ability to discern between these two cases is actually worth the complexity of having two exceptions that are pretty similar (they both mean I can peel the object as requested). However, everything is working as designed in libgit2 - I wouldn't argue to change the current design unless we get feedback that this is confusing.

jamill · 2015-01-20T15:13:42Z

@jamill I hear you and I agree, however I'm a bit "dry" when it comes to enhancing the exception xml doc. Any idea?

I can take a pass at updating the xml docs in a bit. I will tack it on the end of this PR.

nulltoken · 2015-01-20T15:22:08Z

I can take a pass at updating the xml docs in a bit. I will tack it on the end of this PR.

@jamill ❤️

nulltoken · 2015-01-20T22:38:50Z

LibGit2Sharp/InvalidSpecificationException.cs

nulltoken · 2015-01-20T22:39:14Z

Does anyone oppose to see this merged?

Therzok · 2015-01-20T23:02:43Z

I hereby grant you permission to break API!

This change is to throw more specific exceptions in several cases when possible. If an object cannot be found, throw a more specific NotFoundException.

Update documentation on InvalidSpecificationException to better reflect the current meaning of this exception.

nulltoken · 2015-01-21T11:28:03Z

Let's do this, then 💥

Throw NotFound and CannotDereference Exceptions - Reborn

nulltoken mentioned this pull request Oct 8, 2014

Error updates #839

Merged

nulltoken force-pushed the jamill/error_update branch from 49987df to cdc4afc Compare December 22, 2014 16:08

nulltoken added this to the v0.21 milestone Dec 22, 2014

nulltoken force-pushed the jamill/error_update branch from cdc4afc to a75c2ce Compare December 24, 2014 07:12

nulltoken force-pushed the jamill/error_update branch from a75c2ce to 7b10d60 Compare January 6, 2015 21:07

nulltoken force-pushed the jamill/error_update branch from 7b10d60 to 2a72135 Compare January 6, 2015 21:29

nulltoken reviewed Jan 7, 2015
View reviewed changes

nulltoken force-pushed the jamill/error_update branch from f629c71 to f3f96ea Compare January 9, 2015 07:03

nulltoken mentioned this pull request Jan 10, 2015

Expose more low level Index operations #907

Merged

nulltoken reviewed Jan 20, 2015
View reviewed changes

LibGit2Sharp/InvalidSpecificationException.cs

Copy link

Member Author

nulltoken Jan 20, 2015

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

jamill and others added 4 commits January 21, 2015 12:20

Throw more meaningful NotFoundException when appropriate

23f97d9

This change is to throw more specific exceptions in several cases when possible. If an object cannot be found, throw a more specific NotFoundException.

Enhance Repository.Lookup() test coverage

fc2a47c

Introduce PeelException

1a3249c

Update documentation on InvalidSpecificationException

24446f5

Update documentation on InvalidSpecificationException to better reflect the current meaning of this exception.

nulltoken force-pushed the jamill/error_update branch from 30485a4 to 24446f5 Compare January 21, 2015 11:20

nulltoken added a commit that referenced this pull request Jan 21, 2015

Merge pull request #842 from libgit2/jamill/error_update

05e81e7

Throw NotFound and CannotDereference Exceptions - Reborn

nulltoken merged commit 05e81e7 into vNext Jan 21, 2015

nulltoken deleted the jamill/error_update branch January 21, 2015 11:28

Conversation

nulltoken commented Oct 8, 2014

Uh oh!

nulltoken commented Nov 10, 2014

Uh oh!

nulltoken commented Nov 23, 2014

Uh oh!

Therzok commented Dec 8, 2014

Uh oh!

jamill commented Dec 27, 2014

Uh oh!

nulltoken commented Jan 6, 2015

Uh oh!

nulltoken Jan 7, 2015

Choose a reason for hiding this comment

Uh oh!

carlosmn Jan 13, 2015

Choose a reason for hiding this comment

Uh oh!

jamill Jan 20, 2015

Choose a reason for hiding this comment

Uh oh!

nulltoken Jan 20, 2015

Choose a reason for hiding this comment

Uh oh!

jamill commented Jan 20, 2015

Uh oh!

nulltoken commented Jan 20, 2015

Uh oh!

nulltoken Jan 20, 2015

Choose a reason for hiding this comment

Uh oh!

nulltoken commented Jan 20, 2015

Uh oh!

Therzok commented Jan 20, 2015

Uh oh!

nulltoken commented Jan 21, 2015

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Comments