Raise more specific exceptions for GIT_EEXISTS and GIT_EINVALIDSPEC

robinst · robinst · commit a599470491e4 · 2018-10-19T10:11:48.000+11:00
Before, both would raise ValueError, making it hard to distinguish
between them to show users a meaningful error message.

The new exceptions AlreadyExistsError and InvalidSpecError extend
ValueError, so this change should be backwards-compatible.
diff --git a/docs/general.rst b/docs/general.rst
@@ -59,3 +59,16 @@ Exceptions
    :show-inheritance:
    :undoc-members:
 
+.. autoexception:: pygit2.AlreadyExistsError
+   :members:
+   :show-inheritance:
+   :undoc-members:
+
+Exception when trying to create an object (reference, etc) that already exists.
+
+.. autoexception:: pygit2.InvalidSpecError
+   :members:
+   :show-inheritance:
+   :undoc-members:
+
+Exception when an input specification such as a reference name is invalid.
diff --git a/src/error.c b/src/error.c
@@ -28,6 +28,8 @@
 #include "error.h"
 
 PyObject *GitError;
+PyObject *AlreadyExistsError;
+PyObject *InvalidSpecError;
 
 PyObject *
 Error_type(int type)
@@ -41,7 +43,7 @@ Error_type(int type)
 
         /* A reference with this name already exists */
         case GIT_EEXISTS:
-            return PyExc_ValueError;
+            return AlreadyExistsError;
 
         /* The given short oid is ambiguous */
         case GIT_EAMBIGUOUS:
@@ -53,7 +55,7 @@ Error_type(int type)
 
         /* Invalid input spec */
         case GIT_EINVALIDSPEC:
-            return PyExc_ValueError;
+            return InvalidSpecError;
 
         /* Skip and passthrough the given ODB backend */
         case GIT_PASSTHROUGH:
diff --git a/src/pygit2.c b/src/pygit2.c
@@ -37,6 +37,8 @@
 #include "options.h"
 
 extern PyObject *GitError;
+extern PyObject *AlreadyExistsError;
+extern PyObject *InvalidSpecError;
 
 extern PyTypeObject RepositoryType;
 extern PyTypeObject OidType;
@@ -245,6 +247,14 @@ moduleinit(PyObject* m)
     Py_INCREF(GitError);
     PyModule_AddObject(m, "GitError", GitError);
 
+    AlreadyExistsError = PyErr_NewException("_pygit2.AlreadyExistsError", PyExc_ValueError, NULL);
+    Py_INCREF(AlreadyExistsError);
+    PyModule_AddObject(m, "AlreadyExistsError", AlreadyExistsError);
+
+    InvalidSpecError = PyErr_NewException("_pygit2.InvalidSpecError", PyExc_ValueError, NULL);
+    Py_INCREF(InvalidSpecError);
+    PyModule_AddObject(m, "InvalidSpecError", InvalidSpecError);
+
     /* Repository */
     INIT_TYPE(RepositoryType, NULL, PyType_GenericNew)
     ADD_TYPE(m, Repository)
diff --git a/test/test_refs.py b/test/test_refs.py
@@ -32,8 +32,9 @@
 
 import pytest
 
-from pygit2 import GitError, GIT_REF_OID, GIT_REF_SYMBOLIC, Signature
+from pygit2 import GIT_REF_OID, GIT_REF_SYMBOLIC, Signature
 from pygit2 import Commit, Tree, reference_is_valid_name
+from pygit2 import AlreadyExistsError, GitError, InvalidSpecError
 from . import utils
 
 LAST_COMMIT = '2be5719152d4f82c7302b1c0932d8e5f0a4a0e98'
@@ -376,8 +377,9 @@ def test_create_reference(self):
         assert reference.target.hex == LAST_COMMIT
 
         # try to create existing reference
-        with pytest.raises(ValueError):
+        with pytest.raises(AlreadyExistsError) as error:
             self.repo.create_reference('refs/tags/version1', LAST_COMMIT)
+        assert isinstance(error.value, ValueError)
 
         # try to create existing reference with force
         reference = self.repo.create_reference('refs/tags/version1',
@@ -394,15 +396,24 @@ def test_create_symbolic_reference(self):
         assert reference.target == 'refs/heads/master'
 
         # try to create existing symbolic reference
-        with pytest.raises(ValueError):
+        with pytest.raises(AlreadyExistsError) as error:
             repo.create_reference('refs/tags/beta', 'refs/heads/master')
+        assert isinstance(error.value, ValueError)
 
         # try to create existing symbolic reference with force
         reference = repo.create_reference('refs/tags/beta',
                                           'refs/heads/master', force=True)
         assert reference.type == GIT_REF_SYMBOLIC
         assert reference.target == 'refs/heads/master'
 
+    def test_create_invalid_reference(self):
+        repo = self.repo
+
+        # try to create a reference with an invalid name
+        with pytest.raises(InvalidSpecError) as error:
+            repo.create_reference('refs/tags/in..valid', 'refs/heads/master')
+        assert isinstance(error.value, ValueError)
+
     #   def test_packall_references(self):
     #       self.repo.packall_references()
 
