diff --git a/libzim/lib.cxx b/libzim/lib.cxx index 58af0a11..04825431 100644 --- a/libzim/lib.cxx +++ b/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; } diff --git a/libzim/wrapper.pyx b/libzim/wrapper.pyx index 5f3a9b16..e04fcab3 100644 --- a/libzim/wrapper.pyx +++ b/libzim/wrapper.pyx @@ -119,7 +119,11 @@ 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: @@ -127,18 +131,30 @@ cdef public api: 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 func() + try: + return func() + except Exception: + error[0] = 2 + raise cdef class Creator: """ Zim Creator diff --git a/tests/test_libzim.py b/tests/test_libzim.py index 9552baa8..9837148d 100644 --- a/tests/test_libzim.py +++ b/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 . -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()