Merge pull request #106 from giordano/mg/error-msg

Improve reporting of errors in documents

Project.toml

 name = "SBML"
 uuid = "e5567a89-2604-4b09-9718-f5f78e97c3bb"
 authors = ["Mirek Kratochvil <miroslav.kratochvil@uni.lu>", "LCSB R3 team <lcsb-r3@uni.lu>"]
-version = "0.5.4"
+version = "0.5.5"
 
 [deps]
 Libdl = "8f399da3-3557-5675-b5ff-fb832c97cbdb"

src/SBML.jl

@@ -7,10 +7,10 @@ using Symbolics
 include("structs.jl")
 include("version.jl")
 
+include("utils.jl")
 include("readsbml.jl")
 include("converters.jl")
 include("math.jl")
-include("utils.jl")
 include("symbolics.jl")
 
 sbml(sym::Symbol) = dlsym(SBML_jll.libsbml_handle, sym)

src/converters.jl

@@ -6,16 +6,11 @@ A converter to pass into [`readSBML`](@ref) that enforces certain SBML level
 and version.
 """
 set_level_and_version(level, version) =
-    doc -> begin
-        ccall(
-            sbml(:SBMLDocument_setLevelAndVersion),
-            Cint,
-            (VPtr, Cint, Cint),
-            doc,
-            level,
-            version,
-        ) == 0 && throw(ErrorException("Setting of level and version did not succeed"))
-    end
+    doc -> check_errors(ccall(sbml(:SBMLDocument_setLevelAndVersion), Cint,
+                              (VPtr, Cint, Cint),
+                              doc, level, version),
+                        doc,
+                        ErrorException("Setting of level and version did not succeed"))
 
 """
     libsbml_convert(conversion_options::Vector{Pair{String, Dict{String, String}}})
@@ -43,8 +38,8 @@ libsbml_convert(
                     opt,
                 )
             end
-            ccall(sbml(:SBMLDocument_convert), Cint, (VPtr, VPtr), doc, props) == 0 &&
-                throw(ErrorException("Conversion returned errors"))
+            check_errors(ccall(sbml(:SBMLDocument_convert), Cint, (VPtr, VPtr), doc, props),
+                         doc, ErrorException("Conversion returned errors"))
         end
     end
 

src/readsbml.jl

-
-const VPtr = Ptr{Cvoid}
-
 """
     get_string(x::VPtr, fn_sym)::Maybe{String}
 
@@ -96,15 +93,8 @@ end)
 function readSBML(fn::String, sbml_conversion = document -> nothing)::SBML.Model
     doc = ccall(sbml(:readSBML), VPtr, (Cstring,), fn)
     try
-        n_errs = ccall(sbml(:SBMLDocument_getNumErrors), Cuint, (VPtr,), doc)
-        for i = 0:n_errs-1
-            err = ccall(sbml(:SBMLDocument_getError), VPtr, (VPtr, Cuint), doc, i)
-            msg = strip(get_string(err, :XMLError_getMessage))
-            @error "SBML reported error: $msg"
-        end
-        if n_errs > 0
-            throw(AssertionError("Opening SBML document has reported errors"))
-        end
+        get_error_messages(doc,
+                           AssertionError("Opening SBML document has reported errors"))
 
         sbml_conversion(doc)
 

src/utils.jl

+const VPtr = Ptr{Cvoid}
+
 """
     function getS(m::SBML.Model; zeros=spzeros)::Tuple{Vector{String},Vector{String},AbstractMatrix{Float64}}
 
@@ -130,3 +132,34 @@ function extensive_kinetic_math(m::SBML.Model, formula::SBML.Math)
 
     conv(formula)
 end
+
+"""
+    get_error_messages(doc::Ptr{Cvoid}, error::Exception)
+
+Show the error messages reported by SBML in the `doc` document and throw the
+`error` if they are more than 1.
+"""
+function get_error_messages(doc::VPtr, error::Exception)
+    n_errs = ccall(sbml(:SBMLDocument_getNumErrors), Cuint, (VPtr,), doc)
+    for i = 1:n_errs
+        err = ccall(sbml(:SBMLDocument_getError), VPtr, (VPtr, Cuint), doc, i-1)
+        msg = string(strip(get_string(err, :XMLError_getMessage)))
+        @error "SBML reported error: $(msg)"
+    end
+    if n_errs > 0
+        throw(error)
+    end
+    nothing
+end
+
+"""
+    check_errors(success::Integer, doc::Ptr{Cvoid}, error::Exception)
+
+If success is a 0-valued `Integer` (a logical `false`), then call
+[`get_error_messages`](@ref) to show the error messages reported by SBML in the
+`doc` document and throw the `error` if they are more than 1.  `success` is
+typically the value returned by an SBML C function operating on `doc` which
+returns a boolean flag to signal a successful operation.
+"""
+check_errors(success::Integer, doc::VPtr, error::Exception) =
+    Bool(success) || get_error_messages(doc, error)

test/loadmodels.jl

@@ -135,7 +135,7 @@ end
 end
 
 @testset "converters fail gracefully" begin
-    @test_throws ErrorException readSBML(
+    @test_logs (:error, r"^SBML reported error:") (:error, r"^SBML reported error:") @test_throws ErrorException readSBML(
         joinpath(@__DIR__, "data", "sbml01289.xml"),
         doc -> begin
             set_level_and_version(3, 1)(doc)