Skip to content

Commit

Permalink
Improved error messages from plugins (#1031)
Browse files Browse the repository at this point in the history
# Description
Issue a warning message with plugins that fails to load.
  • Loading branch information
francescalb authored Dec 9, 2024
2 parents 9097409 + 1943481 commit bcb844c
Show file tree
Hide file tree
Showing 7 changed files with 78 additions and 33 deletions.
5 changes: 3 additions & 2 deletions src/dlite-mapping-plugins.c
Original file line number Diff line number Diff line change
Expand Up @@ -132,11 +132,12 @@ const DLiteMappingPlugin *dlite_mapping_plugin_get(const char *name)
{
const DLiteMappingPlugin *api;
PluginInfo *info;
int code = dliteMappingError;

if (!(info = get_mapping_plugin_info())) return NULL;
if ((api = (DLiteMappingPlugin *)plugin_get_api(info, name))) return api;
if ((api = (DLiteMappingPlugin *)plugin_get_api(info, name, code))) return api;
load_mapping_plugins();
if ((api = (DLiteMappingPlugin *)plugin_get_api(info, name))) return api;
if ((api = (DLiteMappingPlugin *)plugin_get_api(info, name, code))) return api;
#ifdef WITH_PYTHON
if ((api = dlite_python_mapping_get_api(name))) return api;
#endif
Expand Down
37 changes: 26 additions & 11 deletions src/dlite-storage-plugins.c
Original file line number Diff line number Diff line change
Expand Up @@ -110,9 +110,10 @@ const DLiteStoragePlugin *dlite_storage_plugin_get(const char *name)
if (!(info = get_storage_plugin_info())) return NULL;

/* Return plugin if it is loaded */
ErrTry: // silence errors
api = (const DLiteStoragePlugin *)plugin_get_api(info, name);
ErrOther: // hmm, we should update plugins.c to produce more specific errors
ErrTry: // silence dliteStorageLoadError
api = (const DLiteStoragePlugin *)plugin_get_api(info, name,
dliteStorageLoadError);
ErrCatch(dliteStorageLoadError):
break;
ErrEnd;
if (api) return api;
Expand All @@ -125,9 +126,10 @@ const DLiteStoragePlugin *dlite_storage_plugin_get(const char *name)
plugin_load_all(info);
memcpy(g->storage_plugin_path_hash, hash, sizeof(hash));

ErrTry: // silence errors
api = (const DLiteStoragePlugin *)plugin_get_api(info, name);
ErrOther: // update plugins.c to produce more specific errors
ErrTry: // silence dliteStorageLoadError
api = (const DLiteStoragePlugin *)plugin_get_api(info, name,
dliteStorageLoadError);
ErrCatch(dliteStorageLoadError):
break;
ErrEnd;
if (api) return api;
Expand Down Expand Up @@ -179,12 +181,25 @@ const DLiteStoragePlugin *dlite_storage_plugin_get(const char *name)
}
}

if (n <= 1)
if (n <= 1) {
m += asnpprintf(&buf, &size, m,
" Are the required Python packages installed or %s\n"
" DLITE_STORAGE_PLUGIN_DIRS or "
"DLITE_PYTHON_STORAGE_PLUGIN_DIRS\n"
" environment variables set?", submsg);
" If the plugin is listed above, but could not be "
"loaded, it may be an\n"
" error in the plugin. Are the required Python "
"packages installed?\n");
}
if (!getenv("DLITE_PYDEBUG")) {
r = asnpprintf(&buf, &size, m,
" Please rerun with the DLITE_PYDEBUG environment "
"variable set.\n");
if (r >= 0) m += r;
}
r = asnpprintf(&buf, &size, m,
" If the plugin is not listed above, it may not be "
"in the search path.\n"
" Are the %sDLITE_STORAGE_PLUGIN_DIRS or "
"DLITE_PYTHON_STORAGE_PLUGIN_DIRS\n"
" environment variables set?", submsg);
errx(dliteStorageOpenError, "%s", buf);
#endif
free(buf);
Expand Down
33 changes: 24 additions & 9 deletions src/pyembed/dlite-pyembed.c
Original file line number Diff line number Diff line change
Expand Up @@ -321,6 +321,7 @@ int dlite_pyembed_errmsg(char *errmsg, size_t errlen)
PyErr_Fetch(&type, &value, &tb);
if (!type) return 0;
PyErr_NormalizeException(&type, &value, &tb);
if (!tb) PyException_SetTraceback(value, tb);

/* Try to create a error message from Python using the treaceback package */
if (errmsg) {
Expand Down Expand Up @@ -624,6 +625,8 @@ PyObject *dlite_pyembed_load_plugins(FUPaths *paths, PyObject *baseclass,
PyObject *subclassnames=NULL;
FUIter *iter;
int i;
FILE *fp=NULL;
size_t errors_pos=0;
char errors[4098] = "";

dlite_errclr();
Expand Down Expand Up @@ -655,12 +658,9 @@ PyObject *dlite_pyembed_load_plugins(FUPaths *paths, PyObject *baseclass,

if ((stem = fu_stem(path))) {
int stat;
FILE *fp=NULL;
PyObject *plugindict;

if (!(plugindict = dlite_python_plugindict(stem))) goto fail;
//if (!(plugindict = dlite_python_dlitedict())) goto fail;

if (!(ppath = PyUnicode_FromString(path)))
FAIL1("cannot create Python string from path: '%s'", path);
stat = PyDict_SetItemString(plugindict, "__file__", ppath);
Expand All @@ -679,17 +679,29 @@ PyObject *dlite_pyembed_load_plugins(FUPaths *paths, PyObject *baseclass,
PyObject *ret = PyRun_File(fp, path, Py_file_input, plugindict,
plugindict);
if (!ret) {

if (failed_paths && failed_len) {
char **new = strlst_append(*failed_paths, failed_len, path);
if (!new) FAIL("allocation failure");
*failed_paths = new;
}

dlite_pyembed_errmsg(NULL, 0);
fclose(fp);
int m;
if (errors_pos < sizeof(errors) &&
(m = snprintf(errors+errors_pos, sizeof(errors)-errors_pos,
" - %s: (%s): ", stem, path)) > 0)
errors_pos += m;
if (errors_pos < sizeof(errors) &&
(m = dlite_pyembed_errmsg(errors+errors_pos,
sizeof(errors)-errors_pos)) > 0)
errors_pos += m;
if (errors_pos < sizeof(errors) &&
(m = snprintf(errors+errors_pos, sizeof(errors)-errors_pos,
"\n")) > 0)
errors_pos += m;
}
Py_XDECREF(ret);
fclose(fp);
fp = NULL;
}
}
free(stem);
Expand All @@ -698,11 +710,13 @@ PyObject *dlite_pyembed_load_plugins(FUPaths *paths, PyObject *baseclass,
}
if (fu_pathsiter_deinit(iter)) goto fail;

if (errors[0])
if (errors[0]) {
dlite_warn("Could not load the following Python plugins:\n%s"
" You might have to install corresponding python "
"package(s).\n",
"You may have to install missing python package(s).\n",
errors);
}



/* Append new subclasses to the list of Python plugins that will be
returned */
Expand All @@ -729,6 +743,7 @@ PyObject *dlite_pyembed_load_plugins(FUPaths *paths, PyObject *baseclass,
fail:
Py_XDECREF(lst);
Py_XDECREF(subclassnames);
if (fp) fclose(fp);
return subclasses;
}

Expand Down
8 changes: 7 additions & 1 deletion src/utils/err.c
Original file line number Diff line number Diff line change
Expand Up @@ -287,8 +287,9 @@ int _err_vformat(ErrLevel errlevel, int eval, int errnum, const char *file,
if (errlevel >= errLevelError && tls->err_record->state)
tls->err_record->reraise = eval;

/* If we are not within a ErrTry...ErrEnd clause */
/* Call error handler */
if (!tls->err_record->prev) {
/* If we are not within a ErrTry...ErrEnd clause */

/* ...call the error handler */
if (handler) handler(tls->err_record);
Expand All @@ -309,6 +310,11 @@ int _err_vformat(ErrLevel errlevel, int eval, int errnum, const char *file,
if (!handler) err_default_handler(tls->err_record);
exit(eval);
}

} else if (errlevel == errLevelWarn) {
/* Handle warnings within a ErrTry...ErrEnd clause */

if (handler) handler(tls->err_record);
}

/* Clear errno */
Expand Down
17 changes: 9 additions & 8 deletions src/utils/plugin.c
Original file line number Diff line number Diff line change
Expand Up @@ -180,13 +180,13 @@ static int register_plugin(PluginInfo *info, const PluginAPI *api,
If `name` is NULL, all plugins matching `pattern` are registered and a
pointer to latest successfully loaded API is returned.
If `emit_err` is non-zero, an error message will be emitted in case a
If `errcode` is non-zero, an error with this code will be emitted in case a
named plugin cannot be loaded.
Returns a pointer to the plugin API or NULL on error.
*/
const PluginAPI *plugin_load(PluginInfo *info, const char *name,
const char *pattern, int emit_err)
const char *pattern, int errcode)
{
FUIter *iter=NULL;
const char *filepath;
Expand Down Expand Up @@ -255,7 +255,7 @@ const PluginAPI *plugin_load(PluginInfo *info, const char *name,
}
}
if (name) {
if (emit_err) errx(1, "no such api: \"%s\"", name);
if (errcode) errx(errcode, "no such plugin: \"%s\"", name);
retval = NULL;
} else {
retval = loaded_api;
Expand Down Expand Up @@ -308,9 +308,11 @@ int plugin_has_api(PluginInfo *info, const char *name)
any shared library. If a plugin with the provided name is found, it
is loaded, registered and returned.
If the plugin is not found, `err()` is called with `eval` set to `errcode`.
Otherwise NULL is returned.
*/
const PluginAPI *plugin_get_api(PluginInfo *info, const char *name)
const PluginAPI *plugin_get_api(PluginInfo *info, const char *name, int errcode)
{
const PluginAPI *api=NULL;
PluginAPI **p;
Expand All @@ -322,12 +324,11 @@ const PluginAPI *plugin_get_api(PluginInfo *info, const char *name)

/* Load plugin from search path */
if (!(pattern = malloc(strlen(name) + strlen(DSL_EXT) + 1)))
return err(1, "allocation failure"), NULL;
return err(pluginMemoryError, "allocation failure"), NULL;
strcpy(pattern, name);
strcat(pattern, DSL_EXT);
if (!(api = plugin_load(info, name, pattern, 0)) &&
!(api = plugin_load(info, name, "*" DSL_EXT, 1)))
err(1, "cannot find api: '%s'", name);
if (!(api = plugin_load(info, name, pattern, 0)))
api = plugin_load(info, name, "*" DSL_EXT, errcode);

if (pattern) free(pattern);
return api;
Expand Down
9 changes: 8 additions & 1 deletion src/utils/plugin.h
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,11 @@
#include "globmatch.h"
#include "map.h"

/* SWIG-compatible error codes */
typedef enum {
pluginMemoryError=-12 /*!< Out of memory */
} PluginErrCodes;


/** Initial fields in all plugin APIs. */
#define PluginAPI_HEAD \
Expand Down Expand Up @@ -138,9 +143,11 @@ int plugin_has_api(PluginInfo *info, const char *name);
any shared library. If a plugin with the provided name is found, it
is loaded, registered and returned.
If the plugin is not found, err() is called with `eval` set to `errcode`.
Otherwise NULL is returned.
*/
const PluginAPI *plugin_get_api(PluginInfo *info, const char *name);
const PluginAPI *plugin_get_api(PluginInfo *info, const char *name, int errcode);

/**
Load all plugins that can be found in the plugin search path.
Expand Down
2 changes: 1 addition & 1 deletion src/utils/tests/test_plugin.c
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ MU_TEST(test_info_create)
MU_TEST(test_get_api)
{
const TestAPI *api;
mu_check((api = (const TestAPI *)plugin_get_api(info, "testapi")));
mu_check((api = (const TestAPI *)plugin_get_api(info, "testapi", -2)));
mu_assert_string_eq("testapi", api->name);
mu_assert_int_eq(4, api->fun1(1, 3));
mu_assert_double_eq(6.28, api->fun2(3.14));
Expand Down

0 comments on commit bcb844c

Please sign in to comment.