Forward exceptions from Article's virtual methods

libzim/lib.cxx

@@ -65,8 +65,10 @@ std::string ZimArticleWrapper::callCythonReturnString(std::string methodName) co
     int error;
 
     std::string ret_val = string_cy_call_fct(this->m_obj, methodName, &error);
-    if (error)
+    if (error && error == 1)
         throw std::runtime_error("The pure virtual function " + methodName + " must be override");
+    if (error)
+        throw std::runtime_error("The virtual function " + methodName + " threw an exception");
 
     return ret_val;
 }
@@ -79,8 +81,10 @@ zim::Blob ZimArticleWrapper::callCythonReturnBlob(std::string methodName) const
     int error;
 
     zim::Blob ret_val = blob_cy_call_fct(this->m_obj, methodName, &error);
-    if (error)
+    if (error && error == 1)
         throw std::runtime_error("The pure virtual function " + methodName + " must be override");
+    if (error)
+        throw std::runtime_error("The virtual function " + methodName + " threw an exception");
 
     return ret_val;
 }
@@ -93,8 +97,10 @@ bool ZimArticleWrapper::callCythonReturnBool(std::string methodName) const
     int error;
 
     bool ret_val = bool_cy_call_fct(this->m_obj, methodName, &error);
-    if (error)
+    if (error && error == 1)
         throw std::runtime_error("The pure virtual function " + methodName + " must be override");
+    if (error)
+        throw std::runtime_error("The virtual function " + methodName + " threw an exception");
 
     return ret_val;
 }
@@ -107,8 +113,10 @@ uint64_t ZimArticleWrapper::callCythonReturnInt(std::string methodName) const
     int error;
 
     int ret_val = int_cy_call_fct(this->m_obj, methodName, &error);
-    if (error)
+    if (error && error == 1)
         throw std::runtime_error("The pure virtual function " + methodName + " must be override");
+    if (error)
+        throw std::runtime_error("The virtual function " + methodName + " threw an exception");
 
     return ret_val;
 }

libzim/wrapper.pyx

@@ -119,26 +119,42 @@ cdef public api:
     string string_cy_call_fct(object obj, string method, int *error) with gil:
         """Lookup and execute a pure virtual method on ZimArticle returning a string"""
         func = get_article_method_from_object(obj, method, error)
-        ret_str = func()
+        try:
+            ret_str = func()
+        except Exception:
+            error[0] = 2
+            raise
         return ret_str.encode('UTF-8')
 
     wrapper.Blob blob_cy_call_fct(object obj, string method, int *error) with gil:
         """Lookup and execute a pure virtual method on ZimArticle returning a Blob"""
         cdef WritingBlob blob
 
         func = get_article_method_from_object(obj, method, error)
-        blob = func()
+        try:
+            blob = func()
+        except Exception:
+            error[0] = 2
+            raise
         return dereference(blob.c_blob)
 
     bool bool_cy_call_fct(object obj, string method, int *error) with gil:
         """Lookup and execute a pure virtual method on ZimArticle returning a bool"""
         func = get_article_method_from_object(obj, method, error)
-        return func()
+        try:
+            return func()
+        except Exception:
+            error[0] = 2
+            raise
 
     uint64_t int_cy_call_fct(object obj, string method, int *error) with gil:
         """Lookup and execute a pure virtual method on ZimArticle returning an int"""
         func = get_article_method_from_object(obj, method, error)
-        return <uint64_t> func()
+        try:
+            return <uint64_t> func()
+        except Exception:
+            error[0] = 2
+            raise
 
 cdef class Creator:
     """ Zim Creator

tests/test_libzim.py

@@ -16,9 +16,12 @@
 # You should have received a copy of the GNU General Public License
 # along with this program. If not, see <http://www.gnu.org/licenses/>.
 
-import pytest
+import sys
+import subprocess
 import pathlib
 
+import pytest
+
 from libzim.writer import Article, Blob, Creator
 from libzim.reader import File
 
@@ -188,3 +191,63 @@ def test_filename_param_types(tmpdir):
     with Creator(str(path), "welcome") as creator:
         assert creator.filename == path
         assert isinstance(creator.filename, pathlib.Path)
+
+
+def test_in_article_exceptions(tmpdir):
+    """ make sure we raise RuntimeError from article's virtual methods """
+
+    class BoolErrorArticle(SimpleArticle):
+        def is_redirect(self):
+            raise IOError
+
+    class StringErrorArticle(SimpleArticle):
+        def get_url(self):
+            raise IOError
+
+    class BlobErrorArticle(SimpleArticle):
+        def get_data(self):
+            raise IOError
+
+    path, main_page = tmpdir / "test.zim", "welcome"
+    args = {"title": "Hello", "mime_type": "text/html", "content": "", "url": "welcome"}
+
+    with Creator(path, main_page) as zim_creator:
+        # make sure we can can exception of all types (except int, not used)
+        with pytest.raises(RuntimeError):
+            zim_creator.add_article(BoolErrorArticle(**args))
+        with pytest.raises(RuntimeError):
+            zim_creator.add_article(StringErrorArticle(**args))
+        with pytest.raises(RuntimeError):
+            zim_creator.add_article(BlobErrorArticle(**args))
+
+    # make sure we can catch it from outside creator
+    with pytest.raises(RuntimeError):
+        with Creator(path, main_page) as zim_creator:
+            zim_creator.add_article(BlobErrorArticle(**args))
+
+
+def test_dontcreatezim_onexception(tmpdir):
+    """ make sure we can prevent ZIM file creation (workaround missing cancel())
+
+        A new interpreter is instanciated to get a different memory space.
+        This workaround is not safe and may segfault at GC under some circumstances
+
+        Unless we get a proper cancel() on libzim, that's the only way to not create
+        a ZIM file on error """
+    path, main_page = tmpdir / "test.zim", "welcome"
+    pycode = f"""
+from libzim.writer import Creator
+from libzim.writer import Article
+class BlobErrorArticle(Article):
+    def get_data(self):
+        raise ValueError
+zim_creator = Creator("{path}", "{main_page}")
+try:
+    zim_creator.add_article(BlobErrorArticle(**args))
+except Exception:
+    zim_creator._closed = True
+"""
+
+    py = subprocess.run([sys.executable, "-c", pycode])
+    assert py.returncode == 0
+    assert not path.exists()