From 6f38fb49ea2dbb6252bf44cd6d239a6adff6050e Mon Sep 17 00:00:00 2001 From: Jesper Friis Date: Sat, 17 Aug 2024 19:06:54 +0200 Subject: [PATCH] Cleaned up errors and made them more maintainable. (#929) Cleaned up errors and made them more maintainable * Removed duplicated code * Use switch on enum to ensure that compiler warnings if not all places in the code are updated when a new error code is added. * Added NameError, PermissionError and InvalidMetadataError. * Renamed dliteSearchError to dliteLookupError for better correspondance with python. * Added deprecation warning for DLiteSearchError --- bindings/python/dlite-collection-python.i | 2 +- bindings/python/dlite-entity-python.i | 8 +- bindings/python/dlite-misc-python.i | 6 +- bindings/python/dlite-python.i | 10 +++ bindings/python/tests/test_collection.py | 6 +- src/dlite-errors.c | 14 +++- src/dlite-errors.h | 63 ++++++++------- src/dlite-misc.c | 15 +++- src/pyembed/dlite-pyembed.c | 97 +++++++++++++++-------- src/pyembed/dlite-python-singletons.c | 31 +------- src/tests/test_collection.c | 2 +- src/tests/test_triplestore.c | 2 +- src/triplestore.c | 4 +- storages/rdf/dlite-rdf.c | 6 +- 14 files changed, 151 insertions(+), 115 deletions(-) diff --git a/bindings/python/dlite-collection-python.i b/bindings/python/dlite-collection-python.i index 9759c20d3..de53c2309 100644 --- a/bindings/python/dlite-collection-python.i +++ b/bindings/python/dlite-collection-python.i @@ -361,7 +361,7 @@ class Collection(Instance): Raises: DLiteTypeError: Not exactly two of `s`, `p` and `o` are None. - DLiteSearchError: No match can be found or more that one match + DLiteLookupError: No match can be found or more that one match if `any` is true. """ return _dlite._collection_value(self, s, p, o, d, default, any) diff --git a/bindings/python/dlite-entity-python.i b/bindings/python/dlite-entity-python.i index a6c26878e..590eb2108 100644 --- a/bindings/python/dlite-entity-python.i +++ b/bindings/python/dlite-entity-python.i @@ -8,8 +8,8 @@ from uuid import UUID import numpy as np -class InvalidMetadataError: - """Malformed or invalid metadata.""" +#class InvalidMetadataError: +# """Malformed or invalid metadata.""" class Metadata(Instance): @@ -91,7 +91,7 @@ class Metadata(Instance): def getprop(self, name): """Returns the metadata property object with the given name.""" if "properties" not in self.properties: - raise InvalidMetadataError( + raise _dlite.DLiteInvalidMetadataError( 'self.properties on metadata must contain a "properties" item' ) lst = [p for p in self.properties["properties"] if p.name == name] @@ -109,7 +109,7 @@ class Metadata(Instance): def propnames(self): """Returns a list of all property names in this metadata.""" if "properties" not in self.properties: - raise InvalidMetadataError( + raise _dlite.DLiteInvalidMetadataError( 'self.properties on metadata must contain a "properties" item' ) return [p.name for p in self.properties['properties']] diff --git a/bindings/python/dlite-misc-python.i b/bindings/python/dlite-misc-python.i index 866a222d6..36b2e2508 100644 --- a/bindings/python/dlite-misc-python.i +++ b/bindings/python/dlite-misc-python.i @@ -97,7 +97,11 @@ def deprecation_warning(version_removed, descr): return _deprecation_warning_record.add(descr) - warnings.warn(descr, DeprecationWarning, stacklevel=2) + warnings.warn( + f"{descr}\nIt will be removed in v{version_removed}", + DeprecationWarning, + stacklevel=2, + ) dlite_version = get_version() if chk_semver(version_removed) < 0: diff --git a/bindings/python/dlite-python.i b/bindings/python/dlite-python.i index d4861ef82..46ea88456 100644 --- a/bindings/python/dlite-python.i +++ b/bindings/python/dlite-python.i @@ -1413,4 +1413,14 @@ def instance_cast(inst, newtype=None): inst.__class__ = Metadata return inst + +# Deprecated exceptions +class DLiteSearchError(_dlite.DLiteLookupError): + def __init__(self, *args, **kwargs): + super().__init__(*args, **kwargs) + deprecation_warning( + "0.7.0", + "DLiteSearchError has been renamed to DLiteLookupError." + ) + %} diff --git a/bindings/python/tests/test_collection.py b/bindings/python/tests/test_collection.py index 3d9dee18b..1b6f8119f 100644 --- a/bindings/python/tests/test_collection.py +++ b/bindings/python/tests/test_collection.py @@ -178,13 +178,13 @@ with raises(dlite.DLiteTypeError): coll.value() -with raises(dlite.DLiteSearchError): +with raises(dlite.DLiteLookupError): coll.value(s="dog", p="x") -with raises(dlite.DLiteSearchError): +with raises(dlite.DLiteLookupError): coll.value(p="_is-a", o="Instance") -with raises(dlite.DLiteSearchError): +with raises(dlite.DLiteLookupError): coll.value(s="meta", p="_has-uuid", d="xsd:int") diff --git a/src/dlite-errors.c b/src/dlite-errors.c index bcb6bdc1f..0c73090ea 100644 --- a/src/dlite-errors.c +++ b/src/dlite-errors.c @@ -17,7 +17,7 @@ const char *dlite_errname(DLiteErrCode code) case dliteRuntimeError: return "DLiteRuntime"; case dliteIndexError: return "DLiteIndex"; case dliteTypeError: return "DLiteType"; - case dliteDivisionByZero: return "DLiteDivisionByZero"; + case dliteDivisionByZeroError: return "DLiteDivisionByZero"; case dliteOverflowError: return "DLiteOverflow"; case dliteSyntaxError: return "DLiteSyntax"; case dliteValueError: return "DLiteValue"; @@ -28,12 +28,15 @@ const char *dlite_errname(DLiteErrCode code) case dliteOSError: return "DLiteOS"; case dliteKeyError: return "DLiteKey"; - case dliteSearchError: return "DLiteSearch"; + case dliteNameError: return "DLiteName"; + case dliteLookupError: return "DLiteLookup"; case dliteParseError: return "DLiteParse"; + case dlitePermissionError: return "DLitePermission"; case dliteSerialiseError: return "DLiteSerialise"; case dliteUnsupportedError: return "DLiteUnsupported"; case dliteVerifyError: return "DLiteVerify"; case dliteInconsistentDataError: return "DLiteInconsistentData"; + case dliteInvalidMetadataError: return "DLiteInvalidMetadata"; case dliteStorageOpenError: return "DLiteStorageOpen"; case dliteStorageLoadError: return "DLiteStorageLoad"; case dliteStorageSaveError: return "DLiteStorageSave"; @@ -63,7 +66,7 @@ const char *dlite_errdescr(DLiteErrCode code) case dliteRuntimeError: return "Unspecified run-time error"; case dliteIndexError: return "Index out of range"; case dliteTypeError: return "Inappropriate argument type"; - case dliteDivisionByZero: return "Division by zero"; + case dliteDivisionByZeroError: return "Division by zero"; case dliteOverflowError: return "Result too large to be represented"; case dliteSyntaxError: return "Invalid syntax"; case dliteValueError: return "Inappropriate argument value (of correct type)"; @@ -74,12 +77,15 @@ const char *dlite_errdescr(DLiteErrCode code) case dliteOSError: return "Error calling a system function"; case dliteKeyError: return "Mapping key is not found"; - case dliteSearchError: return "Error in search"; + case dliteNameError: return "Name not found"; + case dliteLookupError: return "Error looking up item"; case dliteParseError: return "Cannot parse input"; + case dlitePermissionError: return "Not enough permissions"; case dliteSerialiseError: return "Cannot serialise output"; case dliteUnsupportedError: return "Feature is not implemented/supported"; case dliteVerifyError: return "Object cannot be verified"; case dliteInconsistentDataError: return "Inconsistent data"; + case dliteInvalidMetadataError: return "Invalid metadata"; case dliteStorageOpenError: return "Cannot open storage plugin"; case dliteStorageLoadError: return "Cannot load storage plugin"; case dliteStorageSaveError: return "Cannot save storage plugin"; diff --git a/src/dlite-errors.h b/src/dlite-errors.h index d8a0f57b0..816743ec4 100644 --- a/src/dlite-errors.h +++ b/src/dlite-errors.h @@ -5,42 +5,45 @@ /** DLite error codes */ typedef enum { /* Error codes copied form SWIG */ - dliteSuccess=0, /*!< Success */ - dliteUnknownError=-1, /*!< Generic unknown error */ - dliteIOError=-2, /*!< File input/output error */ - dliteRuntimeError=-3, /*!< Unspecified run-time error */ - dliteIndexError=-4, /*!< Index out of range. Ex: int x[]={1,2,3}; x[7]; */ - dliteTypeError=-5, /*!< Inappropriate argument type */ - dliteDivisionByZero=-6, /*!< Division by zero */ - dliteOverflowError=-7, /*!< Result too large to be represented */ - dliteSyntaxError=-8, /*!< Invalid syntax */ - dliteValueError=-9, /*!< Inappropriate argument value */ - dliteSystemError=-10, /*!< Internal error in DLite. Please report this */ - dliteAttributeError=-11,/*!< Attribute or variable not found */ - dliteMemoryError=-12, /*!< Out of memory */ - dliteNullReferenceError=-13, /*!< Unexpected NULL argument */ + dliteSuccess=0, /*!< Success */ + dliteUnknownError=-1, /*!< Generic unknown error */ + dliteIOError=-2, /*!< File input/output error */ + dliteRuntimeError=-3, /*!< Unspecified run-time error */ + dliteIndexError=-4, /*!< Index out of range. Ex: int x[]={1,2,3}; x[7]; */ + dliteTypeError=-5, /*!< Inappropriate argument type */ + dliteDivisionByZeroError=-6, /*!< Division by zero */ + dliteOverflowError=-7, /*!< Result too large to be represented */ + dliteSyntaxError=-8, /*!< Invalid syntax */ + dliteValueError=-9, /*!< Inappropriate argument value */ + dliteSystemError=-10, /*!< Internal error in DLite. Please report this */ + dliteAttributeError=-11, /*!< Attribute or variable not found */ + dliteMemoryError=-12, /*!< Out of memory */ + dliteNullReferenceError=-13, /*!< Unexpected NULL argument */ /* Additional DLite-specific errors */ dliteOSError=-14, /*!< Error calling a system function */ dliteKeyError=-15, /*!< Mapping key not found */ - dliteSearchError=-16, /*!< Error in search */ - dliteParseError=-17, /*!< Cannot parse input */ - dliteSerialiseError=-18, /*!< Cannot serialise output */ - dliteUnsupportedError=-19, /*!< Feature is not implemented/supported */ - dliteVerifyError=-20, /*!< Object cannot be verified */ - dliteInconsistentDataError=-21,/*!< Inconsistent data */ - dliteStorageOpenError=-22, /*!< Cannot open storage plugin */ - dliteStorageLoadError=-23, /*!< Cannot load storage plugin */ - dliteStorageSaveError=-24, /*!< Cannot save storage plugin */ - dliteOptionError=-25, /*!< Invalid storage plugin option */ - dliteMissingInstanceError=-26, /*!< No instance with given id can be found */ - dliteMissingMetadataError=-27, /*!< No metadata with given id can be found */ - dliteMetadataExistError=-28, /*!< Metadata with given id already exists */ - dliteMappingError=-29, /*!< Error in instance mappings */ - dlitePythonError=-30, /*!< Error calling Python API */ + dliteNameError=-16, /*!< Name not found */ + dliteLookupError=-17, /*!< Error looking up item */ + dliteParseError=-18, /*!< Cannot parse input */ + dlitePermissionError=-19, /*!< Not enough permissions */ + dliteSerialiseError=-20, /*!< Cannot serialise output */ + dliteUnsupportedError=-21, /*!< Feature is not implemented/supported */ + dliteVerifyError=-22, /*!< Object cannot be verified */ + dliteInconsistentDataError=-23,/*!< Inconsistent data */ + dliteInvalidMetadataError=-24, /*!< Invalid metadata */ + dliteStorageOpenError=-25, /*!< Cannot open storage plugin */ + dliteStorageLoadError=-26, /*!< Cannot load storage plugin */ + dliteStorageSaveError=-27, /*!< Cannot save storage plugin */ + dliteOptionError=-28, /*!< Invalid storage plugin option */ + dliteMissingInstanceError=-29, /*!< No instance with given id can be found */ + dliteMissingMetadataError=-30, /*!< No metadata with given id can be found */ + dliteMetadataExistError=-31, /*!< Metadata with given id already exists */ + dliteMappingError=-32, /*!< Error in instance mappings */ + dlitePythonError=-33, /*!< Error calling Python API */ /* Should always be the last error */ - dliteLastError=-31 + dliteLastError=-34 } DLiteErrCode; diff --git a/src/dlite-misc.c b/src/dlite-misc.c index 1cef30b22..866e7dd01 100644 --- a/src/dlite-misc.c +++ b/src/dlite-misc.c @@ -540,11 +540,22 @@ void dlite_globals_set(DLiteGlobals *globals_handler) } -/* Error handler for DLite. */ +/* Error handler for DLite. + + Since errors + Print warnings, but not errors unless DLITE_PYDEBUG is set. + */ static void dlite_err_handler(const ErrRecord *record) { - if (!dlite_err_ignored_get(record->eval) && getenv("DLITE_PYDEBUG")) +#ifdef WITH_PYTHON + if (record->level != errLevelError || + !getenv("DLITE_PYDEBUG") || + !dlite_err_ignored_get(record->eval)) err_default_handler(record); +#else /* WITH_PYTHON */ + if (!dlite_err_ignored_get(record->eval)) + err_default_handler(record); +#endif } diff --git a/src/pyembed/dlite-pyembed.c b/src/pyembed/dlite-pyembed.c index e4f54cf66..ac1c7be9d 100644 --- a/src/pyembed/dlite-pyembed.c +++ b/src/pyembed/dlite-pyembed.c @@ -52,30 +52,77 @@ static PyembedGlobals *get_globals(void) return g; } + +/* + Return Python exception class corresponding to given DLite error code. + Returns NULL if `code` is zero. + */ +PyObject *dlite_pyembed_exception(DLiteErrCode code) +{ + switch (code) { + case dliteSuccess: return NULL; + case dliteUnknownError: break; + case dliteIOError: return PyExc_IOError; + case dliteRuntimeError: return PyExc_RuntimeError; + case dliteIndexError: return PyExc_IndexError; + case dliteTypeError: return PyExc_TypeError; + case dliteDivisionByZeroError: return PyExc_ZeroDivisionError; + case dliteOverflowError: return PyExc_OverflowError; + case dliteSyntaxError: return PyExc_SyntaxError; + case dliteValueError: return PyExc_ValueError; + case dliteSystemError: return PyExc_SystemError; + case dliteAttributeError: return PyExc_AttributeError; + case dliteMemoryError: return PyExc_MemoryError; + case dliteNullReferenceError: break; + + case dliteOSError: return PyExc_OSError; + case dliteKeyError: return PyExc_KeyError; + case dliteNameError: return PyExc_NameError; + case dliteLookupError: return PyExc_LookupError; + case dliteParseError: return PyExc_IOError; // dup + case dlitePermissionError: return PyExc_PermissionError; + case dliteSerialiseError: return PyExc_IOError; // dup + case dliteUnsupportedError: break; + case dliteVerifyError: break; + case dliteInconsistentDataError: return PyExc_ValueError; // dup + case dliteInvalidMetadataError: return PyExc_ValueError; // dup + case dliteStorageOpenError: return PyExc_IOError; // dup + case dliteStorageLoadError: return PyExc_IOError; // dup + case dliteStorageSaveError: return PyExc_IOError; // dup + case dliteOptionError: return PyExc_ValueError; // dup + case dliteMissingInstanceError: return PyExc_LookupError; // dup + case dliteMissingMetadataError: return PyExc_LookupError; // dup + case dliteMetadataExistError: break; + case dliteMappingError: break; + case dlitePythonError: break; + case dliteLastError: break; + } + return PyExc_Exception; +} + + /* Help function returning a constant pointer to a NULL-terminated array of ErrorCorrelation records. */ static const ErrorCorrelation *error_correlations(void) { PyembedGlobals *g = get_globals(); if (!g->errcorr) { - ErrorCorrelation corr[] = { - {PyExc_KeyError, dliteKeyError}, - {PyExc_MemoryError, dliteMemoryError}, - {PyExc_AttributeError, dliteAttributeError}, - {PyExc_SystemError, dliteSystemError}, - {PyExc_ValueError, dliteValueError}, - {PyExc_SyntaxError, dliteSyntaxError}, - {PyExc_OverflowError, dliteOverflowError}, - {PyExc_ZeroDivisionError, dliteDivisionByZero}, - {PyExc_TypeError, dliteTypeError}, - {PyExc_IndexError, dliteIndexError}, - {PyExc_RuntimeError, dliteRuntimeError}, - {PyExc_IOError, dliteIOError}, - {NULL, 0} - }; - if (!(g->errcorr = malloc(sizeof(corr)))) + int i, code, n=1; + for (code=-1; code>dliteLastError; code--) + if (dlite_pyembed_exception(code) != PyExc_Exception) n++; + + if (!(g->errcorr = calloc(n, sizeof(ErrorCorrelation)))) return dlite_err(dliteMemoryError, "allocation failure"), NULL; - memcpy(g->errcorr, corr, sizeof(corr)); + + for (code=-1, i=0; code>dliteLastError; code--) { + PyObject *exc; + if ((exc = dlite_pyembed_exception(code)) != PyExc_Exception) { + g->errcorr[i].exc = exc; + g->errcorr[i].errcode = code; + i++; + } + } + assert(i == n-1); } return g->errcorr; } @@ -207,22 +254,6 @@ DLiteErrCode dlite_pyembed_errcode(PyObject *type) return dliteUnknownError; } -/* - Return Python exception class corresponding to given DLite error code. - Returns NULL if `code` is zero. - */ -PyObject *dlite_pyembed_exception(DLiteErrCode code) -{ - const ErrorCorrelation *corr = error_correlations(); - if (!code) return NULL; - while (corr->exc) { - if (code == corr->errcode) return corr->exc; - corr++; - } - return PyExc_Exception; -} - - /* Writes Python error message to `errmsg` (of length `len`) if an Python error has occured. diff --git a/src/pyembed/dlite-python-singletons.c b/src/pyembed/dlite-python-singletons.c index 5c3458b67..844708b95 100644 --- a/src/pyembed/dlite-python-singletons.c +++ b/src/pyembed/dlite-python-singletons.c @@ -122,35 +122,6 @@ PyObject *dlite_python_module_class(const char *classname) } -/* - Help function returning builtin Python exception corresponding to - error code or NULL, if no such built-in exception exists in Python. - */ -static PyObject *_python_exc(DLiteErrCode code) -{ - switch (code) { - case dliteIOError: return PyExc_IOError; - case dliteRuntimeError: return PyExc_RuntimeError; - case dliteIndexError: return PyExc_IndexError; - case dliteTypeError: return PyExc_TypeError; - case dliteDivisionByZero: return PyExc_ZeroDivisionError; - case dliteOverflowError: return PyExc_OverflowError; - case dliteSyntaxError: return PyExc_SyntaxError; - case dliteValueError: return PyExc_ValueError; - case dliteSystemError: return PyExc_SystemError; - case dliteAttributeError: return PyExc_AttributeError; - case dliteMemoryError: return PyExc_MemoryError; - - case dliteOSError: return PyExc_OSError; - case dliteKeyError: return PyExc_KeyError; - case dliteUnsupportedError: return PyExc_NotImplementedError; - case dliteStorageOpenError: return PyExc_ConnectionError; - - default: return NULL; - } -} - - /* Returns a borrowed reference to singleton Python exception object for the given DLite error code. @@ -199,7 +170,7 @@ PyObject *dlite_python_module_error(DLiteErrCode code) if ((exc = PyDict_GetItemString(dict, errname))) return exc; /* Base exceptions */ - if ((pyexc = _python_exc(code))) { + if ((pyexc = dlite_pyembed_exception(code)) && pyexc != PyExc_Exception) { if (!(base = Py_BuildValue("(O,O)", dliteError, pyexc))) FAILCODE(dlitePythonError, "cannot build dlite exception base"); } else { diff --git a/src/tests/test_collection.c b/src/tests/test_collection.c index 5bcc2dd11..8f713627d 100644 --- a/src/tests/test_collection.c +++ b/src/tests/test_collection.c @@ -140,7 +140,7 @@ MU_TEST(test_collection_value) "animal", NULL, NULL, 0)); mu_assert_string_eq(NULL, dlite_collection_value(coll, NULL, "is_a", "mammal", NULL, NULL, 0)); - ErrCatch(dliteSearchError): + ErrCatch(dliteLookupError): break; ErrEnd; } diff --git a/src/tests/test_triplestore.c b/src/tests/test_triplestore.c index 4877daa2a..439623d13 100644 --- a/src/tests/test_triplestore.c +++ b/src/tests/test_triplestore.c @@ -162,7 +162,7 @@ MU_TEST(test_value) // no match (datatype does not match) mu_assert_string_eq(NULL, triplestore_value(ts, "book-weight", "has-unit", NULL, "xsd:float", NULL, 0)); - ErrCatch(dliteSearchError): // suppress errors + ErrCatch(dliteLookupError): // suppress errors break; ErrEnd; diff --git a/src/triplestore.c b/src/triplestore.c index 93e6450be..741f35fad 100644 --- a/src/triplestore.c +++ b/src/triplestore.c @@ -78,7 +78,7 @@ const char *triplestore_value(TripleStore *ts, const char *s, const char *p, assert(i >= 0); if (!(t = triplestore_find(&state, s, p, o, d))) { if (!fallback) - FAILCODE4(dliteSearchError, "no values matching the criteria: " + FAILCODE4(dliteLookupError, "no values matching the criteria: " "s='%s', p='%s', o='%s', d='%s'", s, p, o, d); value = fallback; } else { @@ -89,7 +89,7 @@ const char *triplestore_value(TripleStore *ts, const char *s, const char *p, } } if (!any && triplestore_find(&state, s, p, o, d)) - FAILCODE4(dliteSearchError, "more than one value matching the criteria: " + FAILCODE4(dliteLookupError, "more than one value matching the criteria: " "s='%s', p='%s', o='%s', d='%s'. Maybe you want to set `any` " "to true?", s, p, o, d); triplestore_deinit_state(&state); diff --git a/storages/rdf/dlite-rdf.c b/storages/rdf/dlite-rdf.c index 99b345ced..993696f50 100644 --- a/storages/rdf/dlite-rdf.c +++ b/storages/rdf/dlite-rdf.c @@ -336,7 +336,7 @@ DLiteInstance *rdf_load_instance(const DLiteStorage *storage, const char *id) pid = (s->base_uri) ? aprintf("%s:%s", s->base_uri, uuid) : NULL; if (!(value = triplestore_value(ts, pid, _P ":hasMeta", NULL, NULL, NULL, 0))) - FAILCODE2(dliteSearchError, + FAILCODE2(dliteLookupError, "cannot find instance '%s' in RDF storage: %s", pid, s->location); dlite_get_uuid(muuid, value); @@ -348,10 +348,10 @@ DLiteInstance *rdf_load_instance(const DLiteStorage *storage, const char *id) } t2 = triplestore_find(&state, NULL, _P ":hasMeta", NULL, NULL); triplestore_deinit_state(&state); - if (!t) FAILCODE1(dliteSearchError, + if (!t) FAILCODE1(dliteLookupError, "no instances in RDF storage: %s", s->location); - if (t2) FAILCODE1(dliteSearchError, "ID must be provided if storage " + if (t2) FAILCODE1(dliteLookupError, "ID must be provided if storage " "holds more than one instance: %s", s->location); if (!(value = triplestore_value(ts, pid, _P ":hasUUID", NULL, NULL, NULL, 0)))