From 3b2be7085899350a58fe836dc5e1a9650ce369b2 Mon Sep 17 00:00:00 2001 From: Simon Marchi Date: Wed, 17 Jul 2019 18:44:23 -0400 Subject: [PATCH] bt2: make bt2.Error wrap current thread's error This patch makes the Python bindings integrate better with the Babeltrace error framework. When a Babeltrace API call fails, it returns an ERROR or MEMORY_ERROR status and appends a cause to the current thread's error object. When that API call is made from the Python, we convert that to raising a bt2.Error execption. With this patch, we now steal the current thread's error object and give it to the bt2.Error exception object. The Python code can catch this exception and consult the error causes in the error object: def _consume(self): try: something_that_raises_a_bt2_error() except bt2.Error as exc: for cause in exc: print(cause) If the Python code catches the exception and does nothing else with it (as in the example above), the exception object is destroyed, destroying the wrapped bt_error object with it. It can be seen as if the Python code catches the error and recovers from it. If the Python code lets the exception propagate back to the C code, like this: def _consume(self): something_that_raises_a_bt2_error() ... the bt2.Error is converted back to an ERROR status. But we now also take back the bt_error object from the bt2.Error exception object and restore it as the current thread's error object, so that the error is propagated. We also append a new cause to it, with the traceback of where the bt2.Error exception was raised. A more complex case is if the user raises their own exception from the bt2.Error, using Python exception chaining: def _consume(self): try: something_that_raises_a_bt2_error() except bt2.Error as exc: raise MyOwnExceptionType from exc In this case, we start by restoring the bt_error as the thread's error object, to put things back as they were before we called Python. We then append one cause per exception in the chain, starting with the end of the chain. That is, one cause for the bt2.Error (with the traceback to where it was raised) and one cause for the MyOwnExceptionType exception (with the traceback to where it was raised). When it handles a bt2.Error, the Python code can obtain information about the error, mainly to access the list of causes. It does so by accessing the bt2.Error as a sequence of causes. Each cause kind (from unknown actor, from component class actor, from component actor and from message iterator actor) is represented by a class, that gives access to the appropriate properties of the cause (e.g. component class name, port name, etc). Various design choices: - Some causes have a `component_class_type` property, which returns the type of component class in which the cause happened. This property returns a value of the `bt_component_class_type` enum. The user can compare the returned value against the properties of the newly introduced ComponentClassType class. - Because it's now non-trivial, the Error type was moved to the new error.py file. - Add BT_CURRENT_THREAD_MOVE_ERROR_AND_RESET macro, which calls bt_current_thread_move_error and then clears the passed lvalue. It's not essential, but may help avoid mistakes, since once you moved back the error into place, it's no longer yours. - Removed the raise bt2.Error in message._create_from_ptr. If we get an unexpected message type, there's not a lot we can do, it's just a programming error / bug that needs to be fixed. If it happens, we will get a KeyError with the key that was not found, so it should be pretty obvious. Change-Id: Ibb63d57838a248fadda9cb741d920538fe588680 Signed-off-by: Simon Marchi Reviewed-on: https://review.lttng.org/c/babeltrace/+/1738 Reviewed-by: Philippe Proulx Tested-by: jenkins --- include/babeltrace2/current-thread.h | 6 + src/bindings/python/bt2/Makefile.am | 2 + src/bindings/python/bt2/bt2/__init__.py.in | 13 +- src/bindings/python/bt2/bt2/component.py | 2 +- src/bindings/python/bt2/bt2/error.py | 210 +++++++++++++++++ src/bindings/python/bt2/bt2/field_class.py | 8 +- src/bindings/python/bt2/bt2/message.py | 4 - src/bindings/python/bt2/bt2/native_bt.i | 1 + .../bt2/bt2/native_bt_component_class.i | 193 ++++++++++++++-- src/bindings/python/bt2/bt2/native_bt_error.i | 31 +++ src/bindings/python/bt2/bt2/native_bt_graph.i | 4 +- src/bindings/python/bt2/bt2/native_bt_trace.i | 2 +- .../python/bt2/bt2/native_bt_trace_class.i | 2 +- src/bindings/python/bt2/bt2/plugin.py | 2 +- src/bindings/python/bt2/bt2/stream.py | 2 +- .../bt2/trace_collection_message_iterator.py | 16 +- src/bindings/python/bt2/bt2/utils.py | 6 +- .../python/bt2/test_component_class.py | 2 +- tests/bindings/python/bt2/test_error.py | 213 ++++++++++++++++++ tests/bindings/python/bt2/test_field_class.py | 6 +- tests/bindings/python/bt2/test_plugin.py | 2 +- .../python/bt2/test_query_executor.py | 10 +- .../test_trace_collection_message_iterator.py | 4 +- 23 files changed, 674 insertions(+), 67 deletions(-) create mode 100644 src/bindings/python/bt2/bt2/error.py create mode 100644 src/bindings/python/bt2/bt2/native_bt_error.i create mode 100644 tests/bindings/python/bt2/test_error.py diff --git a/include/babeltrace2/current-thread.h b/include/babeltrace2/current-thread.h index 18da2513..39c6ae22 100644 --- a/include/babeltrace2/current-thread.h +++ b/include/babeltrace2/current-thread.h @@ -89,6 +89,12 @@ bt_current_thread_error_append_cause_from_message_iterator( bt_current_thread_error_append_cause_from_message_iterator( \ (_self_iter), __FILE__, __LINE__, (_msg_fmt), ##__VA_ARGS__) +#define BT_CURRENT_THREAD_MOVE_ERROR_AND_RESET(_var) \ + do { \ + bt_current_thread_move_error(_var); \ + (_var) = NULL; \ + } while (0) + #ifdef __cplusplus } #endif diff --git a/src/bindings/python/bt2/Makefile.am b/src/bindings/python/bt2/Makefile.am index 158b8ccd..c8719d66 100644 --- a/src/bindings/python/bt2/Makefile.am +++ b/src/bindings/python/bt2/Makefile.am @@ -12,6 +12,7 @@ SWIG_INTERFACE_FILES = \ bt2/native_bt_component_class.i \ bt2/native_bt_component.i \ bt2/native_bt_connection.i \ + bt2/native_bt_error.i \ bt2/native_bt_event_class.i \ bt2/native_bt_event.i \ bt2/native_bt_field_class.i \ @@ -44,6 +45,7 @@ STATIC_BINDINGS_DEPS = \ bt2/clock_snapshot.py \ bt2/component.py \ bt2/connection.py \ + bt2/error.py \ bt2/event_class.py \ bt2/event.py \ bt2/field.py \ diff --git a/src/bindings/python/bt2/bt2/__init__.py.in b/src/bindings/python/bt2/bt2/__init__.py.in index 49e02d4a..ba17f1be 100644 --- a/src/bindings/python/bt2/bt2/__init__.py.in +++ b/src/bindings/python/bt2/bt2/__init__.py.in @@ -37,6 +37,7 @@ from bt2.component import _UserSinkComponent from bt2.component import _UserSourceComponent from bt2.connection import * from bt2.connection import _Connection +from bt2.error import * from bt2.event import _Event from bt2.event_class import * from bt2.field_class import * @@ -67,19 +68,15 @@ from bt2.value import _IntegerValue from bt2.clock_snapshot import _UnknownClockSnapshot -class Error(Exception): - pass - - class CreationError(Error): '''Raised when object creation fails due to memory issues.''' -class InvalidObject(Error): +class InvalidObject(Exception): pass -class InvalidParams(Error): +class InvalidParams(Exception): pass @@ -99,7 +96,7 @@ class Stop(StopIteration): pass -class IncompleteUserClass(Error): +class IncompleteUserClass(Exception): pass @@ -107,7 +104,7 @@ class Canceled(Exception): pass -class NonexistentClockSnapshot(Error): +class NonexistentClockSnapshot(Exception): pass diff --git a/src/bindings/python/bt2/bt2/component.py b/src/bindings/python/bt2/bt2/component.py index f3184218..07cccec9 100644 --- a/src/bindings/python/bt2/bt2/component.py +++ b/src/bindings/python/bt2/bt2/component.py @@ -522,7 +522,7 @@ class _UserComponentType(type): return self def __call__(cls, *args, **kwargs): - raise bt2.Error( + raise RuntimeError( 'cannot directly instantiate a user component from a Python module' ) diff --git a/src/bindings/python/bt2/bt2/error.py b/src/bindings/python/bt2/bt2/error.py new file mode 100644 index 00000000..7fd46482 --- /dev/null +++ b/src/bindings/python/bt2/bt2/error.py @@ -0,0 +1,210 @@ +from bt2 import utils, native_bt +from collections import abc + + +class ComponentClassType: + SOURCE = native_bt.COMPONENT_CLASS_TYPE_SOURCE + FILTER = native_bt.COMPONENT_CLASS_TYPE_FILTER + SINK = native_bt.COMPONENT_CLASS_TYPE_SINK + + +_COMPONENT_CLASS_TYPE_TO_STR = { + native_bt.COMPONENT_CLASS_TYPE_SOURCE: 'source', + native_bt.COMPONENT_CLASS_TYPE_FILTER: 'filter', + native_bt.COMPONENT_CLASS_TYPE_SINK: 'sink', +} + + +def _create_error_cause_from_ptr(ptr): + actor_type = native_bt.error_cause_get_actor_type(ptr) + return _ACTOR_TYPE_TO_CLS[actor_type](ptr) + + +class _ErrorCause: + def __init__(self, ptr): + self._message = native_bt.error_cause_get_message(ptr) + self._module_name = native_bt.error_cause_get_module_name(ptr) + self._file_name = native_bt.error_cause_get_file_name(ptr) + self._line_number = native_bt.error_cause_get_line_number(ptr) + + def __str__(self): + s = '[{}] ({}:{})\n'.format(self.module_name, self.file_name, self.line_number) + s += self.message + return s + + @property + def message(self): + return self._message + + @property + def module_name(self): + return self._module_name + + @property + def file_name(self): + return self._file_name + + @property + def line_number(self): + return self._line_number + + +class _ComponentErrorCause(_ErrorCause): + def __init__(self, ptr): + super().__init__(ptr) + self._component_name = native_bt.error_cause_component_actor_get_component_name( + ptr + ) + self._component_class_type = native_bt.error_cause_component_actor_get_component_class_type( + ptr + ) + self._component_class_name = native_bt.error_cause_component_actor_get_component_class_name( + ptr + ) + self._plugin_name = native_bt.error_cause_component_actor_get_plugin_name(ptr) + + @property + def component_name(self): + return self._component_name + + @property + def component_class_type(self): + return self._component_class_type + + @property + def component_class_name(self): + return self._component_class_name + + @property + def plugin_name(self): + return self._plugin_name + + +class _ComponentClassErrorCause(_ErrorCause): + def __init__(self, ptr): + super().__init__(ptr) + self._component_class_type = native_bt.error_cause_component_class_actor_get_component_class_type( + ptr + ) + self._component_class_name = native_bt.error_cause_component_class_actor_get_component_class_name( + ptr + ) + self._plugin_name = native_bt.error_cause_component_class_actor_get_plugin_name( + ptr + ) + + @property + def component_class_type(self): + return self._component_class_type + + @property + def component_class_name(self): + return self._component_class_name + + @property + def plugin_name(self): + return self._plugin_name + + +class _MessageIteratorErrorCause(_ErrorCause): + def __init__(self, ptr): + super().__init__(ptr) + self._component_name = native_bt.error_cause_message_iterator_actor_get_component_name( + ptr + ) + self._component_output_port_name = native_bt.error_cause_message_iterator_actor_get_component_output_port_name( + ptr + ) + self._component_class_type = native_bt.error_cause_message_iterator_actor_get_component_class_type( + ptr + ) + self._component_class_name = native_bt.error_cause_message_iterator_actor_get_component_class_name( + ptr + ) + self._plugin_name = native_bt.error_cause_message_iterator_actor_get_plugin_name( + ptr + ) + + @property + def component_name(self): + return self._component_name + + @property + def component_output_port_name(self): + return self._component_output_port_name + + @property + def component_class_type(self): + return self._component_class_type + + @property + def component_class_name(self): + return self._component_class_name + + @property + def plugin_name(self): + return self._plugin_name + + +_ACTOR_TYPE_TO_CLS = { + native_bt.ERROR_CAUSE_ACTOR_TYPE_UNKNOWN: _ErrorCause, + native_bt.ERROR_CAUSE_ACTOR_TYPE_COMPONENT: _ComponentErrorCause, + native_bt.ERROR_CAUSE_ACTOR_TYPE_COMPONENT_CLASS: _ComponentClassErrorCause, + native_bt.ERROR_CAUSE_ACTOR_TYPE_MESSAGE_ITERATOR: _MessageIteratorErrorCause, +} + + +class Error(Exception, abc.Sequence): + """ + Babeltrace API call error. + + This exception is raised when a call to the Babeltrace API returns with + the ERROR or MEMORY_ERROR status codes. + """ + + def __init__(self, msg, ptr=None): + super().__init__(msg) + # Steal the current thread's error. + self._ptr = native_bt.current_thread_take_error() + assert self._ptr is not None + + # Read everything we might need from the error pointer, so we don't + # depend on it. It's possible for the user to keep an Error object + # and to want to read its causes after the error pointer has been + # restored as the current thread's error (and is therefore + # inaccessible). + cause_count = native_bt.error_get_cause_count(self._ptr) + + # We expect the library to append at least one cause (otherwise there + # wouldn't be an bt_error object anyway). Also, while formatting the + # exception, the Python `traceback` module does: + # + # if (exc_value and ...): + # + # If the cause list was empty, this would evaluate to False (which we + # wouldn't want), because of the __bool__ implementation of + # abc.Sequence. If there's at least one cause, we are sure that + # __bool__ will always return True and avoid any problem here. + assert cause_count > 0 + + self._causes = [] + + for i in range(cause_count): + cause_ptr = native_bt.error_borrow_cause_by_index(self._ptr, i) + assert cause_ptr is not None + cause = _create_error_cause_from_ptr(cause_ptr) + self._causes.append(cause) + + def __del__(self): + # If this exception escapes all the way out of the Python code, the + # native code will steal `_ptr` to restore it as the current thread's + # error. If the exception is caught and discarded by the Python code, + # the exception object still owns the error, so we must release it. + if self._ptr is not None: + native_bt.error_release(self._ptr) + + def __getitem__(self, index): + return self._causes[index] + + def __len__(self): + return len(self._causes) diff --git a/src/bindings/python/bt2/bt2/field_class.py b/src/bindings/python/bt2/bt2/field_class.py index 17a150fa..06980b39 100644 --- a/src/bindings/python/bt2/bt2/field_class.py +++ b/src/bindings/python/bt2/bt2/field_class.py @@ -164,7 +164,7 @@ class _EnumerationFieldClass(_IntegerFieldClass, collections.abc.Mapping): utils._check_type(ranges, self._range_set_type) if label in self: - raise bt2.Error("duplicate mapping label '{}'".format(label)) + raise ValueError("duplicate mapping label '{}'".format(label)) status = self._add_mapping(self._ptr, label, ranges._ptr) utils._handle_func_status( @@ -291,7 +291,7 @@ class _StructureFieldClass(_FieldClass, collections.abc.Mapping): utils._check_type(field_class, _FieldClass) if name in self: - raise bt2.Error("duplicate member name '{}'".format(name)) + raise ValueError("duplicate member name '{}'".format(name)) status = native_bt.field_class_structure_append_member( self._ptr, name, field_class._ptr @@ -447,7 +447,7 @@ class _VariantFieldClassWithoutSelector(_VariantFieldClass): utils._check_type(field_class, _FieldClass) if name in self: - raise bt2.Error("duplicate option name '{}'".format(name)) + raise ValueError("duplicate option name '{}'".format(name)) status = native_bt.field_class_variant_without_selector_append_option( self._ptr, name, field_class._ptr @@ -497,7 +497,7 @@ class _VariantFieldClassWithSelector(_VariantFieldClass): utils._check_type(ranges, self._range_set_type) if name in self: - raise bt2.Error("duplicate option name '{}'".format(name)) + raise ValueError("duplicate option name '{}'".format(name)) if len(ranges) == 0: raise ValueError('range set is empty') diff --git a/src/bindings/python/bt2/bt2/message.py b/src/bindings/python/bt2/bt2/message.py index 35cb1bbe..aeb9ebd8 100644 --- a/src/bindings/python/bt2/bt2/message.py +++ b/src/bindings/python/bt2/bt2/message.py @@ -30,10 +30,6 @@ import bt2 def _create_from_ptr(ptr): msg_type = native_bt.message_get_type(ptr) - - if msg_type not in _MESSAGE_TYPE_TO_CLS: - raise bt2.Error('unknown message type: {}'.format(msg_type)) - return _MESSAGE_TYPE_TO_CLS[msg_type]._create_from_ptr(ptr) diff --git a/src/bindings/python/bt2/bt2/native_bt.i b/src/bindings/python/bt2/bt2/native_bt.i index acf47b9d..d27e4279 100644 --- a/src/bindings/python/bt2/bt2/native_bt.i +++ b/src/bindings/python/bt2/bt2/native_bt.i @@ -202,6 +202,7 @@ typedef int bt_bool; %include "native_bt_component.i" %include "native_bt_component_class.i" %include "native_bt_connection.i" +%include "native_bt_error.i" %include "native_bt_event.i" %include "native_bt_event_class.i" %include "native_bt_field.i" diff --git a/src/bindings/python/bt2/bt2/native_bt_component_class.i b/src/bindings/python/bt2/bt2/native_bt_component_class.i index 0b679ec8..b9e91a4a 100644 --- a/src/bindings/python/bt2/bt2/native_bt_component_class.i +++ b/src/bindings/python/bt2/bt2/native_bt_component_class.i @@ -172,12 +172,162 @@ void native_comp_class_dtor(void) { } } +static +void restore_current_thread_error_and_append_exception_chain_recursive( + PyObject *py_exc_value, + bt_self_component_class *self_component_class, + bt_self_component *self_component, + bt_self_message_iterator *self_message_iterator, + const char *module_name) +{ + PyObject *py_exc_cause_value; + PyObject *py_exc_type = NULL; + PyObject *py_exc_tb = NULL; + GString *gstr = NULL; + + /* If this exception has a cause, handle that one first. */ + py_exc_cause_value = PyException_GetCause(py_exc_value); + if (py_exc_cause_value) { + restore_current_thread_error_and_append_exception_chain_recursive( + py_exc_cause_value, self_component_class, + self_component, self_message_iterator, module_name); + } + + /* + * If the raised exception is a bt2.Error, restore the wrapped error. + */ + if (PyErr_GivenExceptionMatches(py_exc_value, py_mod_bt2_exc_error_type)) { + PyObject *py_error_swig_ptr; + const bt_error *error; + int ret; + + /* + * We never raise a bt2.Error with a cause: it should be the + * end of the chain. + */ + BT_ASSERT(!py_exc_cause_value); + + /* + * We steal the error object from the exception, to move + * it back as the current thread's error. + */ + py_error_swig_ptr = PyObject_GetAttrString(py_exc_value, "_ptr"); + BT_ASSERT(py_error_swig_ptr); + + ret = PyObject_SetAttrString(py_exc_value, "_ptr", Py_None); + BT_ASSERT(ret == 0); + + ret = SWIG_ConvertPtr(py_error_swig_ptr, (void **) &error, + SWIGTYPE_p_bt_error, 0); + BT_ASSERT(ret == 0); + + BT_CURRENT_THREAD_MOVE_ERROR_AND_RESET(error); + + Py_DECREF(py_error_swig_ptr); + } + + py_exc_type = PyObject_Type(py_exc_value); + py_exc_tb = PyException_GetTraceback(py_exc_value); + + gstr = bt_py_common_format_exception(py_exc_type, py_exc_value, + py_exc_tb, BT_LOG_OUTPUT_LEVEL, false); + if (!gstr) { + /* bt_py_common_format_exception has already warned. */ + goto end; + } + + if (self_component_class) { + BT_CURRENT_THREAD_ERROR_APPEND_CAUSE_FROM_COMPONENT_CLASS( + self_component_class, "%s", gstr->str); + } else if (self_component) { + BT_CURRENT_THREAD_ERROR_APPEND_CAUSE_FROM_COMPONENT( + self_component, "%s", gstr->str); + } else if (self_message_iterator) { + BT_CURRENT_THREAD_ERROR_APPEND_CAUSE_FROM_MESSAGE_ITERATOR( + self_message_iterator, "%s", gstr->str); + } else { + BT_CURRENT_THREAD_ERROR_APPEND_CAUSE_FROM_UNKNOWN( + module_name, "%s", gstr->str); + } + +end: + if (gstr) { + g_string_free(gstr, TRUE); + } + + Py_XDECREF(py_exc_cause_value); + Py_XDECREF(py_exc_type); + Py_XDECREF(py_exc_tb); +} + +/* + * If you have the following code: + * + * try: + * try: + * something_that_raises_bt2_error() + * except bt2.Error as e1: + * raise ValueError from e1 + * except ValueError as e2: + * raise TypeError from e2 + * + * We will have the following exception chain: + * + * TypeError -> ValueError -> bt2.Error + * + * Where the TypeError is the current exception (obtained from PyErr_Fetch). + * + * The bt2.Error contains a `struct bt_error *` that used to be the current + * thread's error, at the moment the exception was raised. + * + * This function gets to the bt2.Error and restores the wrapped + * `struct bt_error *` as the current thread's error. + * + * Then, for each exception in the chain, starting with the oldest one, it adds + * an error cause to the current thread's error. + */ +static +void restore_bt_error_and_append_current_exception_chain( + bt_self_component_class *self_component_class, + bt_self_component *self_component, + bt_self_message_iterator *self_message_iterator, + const char *module_name) +{ + BT_ASSERT(PyErr_Occurred()); + + /* Used to access and restore the current exception. */ + PyObject *py_exc_type; + PyObject *py_exc_value; + PyObject *py_exc_tb; + + /* Fetch and normalize the Python exception. */ + PyErr_Fetch(&py_exc_type, &py_exc_value, &py_exc_tb); + PyErr_NormalizeException(&py_exc_type, &py_exc_value, &py_exc_tb); + BT_ASSERT(py_exc_type); + BT_ASSERT(py_exc_value); + BT_ASSERT(py_exc_tb); + + /* + * Set the exception's traceback so it's possible to get it using + * PyException_GetTraceback in + * restore_current_thread_error_and_append_exception_chain_recursive. + */ + PyException_SetTraceback(py_exc_value, py_exc_tb); + + restore_current_thread_error_and_append_exception_chain_recursive(py_exc_value, + self_component_class, self_component, self_message_iterator, + module_name); + + PyErr_Restore(py_exc_type, py_exc_value, py_exc_tb); +} + static inline void log_exception_and_maybe_append_error(int log_level, bool append_error, bt_self_component_class *self_component_class, bt_self_component *self_component, - bt_self_message_iterator *self_message_iterator) + bt_self_message_iterator *self_message_iterator, + const char *module_name) { GString *gstr; @@ -191,19 +341,10 @@ void log_exception_and_maybe_append_error(int log_level, BT_LOG_WRITE(log_level, BT_LOG_TAG, "%s", gstr->str); if (append_error) { - if (self_component_class) { - BT_CURRENT_THREAD_ERROR_APPEND_CAUSE_FROM_COMPONENT_CLASS( - self_component_class, "%s", gstr->str); - } else if (self_component) { - BT_CURRENT_THREAD_ERROR_APPEND_CAUSE_FROM_COMPONENT( - self_component, "%s", gstr->str); - } else if (self_message_iterator) { - BT_CURRENT_THREAD_ERROR_APPEND_CAUSE_FROM_MESSAGE_ITERATOR( - self_message_iterator, "%s", gstr->str); - } else { - BT_CURRENT_THREAD_ERROR_APPEND_CAUSE_FROM_UNKNOWN( - "Python", "%s", gstr->str); - } + restore_bt_error_and_append_current_exception_chain( + self_component_class, self_component, + self_message_iterator, module_name); + } end: @@ -213,28 +354,32 @@ end: } static inline -void loge_exception(void) +void loge_exception(const char *module_name) { - log_exception_and_maybe_append_error(BT_LOG_ERROR, true, NULL, NULL, NULL); + log_exception_and_maybe_append_error(BT_LOG_ERROR, true, NULL, NULL, + NULL, module_name); } static void loge_exception_message_iterator( bt_self_message_iterator *self_message_iterator) { - log_exception_and_maybe_append_error(BT_LOG_ERROR, true, NULL, NULL, self_message_iterator); + log_exception_and_maybe_append_error(BT_LOG_ERROR, true, NULL, NULL, + self_message_iterator, NULL); } static inline void logw_exception(void) { - log_exception_and_maybe_append_error(BT_LOG_WARNING, false, NULL, NULL, NULL); + log_exception_and_maybe_append_error(BT_LOG_WARNING, false, NULL, NULL, + NULL, NULL); } static inline int py_exc_to_status(bt_self_component_class *self_component_class, bt_self_component *self_component, - bt_self_message_iterator *self_message_iterator) + bt_self_message_iterator *self_message_iterator, + const char *module_name) { int status = __BT_FUNC_STATUS_OK; PyObject *exc = PyErr_Occurred(); @@ -262,7 +407,7 @@ int py_exc_to_status(bt_self_component_class *self_component_class, /* Unknown exception: convert to general error */ log_exception_and_maybe_append_error(BT_LOG_WARNING, true, self_component_class, self_component, - self_message_iterator); + self_message_iterator, module_name); status = __BT_FUNC_STATUS_ERROR; } @@ -274,20 +419,20 @@ end: static int py_exc_to_status_component_class(bt_self_component_class *self_component_class) { - return py_exc_to_status(self_component_class, NULL, NULL); + return py_exc_to_status(self_component_class, NULL, NULL, NULL); } static int py_exc_to_status_component(bt_self_component *self_component) { - return py_exc_to_status(NULL, self_component, NULL); + return py_exc_to_status(NULL, self_component, NULL, NULL); } static int py_exc_to_status_message_iterator( bt_self_message_iterator *self_message_iterator) { - return py_exc_to_status(NULL, NULL, self_message_iterator); + return py_exc_to_status(NULL, NULL, self_message_iterator, NULL); } /* Component class proxy methods (delegate to the attached Python object) */ @@ -519,7 +664,7 @@ component_class_seek_beginning(bt_self_message_iterator *self_message_iterator) py_result = PyObject_CallMethod(py_iter, "_bt_seek_beginning_from_native", NULL); BT_ASSERT(!py_result || py_result == Py_None); - status = py_exc_to_status_message_iterator(self_message_iterator); + status = py_exc_to_status_message_iterator(self_message_iterator); Py_XDECREF(py_result); return status; } diff --git a/src/bindings/python/bt2/bt2/native_bt_error.i b/src/bindings/python/bt2/bt2/native_bt_error.i new file mode 100644 index 00000000..e70ebe69 --- /dev/null +++ b/src/bindings/python/bt2/bt2/native_bt_error.i @@ -0,0 +1,31 @@ +/* + * The MIT License (MIT) + * Copyright (c) 2019 EfficiOS Inc. and Linux Foundation + * + * Permission is hereby granted, free of charge, to any person obtaining a copy + * of this software and associated documentation files (the "Software"), to deal + * in the Software without restriction, including without limitation the rights + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell + * copies of the Software, and to permit persons to whom the Software is + * furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in + * all copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE + * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN + * THE SOFTWARE. + */ + +/* + * We include current-thread.h here, because for now, it only contains + * error-related things. + */ +%include +%include +%include + diff --git a/src/bindings/python/bt2/bt2/native_bt_graph.i b/src/bindings/python/bt2/bt2/native_bt_graph.i index ee32e17f..c592452d 100644 --- a/src/bindings/python/bt2/bt2/native_bt_graph.i +++ b/src/bindings/python/bt2/bt2/native_bt_graph.i @@ -145,7 +145,7 @@ static bt_graph_listener_func_status port_added_listener( py_res = PyObject_CallFunction(py_callable, "(OiOi)", py_component_ptr, component_class_type, py_port_ptr, port_type); if (!py_res) { - loge_exception(); + loge_exception("Graph's port added listener (Python)"); PyErr_Clear(); status = __BT_FUNC_STATUS_ERROR; goto end; @@ -390,7 +390,7 @@ bt_graph_listener_func_status ports_connected_listener( py_downstream_component_ptr, downstream_component_class_type, py_downstream_port_ptr); if (!py_res) { - loge_exception(); + loge_exception("Graph's port connected listener (Python)"); PyErr_Clear(); status = __BT_FUNC_STATUS_ERROR; goto end; diff --git a/src/bindings/python/bt2/bt2/native_bt_trace.i b/src/bindings/python/bt2/bt2/native_bt_trace.i index 1ecc4720..34530299 100644 --- a/src/bindings/python/bt2/bt2/native_bt_trace.i +++ b/src/bindings/python/bt2/bt2/native_bt_trace.i @@ -43,7 +43,7 @@ trace_destroyed_listener(const bt_trace *trace, void *py_callable) if (py_res) { BT_ASSERT(py_res == Py_None); } else { - loge_exception(); + loge_exception("Trace's destruction listener (Python)"); } Py_DECREF(py_trace_ptr); diff --git a/src/bindings/python/bt2/bt2/native_bt_trace_class.i b/src/bindings/python/bt2/bt2/native_bt_trace_class.i index 60c3e7d8..fe17de7b 100644 --- a/src/bindings/python/bt2/bt2/native_bt_trace_class.i +++ b/src/bindings/python/bt2/bt2/native_bt_trace_class.i @@ -44,7 +44,7 @@ trace_class_destroyed_listener(const bt_trace_class *trace_class, void *py_calla if (py_res) { BT_ASSERT(py_res == Py_None); } else { - loge_exception(); + loge_exception("Trace class's destruction listener (Python)"); } Py_DECREF(py_trace_class_ptr); diff --git a/src/bindings/python/bt2/bt2/plugin.py b/src/bindings/python/bt2/bt2/plugin.py index 86612920..5198bdf3 100644 --- a/src/bindings/python/bt2/bt2/plugin.py +++ b/src/bindings/python/bt2/bt2/plugin.py @@ -42,7 +42,7 @@ def find_plugins(path, recurse=True, fail_on_load_error=False): path, int(recurse), int(fail_on_load_error) ) else: - raise bt2.Error("invalid path: '{}'".format(path)) + raise ValueError("invalid path: '{}'".format(path)) if status == native_bt.__BT_FUNC_STATUS_NOT_FOUND: return diff --git a/src/bindings/python/bt2/bt2/stream.py b/src/bindings/python/bt2/bt2/stream.py index 451ad6c9..70011758 100644 --- a/src/bindings/python/bt2/bt2/stream.py +++ b/src/bindings/python/bt2/bt2/stream.py @@ -55,7 +55,7 @@ class _Stream(bt2.object._SharedObject): def create_packet(self): if not self.cls.supports_packets: - raise bt2.Error( + raise ValueError( 'cannot create packet: stream class does not support packets' ) diff --git a/src/bindings/python/bt2/bt2/trace_collection_message_iterator.py b/src/bindings/python/bt2/bt2/trace_collection_message_iterator.py index c6d147af..1b68d205 100644 --- a/src/bindings/python/bt2/bt2/trace_collection_message_iterator.py +++ b/src/bindings/python/bt2/bt2/trace_collection_message_iterator.py @@ -149,7 +149,7 @@ class TraceCollectionMessageIterator(bt2.message_iterator._MessageIterator): try: inputs = src_comp_and_spec.spec.params['inputs'] except Exception as e: - raise bt2.Error( + raise ValueError( 'all source components must be created with an "inputs" parameter in stream intersection mode' ) from e @@ -178,7 +178,7 @@ class TraceCollectionMessageIterator(bt2.message_iterator._MessageIterator): pass if begin is None or end is None: - raise bt2.Error( + raise RuntimeError( 'cannot find stream intersection range for port "{}"'.format(port.name) ) @@ -189,10 +189,10 @@ class TraceCollectionMessageIterator(bt2.message_iterator._MessageIterator): plugin = bt2.find_plugin('utils') if plugin is None: - raise bt2.Error('cannot find "utils" plugin (needed for the muxer)') + raise RuntimeError('cannot find "utils" plugin (needed for the muxer)') if 'muxer' not in plugin.filter_component_classes: - raise bt2.Error( + raise RuntimeError( 'cannot find "muxer" filter component class in "utils" plugin' ) @@ -203,10 +203,10 @@ class TraceCollectionMessageIterator(bt2.message_iterator._MessageIterator): plugin = bt2.find_plugin('utils') if plugin is None: - raise bt2.Error('cannot find "utils" plugin (needed for the trimmer)') + raise RuntimeError('cannot find "utils" plugin (needed for the trimmer)') if 'trimmer' not in plugin.filter_component_classes: - raise bt2.Error( + raise RuntimeError( 'cannot find "trimmer" filter component class in "utils" plugin' ) @@ -242,7 +242,7 @@ class TraceCollectionMessageIterator(bt2.message_iterator._MessageIterator): plugin = bt2.find_plugin(comp_spec.plugin_name) if plugin is None: - raise bt2.Error('no such plugin: {}'.format(comp_spec.plugin_name)) + raise ValueError('no such plugin: {}'.format(comp_spec.plugin_name)) if comp_cls_type == _CompClsType.SOURCE: comp_classes = plugin.source_component_classes @@ -251,7 +251,7 @@ class TraceCollectionMessageIterator(bt2.message_iterator._MessageIterator): if comp_spec.class_name not in comp_classes: cc_type = 'source' if comp_cls_type == _CompClsType.SOURCE else 'filter' - raise bt2.Error( + raise ValueError( 'no such {} component class in "{}" plugin: {}'.format( cc_type, comp_spec.plugin_name, comp_spec.class_name ) diff --git a/src/bindings/python/bt2/bt2/utils.py b/src/bindings/python/bt2/bt2/utils.py index 39b5f809..8d5c4e2f 100644 --- a/src/bindings/python/bt2/bt2/utils.py +++ b/src/bindings/python/bt2/bt2/utils.py @@ -139,10 +139,8 @@ def _handle_func_status(status, msg=None): status == native_bt.__BT_FUNC_STATUS_ERROR or status == native_bt.__BT_FUNC_STATUS_MEMORY_ERROR ): - if msg is None: - raise bt2.Error - else: - raise bt2.Error(msg) + assert msg is not None + raise bt2.Error(msg) elif status == native_bt.__BT_FUNC_STATUS_END: if msg is None: raise bt2.Stop diff --git a/tests/bindings/python/bt2/test_component_class.py b/tests/bindings/python/bt2/test_component_class.py index 857d1527..bbd9774a 100644 --- a/tests/bindings/python/bt2/test_component_class.py +++ b/tests/bindings/python/bt2/test_component_class.py @@ -23,7 +23,7 @@ import bt2 class UserComponentClassTestCase(unittest.TestCase): def _test_no_init(self, cls): - with self.assertRaises(bt2.Error): + with self.assertRaises(RuntimeError): cls() def test_no_init_source(self): diff --git a/tests/bindings/python/bt2/test_error.py b/tests/bindings/python/bt2/test_error.py new file mode 100644 index 00000000..81b237aa --- /dev/null +++ b/tests/bindings/python/bt2/test_error.py @@ -0,0 +1,213 @@ +# +# Copyright (C) 2019 EfficiOS Inc. +# +# This program is free software; you can redistribute it and/or +# modify it under the terms of the GNU General Public License +# as published by the Free Software Foundation; only version 2 +# of the License. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program; if not, write to the Free Software +# Foundation, Inc., 59 Temple Place - Suite 330, Boston, MA 02111-1307, USA. +# + +from bt2 import native_bt +import bt2 +import unittest + + +class FailingIter(bt2._UserMessageIterator): + def __next__(self): + raise ValueError('User message iterator is failing') + + +class SourceWithFailingIter( + bt2._UserSourceComponent, message_iterator_class=FailingIter +): + def __init__(self, params): + self._add_output_port('out') + + +class SourceWithFailingInit( + bt2._UserSourceComponent, message_iterator_class=FailingIter +): + def __init__(self, params): + raise ValueError('Source is failing') + + +class WorkingSink(bt2._UserSinkComponent): + def __init__(self, params): + self._in = self._add_input_port('in') + + def _graph_is_configured(self): + self._iter = self._in.create_message_iterator() + + def _consume(self): + next(self._iter) + + +class SinkWithExceptionChaining(bt2._UserSinkComponent): + def __init__(self, params): + self._in = self._add_input_port('in') + + def _graph_is_configured(self): + self._iter = self._in.create_message_iterator() + + def _consume(self): + try: + print(self._iter.__next__) + next(self._iter) + except bt2.Error as e: + print(hex(id(e))) + print(e.__dict__) + raise ValueError('oops') from e + + +class SinkWithFailingQuery(bt2._UserSinkComponent): + def _graph_is_configured(self): + pass + + def _consume(self): + pass + + @staticmethod + def _query(executor, obj, params, log_level): + raise ValueError('Query is failing') + + +class ErrorTestCase(unittest.TestCase): + def _run_failing_graph(self, source_cc, sink_cc): + with self.assertRaises(bt2.Error) as ctx: + graph = bt2.Graph() + src = graph.add_component(source_cc, 'src') + snk = graph.add_component(sink_cc, 'snk') + graph.connect_ports(src.output_ports['out'], snk.input_ports['in']) + graph.run() + + return ctx.exception + + def test_current_thread_error_none(self): + # When a bt2.Error is raised, it steals the current thread's error. + # Verify that it is now NULL. + exc = self._run_failing_graph(SourceWithFailingInit, WorkingSink) + self.assertIsNone(native_bt.current_thread_take_error()) + + def test_len(self): + exc = self._run_failing_graph(SourceWithFailingIter, WorkingSink) + + # The exact number of causes is not too important (it can change if we + # append more or less causes along the way), but the idea is to verify is + # has a value that makes sense. + self.assertEqual(len(exc), 4) + + def test_iter(self): + exc = self._run_failing_graph(SourceWithFailingIter, WorkingSink) + + for c in exc: + # Each cause is an instance of _ErrorCause (including subclasses). + self.assertIsInstance(c, bt2.error._ErrorCause) + + def test_getitem(self): + exc = self._run_failing_graph(SourceWithFailingIter, WorkingSink) + + for i in range(len(exc)): + c = exc[i] + # Each cause is an instance of _ErrorCause (including subclasses). + self.assertIsInstance(c, bt2.error._ErrorCause) + + def test_getitem_indexerror(self): + exc = self._run_failing_graph(SourceWithFailingIter, WorkingSink) + + with self.assertRaises(IndexError): + exc[len(exc)] + + def test_exception_chaining(self): + # Test that if we do: + # + # try: + # ... + # except bt2.Error as exc: + # raise ValueError('oh noes') from exc + # + # We are able to fetch the causes of the original bt2.Error in the + # exception chain. Also, each exception in the chain should become one + # cause once caught. + exc = self._run_failing_graph(SourceWithFailingIter, SinkWithExceptionChaining) + + self.assertEqual(len(exc), 5) + + self.assertIsInstance(exc[0], bt2.error._MessageIteratorErrorCause) + self.assertEqual(exc[0].component_class_name, 'SourceWithFailingIter') + self.assertIn('ValueError: User message iterator is failing', exc[0].message) + + self.assertIsInstance(exc[1], bt2.error._ErrorCause) + + self.assertIsInstance(exc[2], bt2.error._ComponentErrorCause) + self.assertEqual(exc[2].component_class_name, 'SinkWithExceptionChaining') + self.assertIn( + 'bt2.error.Error: unexpected error: cannot advance the message iterator', + exc[2].message, + ) + + self.assertIsInstance(exc[3], bt2.error._ComponentErrorCause) + self.assertEqual(exc[3].component_class_name, 'SinkWithExceptionChaining') + self.assertIn('ValueError: oops', exc[3].message) + + self.assertIsInstance(exc[4], bt2.error._ErrorCause) + + def _common_cause_tests(self, cause): + self.assertIsInstance(cause.module_name, str) + self.assertIsInstance(cause.file_name, str) + self.assertIsInstance(cause.line_number, int) + + def test_unknown_error_cause(self): + exc = self._run_failing_graph(SourceWithFailingIter, SinkWithExceptionChaining) + cause = exc[-1] + self.assertIs(type(cause), bt2.error._ErrorCause) + self._common_cause_tests(cause) + + def test_component_error_cause(self): + exc = self._run_failing_graph(SourceWithFailingInit, SinkWithExceptionChaining) + cause = exc[0] + self.assertIs(type(cause), bt2.error._ComponentErrorCause) + self._common_cause_tests(cause) + + self.assertIn('Source is failing', cause.message) + self.assertEqual(cause.component_name, 'src') + self.assertEqual(cause.component_class_type, bt2.ComponentClassType.SOURCE) + self.assertEqual(cause.component_class_name, 'SourceWithFailingInit') + self.assertIsNone(cause.plugin_name) + + def test_component_class_error_cause(self): + q = bt2.QueryExecutor() + + with self.assertRaises(bt2.Error) as ctx: + q.query(SinkWithFailingQuery, 'hello') + + cause = ctx.exception[0] + self.assertIs(type(cause), bt2.error._ComponentClassErrorCause) + self._common_cause_tests(cause) + + self.assertIn('Query is failing', cause.message) + + self.assertEqual(cause.component_class_type, bt2.ComponentClassType.SINK) + self.assertEqual(cause.component_class_name, 'SinkWithFailingQuery') + self.assertIsNone(cause.plugin_name) + + def test_message_iterator_error_cause(self): + exc = self._run_failing_graph(SourceWithFailingIter, SinkWithExceptionChaining) + cause = exc[0] + self.assertIs(type(cause), bt2.error._MessageIteratorErrorCause) + self._common_cause_tests(cause) + + self.assertIn('User message iterator is failing', cause.message) + self.assertEqual(cause.component_name, 'src') + self.assertEqual(cause.component_output_port_name, 'out') + self.assertEqual(cause.component_class_type, bt2.ComponentClassType.SOURCE) + self.assertEqual(cause.component_class_name, 'SourceWithFailingIter') + self.assertIsNone(cause.plugin_name) diff --git a/tests/bindings/python/bt2/test_field_class.py b/tests/bindings/python/bt2/test_field_class.py index 2b05f009..6914dc0f 100644 --- a/tests/bindings/python/bt2/test_field_class.py +++ b/tests/bindings/python/bt2/test_field_class.py @@ -132,7 +132,7 @@ class _EnumerationFieldClassTestCase(_TestIntegerFieldClassProps): self._fc.add_mapping('allo', 'meow') def test_add_mapping_dup_label(self): - with self.assertRaises(bt2.Error): + with self.assertRaises(ValueError): self._fc.add_mapping('a', self._ranges1) self._fc.add_mapping('a', self._ranges2) @@ -263,7 +263,7 @@ class _TestElementContainer: sub_fc1 = self._tc.create_string_field_class() sub_fc2 = self._tc.create_string_field_class() - with self.assertRaises(bt2.Error): + with self.assertRaises(ValueError): self._append_element_method(self._fc, 'yes', sub_fc1) self._append_element_method(self._fc, 'yes', sub_fc2) @@ -446,7 +446,7 @@ class _VariantFieldClassWithSelectorTestCase: sub_fc1 = self._tc.create_string_field_class() sub_fc2 = self._tc.create_string_field_class() - with self.assertRaises(bt2.Error): + with self.assertRaises(ValueError): self._fc.append_option('yes', sub_fc1, self._ranges1) self._fc.append_option('yes', sub_fc2, self._ranges2) diff --git a/tests/bindings/python/bt2/test_plugin.py b/tests/bindings/python/bt2/test_plugin.py index 9ba6cd9a..d7e0965b 100644 --- a/tests/bindings/python/bt2/test_plugin.py +++ b/tests/bindings/python/bt2/test_plugin.py @@ -49,7 +49,7 @@ class PluginSetTestCase(unittest.TestCase): class FindPluginsTestCase(unittest.TestCase): def test_find_nonexistent_dir(self): - with self.assertRaises(bt2.Error): + with self.assertRaises(ValueError): bt2.find_plugins( '/this/does/not/exist/246703df-cb85-46d5-8406-5e8dc4a88b41' ) diff --git a/tests/bindings/python/bt2/test_query_executor.py b/tests/bindings/python/bt2/test_query_executor.py index f313b173..9f57f30b 100644 --- a/tests/bindings/python/bt2/test_query_executor.py +++ b/tests/bindings/python/bt2/test_query_executor.py @@ -97,9 +97,17 @@ class QueryExecutorTestCase(unittest.TestCase): def _query(cls, query_exec, obj, params, log_level): raise ValueError - with self.assertRaises(bt2.Error): + with self.assertRaises(bt2.Error) as ctx: res = bt2.QueryExecutor().query(MySink, 'obj', [17, 23]) + exc = ctx.exception + self.assertEqual(len(exc), 1) + cause = exc[0] + self.assertIsInstance(cause, bt2.error._ComponentClassErrorCause) + self.assertIn('raise ValueError', cause.message) + self.assertEqual(cause.component_class_type, bt2.ComponentClassType.SINK) + self.assertEqual(cause.component_class_name, 'MySink') + def test_query_invalid_object(self): class MySink(bt2._UserSinkComponent): def _consume(self): diff --git a/tests/bindings/python/bt2/test_trace_collection_message_iterator.py b/tests/bindings/python/bt2/test_trace_collection_message_iterator.py index 63198c68..25c1373a 100644 --- a/tests/bindings/python/bt2/test_trace_collection_message_iterator.py +++ b/tests/bindings/python/bt2/test_trace_collection_message_iterator.py @@ -89,7 +89,7 @@ class TraceCollectionMessageIteratorTestCase(unittest.TestCase): def test_create_no_such_plugin(self): specs = [bt2.ComponentSpec('77', '101', _3EVENTS_INTERSECT_TRACE_PATH)] - with self.assertRaises(bt2.Error): + with self.assertRaises(ValueError): bt2.TraceCollectionMessageIterator(specs) def test_create_begin_s(self): @@ -145,7 +145,7 @@ class TraceCollectionMessageIteratorTestCase(unittest.TestCase): def test_iter_intersection_no_inputs_param(self): specs = [bt2.ComponentSpec('text', 'dmesg', {'read-from-stdin': True})] - with self.assertRaises(bt2.Error): + with self.assertRaises(ValueError): bt2.TraceCollectionMessageIterator(specs, stream_intersection_mode=True) def test_iter_no_intersection_two_traces(self): -- 2.34.1