Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

Consider adding an error code to `LibGit2Exception` #137

Closed
half-ogre opened this Issue · 8 comments

3 participants

@half-ogre

I was writing code to catch GIT_ENOTAREPO to check if the current directory was a GIt repo or not, and it seemed like it would be nice to add a nullable error code property to LibGit2Exception, and in Ensure.Success set the property to the error code that comes from the native call. I'm happy to send a pull request, but figure I should raise it for discussion first.

@nulltoken
Owner

Would indeed seem like a good idea.

Unfortunately, there's a slight issue: libgit2 is currently revamping its error handling mechanism (this is being done in the current active branch new-error-handling. It will eventually be merged back into the main development branch, once the code of every function has been updated to the new mechanism. Shouldn't be long, now). Next version of LibGit2Sharp v0.9.0 will be released as soon as it's done.

You can peek at the initial draft of this rework @ docs/error-handling.md.

Basically, a function will return 0 if everything was ok, -1 if it didn't behave as expected. Additionally, query/lookup methods would be able to return GIT_ENOTFOUND when the requested "thing" hasn't been found.

However, from what I've seen, the current rework slightly differs from the documentation. There's no git_error **error out parameter. Instead, a call to the new function git_error_last() retrieves the detailed error whenever a function returns a negative result. You can peek at some tests describing how it is intended to be used.

The current error codes are supposed to disappear. Instead, a klass property would expose a git_error_class which would roughly indicate what module the error originates from or what it relates to. This should help us create more specific exception. However those ones would still be coarsed-grained ones (more like IOException rather than DirectoryNotFoundException).

@tanoku please feel free to correct me if I've misunderstood anything.

@davidfowl @tclem @xpaulbettsx @dahlbyk Any thoughts?

@Haacked
Owner

Interesting. However, that doesn't necessarily mean we need to change the C# API, right? It just means that the implementation of raising a Git exception from LibGit2Sharp will have to call the libgit2 C API differently. I wouldn't want to impose a GetLastError() method on C# API consumers.

I like the idea of having LibGit2Sharp automatically call git_error_last and exposing the code on the exception object. Right now I have code that parses out the text GIT_ENOTFOUND from an exception message and it made me feel a bit dirty. A quick shower remedied that quickly. :smile:

@nulltoken
Owner

@Haacked

Interesting. However, that doesn't necessarily mean we need to change the C# API, right?

No API will be harmed in the process.

call git_error_last and exposing the code on the exception object.

Beware that those error codes are going away. Current re-implementation will only allow us to add some new types of Exceptions (deriving from LibGit2Exception) which would more or less allow the caller to catch "Something went wrong with a [Repository|Reference|Tag|Tree]...".

@anglicangeek

I was writing code to catch GIT_ENOTAREPO to check if the current directory was a GIt repo or not

Depending on the implementation of git_repository_open(), which might (or not. @arrbee?) return GIT_ENOTFOUND if the path does not lead to a proper workdir or gitdir, we could intercept this and throw a proper TheNumberYouHaveDialedIsNotInServicePleaseCheckTheNumberAndTryAgainException (or a better named one).

@half-ogre

@nulltoken That's what I was thinking, too. I was going to look to see how feasible doing so would be when I'm back to that code this weekend. If I can manage something that looks and feels right I'll send a PR.

@nulltoken
Owner

@anglicangeek

  • current LibGit2Sharp master (latest released branch) points at this version
  • current LibGit2Sharp vNext (development branch) points at this version
  • current libgit2 new-error-handling branch version
@nulltoken nulltoken referenced this issue in libgit2/libgit2
Merged

Improve the interop with bindings #674

@nulltoken
Owner

@drewthedork latest libgit2 has landed in the vNext branch. git_repository_open() should now return GIT_ENOTFOUND

@half-ogre

@nulltoken Awesome, and thanks. Sorry I haven't been able to contribute yet; (paying) work stuff has been higher priority.

@nulltoken nulltoken closed this in 9d09fe9
@nulltoken
Owner

@half-ogre Sir, the requested RepositoryNotFoundException has just been served!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.