Replaced exception handling logic #998
Conversation
|
Let's see an exemple. Today we have: err = C.git_clone(crepo, to_bytes(url), to_bytes(path), opts)
check_error(err)The proposal is to write: GitException.check_result(C.git_clone)(crepo, to_bytes(url), to_bytes(path), opts)Today's code already transforms the error code to a Python exception, and it's in my opinion easier to read. The thing the new code does is to catch and re-raise an exception from What happens, as seen in issue #996, is that a callback written in Python may raise an exception. Then the callback has to store that exception somewhere ( For the record, from libgit2 sources: Then the code that has called
But the PR doesn't address this actually. The code handling It looks to me that this PR is actually trying to resolve issue #830, not #996. Am I right? |
Yes, absolutely. Maybe I should have been more clear about that. When looking into the problem as discussed in #996 I realized that before we can handle that it would make sense to first have a more generic exception handling implemented as suggested in #830. The thing that this PR add's, I hope, is to implement it and thus provides us a single place where all errors - Python and C type - can be handled. Because of this we also get a single place where special error codes like I have not yet integrated any logic that would specially handle |
|
Okay. The callback code was contributed long time ago by someone else, I myself didn't know exactly how it worked. So I've reviewed it and done some commits. It's all internal refactoring, no external changes (not intended at least). Now it should be much easier to read and maintain, at least now I understand how it works. You can give a look at the new I don't think there's any dependency between issues #830 and #996. There're many calls to Next I'll work on the unit tests concerning #996 (not completely sure, but I think that issue is already fixed). |
As discussed in #830 and #996 this is a initial implementation of a new exception handling mechanism. The goals so far have been:
GitExceptionand try to populate it with as much information as available.As suggested / discussed in #830 it was implemented using mutliple inheritance.
Currently the tree looks like this
Exception -> GitError (for backward compatibility) -> BaseGitException (for multi inheritance support and new exception handling logic) -> GitException (as actual class to derive from) -> GitXxxError (Exceptions matching GIT_E* exit codes from libgit2).The new code is in errors.py - the other classes have only been touched to implement it. A single unittest was changed to be more specific when detecting the returned exception, thought that change should not break anything.
There are multiple ways to use it.